Closed
Bug 1288791
Opened 8 years ago
Closed 8 years ago
Binding code can call JS_NewObjectWithGivenProto with a gray proto
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bkelly
:
review+
|
Details | Diff | Splinter Review |
Most simply, consider an object which returns an nsIGlobalObject* from GetParentObject. We'll call GetGlobalJSObject() on it, which does NOT ungray at least on window, and return that. Then we'll get the default proto from it, but if the global is gray the proto will be too.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8773897 -
Flags: review?(bkelly)
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8773898 -
Flags: review?(bkelly)
Assignee | ||
Comment 3•8 years ago
|
||
Attachment #8773899 -
Flags: review?(bkelly)
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8774012 -
Flags: review?(bkelly)
Assignee | ||
Updated•8 years ago
|
Attachment #8773898 -
Attachment is obsolete: true
Attachment #8773898 -
Flags: review?(bkelly)
Comment 5•8 years ago
|
||
Boris, can you help me understand the impact of returning a gray thing from these APIs? The changes all seem consistent and ok, but I want to make sure I understand whats happening.
Flags: needinfo?(bzbarsky)
Comment 6•8 years ago
|
||
(In reply to Ben Kelly [:bkelly] from comment #5)
> Boris, can you help me understand the impact of returning a gray thing from
> these APIs? The changes all seem consistent and ok, but I want to make sure
> I understand whats happening.
The cycle collector requires as an invariant that a black object never point to a gray object ("the black-gray invariant"). We want to maintain that invariant by ensuring that we never write a gray object into a black object. We do that by making sure that any JS object we are going to do anything substantial with is black, so we don't have to worry about whether we can write it or not.
The black-gray invariant is needed because the CC assumes that any gray object is not reachable from a JS root. (It does this to avoid tracing black JS objects, except in an all-traces CC for debugging.)
Updated•8 years ago
|
Attachment #8773897 -
Flags: review?(bkelly) → review+
Comment 8•8 years ago
|
||
Comment on attachment 8774012 [details] [diff] [review]
part 2. Rename WrapNativeParent to FindAssociatedGlobal and update it to actually do that
Review of attachment 8774012 [details] [diff] [review]:
-----------------------------------------------------------------
::: js/xpconnect/crashtests/786142-iframe.html
@@ +24,5 @@
> // getting to scope 3 (as explained below). This means that we can't trigger
> // the PreCreate hook for |input|, because that will call WrapNativeParent
> + // (Well... No, it won't: there is no WrapNativeParent, but there are also no
> + // more pre-create hooks, slimwrappers, parenting to the form, or any of the
> + // stuff this test is trying to test.)
Should we just remove the test?
Attachment #8774012 -
Flags: review?(bkelly) → review+
Updated•8 years ago
|
Attachment #8773899 -
Flags: review?(bkelly) → review+
Assignee | ||
Comment 9•8 years ago
|
||
> Should we just remove the test?
Hard to say. It's still testing an interesting situation, so removing it would lose _some_ test coverage. Just the particular codepaths it was trying to test no longer exist.
Comment 10•8 years ago
|
||
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1188f77967d
part 1. Rename the GetParentObject template bits to make it clearer what they're really doing: finding the associated global for an object's native. r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/45010c0db1c5
part 2. Rename WrapNativeParent to FindAssociatedGlobal and update it to actually do that. r=bkelly
https://hg.mozilla.org/integration/mozilla-inbound/rev/45c51ccab098
part 3. Ensure that FindAssociatedGlobal never returns a gray global. r=bkelly
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d1188f77967d
https://hg.mozilla.org/mozilla-central/rev/45010c0db1c5
https://hg.mozilla.org/mozilla-central/rev/45c51ccab098
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
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
•