Bug 1260417 - Part c: Stop mentioning requests around CallSetup::mRootedCallable; r=bz
authorMs2ger <Ms2ger@gmail.com>
Thu, 07 Apr 2016 09:11:52 +0200
changeset 348380 5d533891bb5134a0891951e767677fa7bde3c96b
parent 348379 6ec261488c7598b3774982480149493148b47ca8
child 348381 65d070edff8bf23b4e4cb975bbdc24b61e301f7a
push id14826
push userbenj@benj.me
push dateThu, 07 Apr 2016 11:56:36 +0000
reviewersbz
bugs1260417
milestone48.0a1
Bug 1260417 - Part c: Stop mentioning requests around CallSetup::mRootedCallable; r=bz Rooted used to only work when the JSContext was in a request. This has long been rectified, so the comments referring to that constraint are confusing at best.
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -130,22 +130,19 @@ CallbackObject::CallSetup::CallSetup(Cal
       mAutoIncumbentScript.emplace(incumbent);
     }
 
     cx = mAutoEntryScript->cx();
 
     // Unmark the callable (by invoking Callback() and not the CallbackPreserveColor()
     // variant), and stick it in a Rooted before it can go gray again.
     // Nothing before us in this function can trigger a CC, so it's safe to wait
-    // until here it do the unmark. This allows us to order the following two
-    // operations _after_ the Push() above, which lets us take advantage of the
-    // JSAutoRequest embedded in the pusher.
-    //
-    // We can do this even though we're not in the right compartment yet, because
-    // Rooted<> does not care about compartments.
+    // until here it do the unmark. This allows us to construct mRootedCallable
+    // with the cx from mAutoEntryScript, avoiding the cost of finding another
+    // JSContext. (Rooted<> does not care about requests or compartments.)
     mRootedCallable.emplace(cx, aCallback->Callback());
   }
 
   // JS-implemented WebIDL is always OK to run, since it runs with Chrome
   // privileges anyway.
   if (mIsMainThread && !aIsJSImplementedWebIDL) {
     // Check that it's ok to run this callback at all.
     // Make sure to use realCallback to get the global of the callback object,
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -250,18 +250,16 @@ protected:
     // Caller's compartment. This will only have a sensible value if
     // mExceptionHandling == eRethrowContentExceptions or eRethrowExceptions.
     JSCompartment* mCompartment;
 
     // And now members whose construction/destruction order we need to control.
     Maybe<AutoEntryScript> mAutoEntryScript;
     Maybe<AutoIncumbentScript> mAutoIncumbentScript;
 
-    // Constructed the rooter within the scope of mCxPusher above, so that it's
-    // always within a request during its lifetime.
     Maybe<JS::Rooted<JSObject*> > mRootedCallable;
 
     // Members which are used to set the async stack.
     Maybe<JS::Rooted<JSObject*>> mAsyncStack;
     Maybe<JS::AutoSetAsyncStackForNewCalls> mAsyncStackSetter;
 
     // Can't construct a JSAutoCompartment without a JSContext either.  Also,
     // Put mAc after mAutoEntryScript so that we exit the compartment before