Closed Bug 1283636 Opened 3 years ago Closed 3 years ago

Use CallbackPreserveColor() less in bindings code

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

Details

Attachments

(1 file)

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.
Sure, having a CallbackKnownNotGray() would be fine.
Assignee: nobody → continuation
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.
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 on attachment 8770702 [details] [diff] [review]
Use CallbackPreserveColor() less.

r=me
Attachment #8770702 - Flags: review?(bzbarsky) → review+
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
https://hg.mozilla.org/mozilla-central/rev/347b46d43b8b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.