Closed
Bug 1283636
Opened 8 years ago
Closed 8 years ago
Use CallbackPreserveColor() less in bindings code
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: mccr8, Assigned: mccr8)
Details
Attachments
(1 file)
4.42 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
DOM bindings codegen uses CallbackPreserveColor() on callbacks that it knows should be already non-gray. The reason for this is not immediately obvious when looking at the call sites, and nothing actually verifies this property. It seems like there should be some kind of CallbackAlreadyNonGray() method (name suggestions welcome) that is like CallbackPreserveColor(), but asserts that the object is non-gray.
Comment 1•8 years ago
|
||
Sure, having a CallbackKnownNotGray() would be fine.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 2•8 years ago
|
||
CallbackObject::CallSetup::CallSetup() does this:
JSObject* realCallback = js::UncheckedUnwrap(aCallback->CallbackPreserveColor());
The callback object is gray here sometimes, for instance during ./mach package, but I think it is okay because the uses of realCallback only grab the C++ native off of the object, AFAICT. UncheckUnwrap itself can do a write to the gray object, but that should be okay, because we're only trying to avoid black->gray edges. Seems a little fragile but I'll leave it alone.
Assignee | ||
Comment 3•8 years ago
|
||
Some places call CallbackPreserveColor() because they know the object must have already been marked black. This patch replaces those calls with a new function CallbackKnownNotGray() which documents and dynamically checks this assumption.
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1204210b2817
Attachment #8770702 -
Flags: review?(bzbarsky)
Comment 4•8 years ago
|
||
Comment on attachment 8770702 [details] [diff] [review]
Use CallbackPreserveColor() less.
r=me
Attachment #8770702 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/347b46d43b8b
Use CallbackPreserveColor() less. r=bz
Keywords: checkin-needed
Comment 6•8 years ago
|
||
bugherder |
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
•