Closed
Bug 1316271
Opened 8 years ago
Closed 8 years ago
Crash in nsContentUtils::SubjectPrincipal
Categories
(Core :: DOM: Core & HTML, defect, P1)
Tracking
()
RESOLVED
DUPLICATE
of bug 1317167
People
(Reporter: ting, Unassigned)
References
Details
(4 keywords)
Crash Data
This bug was filed from the Socorro interface and is
report bp-6dbf1e0c-692f-4fcb-b52c-4b2eb2161108.
=============================================================
#1 top crash on Android for Nightly 20161106030203, 12 crashes from 7 installations.
Comment 1•8 years ago
|
||
The code is using hazard nsContentUtils::IsCallerChrome when nsContentUtils::LegacyIsCallerChromeOrNativeCode() should be used.
Blocks: 1242874
Comment 2•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #1)
> The code is using hazard nsContentUtils::IsCallerChrome when
> nsContentUtils::LegacyIsCallerChromeOrNativeCode() should be used.
Please stop suggesting that - the "Legacy" is there for a reason. We've talked about this several times.
The correct solution is to remove the SubjectPrincipal call, and have the method take a boolean parameter indicating whether the caller is privileged. Then we mark the relevant WebIDL entry points as [NeedsSubjectPrincipal], and pass "true" for system entry points.
Should be a relatively straightforward mechanical change for a DOM hacker.
Comment 3•8 years ago
|
||
Yes, we should be aiming to eliminate IsCallerChrome, as well as the Legacy* APIs, from the tree...
Comment 4•8 years ago
|
||
FWIW, my point is that nsContentUtils::IsCallerChrome() is so super hazard API, that we shouldn't have such method in tree at all, but just use nsContentUtils::LegacyIsCallerChromeOrNativeCode() until we've removed all the cases that is needed.
This particular code is now in release builds, so we happily crash there.
Comment 5•8 years ago
|
||
Bobby, IsCallerChrome tests for system principal or IsUniversalXPConnectEnabled(). The latter isn't really captured by webidl's NeedsSubjectPrincipal, right?
Do we not really care about IsUniversalXPConnectEnabled() anymore?
Flags: needinfo?(bobbyholley)
Comment 6•8 years ago
|
||
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #5)
> Bobby, IsCallerChrome tests for system principal or
> IsUniversalXPConnectEnabled(). The latter isn't really captured by webidl's
> NeedsSubjectPrincipal, right?
>
> Do we not really care about IsUniversalXPConnectEnabled() anymore?
Pretty much. UniversalXPConnect only matters insofar as it's necessary for supporting legacy tests. If you can remove support for it at a callsite while keeping the tree green, feel free to do so. If a test breaks, feel free to fix the test.
Flags: needinfo?(bobbyholley)
Comment 7•8 years ago
|
||
Oh, right. I had totally forgotten how much we had nerfed it.
OK, filed bug 1316480.
Comment 8•8 years ago
|
||
[Tracking Requested - why for this release]: Android Nightly top crash.
tracking-firefox52:
--- → ?
Comment 10•8 years ago
|
||
Bug 1316758 suggests the right course of action is removing the call altogether.
That bug also points out that the existing LegacyIsCallerChromeOrNativeCode() callers in this file are wrong because they allow pages to do stuff they shouldn't be able to do. :(
Comment 11•8 years ago
|
||
Hi Blake, looks we need to have bug 1316758 be prioritized to fix this top crash. Thank you.
Flags: needinfo?(bwu)
Priority: -- → P1
Comment 12•8 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #11)
> Hi Blake, looks we need to have bug 1316758 be prioritized to fix this top
> crash. Thank you.
Sure!
Flags: needinfo?(bwu)
Comment 13•8 years ago
|
||
Same issue, root cause is on the bug1317167 comment13.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Updated•8 years ago
|
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•