Bug 937317 - Pass the entire CallbackObject to CallSetup. r=bz
authorBobby Holley <bobbyholley@gmail.com>
Fri, 06 Dec 2013 12:01:42 -0800
changeset 174967 c5940c217bd9dd7c65eb116d2b49fb69fdd486d0
parent 174966 a6b672cbd54db73e2f0355ae822f30e16c5ffb1a
child 174968 7be223bdc61a6de2aa224ca6fa773576464135ad
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs937317
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 937317 - Pass the entire CallbackObject to CallSetup. r=bz We need this so that we can grab the incumbent global that we stashed on the CallbackObject.
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
dom/bindings/Codegen.py
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -39,17 +39,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(Ca
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(CallbackObject)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(CallbackObject)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mCallback)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
-CallbackObject::CallSetup::CallSetup(JS::Handle<JSObject*> aCallback,
+CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
                                      ErrorResult& aRv,
                                      ExceptionHandling aExceptionHandling,
                                      JSCompartment* aCompartment)
   : mCx(nullptr)
   , mCompartment(aCompartment)
   , mErrorResult(aRv)
   , mExceptionHandling(aExceptionHandling)
   , mIsMainThread(NS_IsMainThread())
@@ -59,17 +59,17 @@ CallbackObject::CallSetup::CallSetup(JS:
   }
   // We need to produce a useful JSContext here.  Ideally one that the callback
   // is in some sense associated with, so that we can sort of treat it as a
   // "script entry point".  Though once we actually have script entry points,
   // we'll need to do the script entry point bits once we have an actual
   // callable.
 
   // First, find the real underlying callback.
-  JSObject* realCallback = js::UncheckedUnwrap(aCallback);
+  JSObject* realCallback = js::UncheckedUnwrap(aCallback->CallbackPreserveColor());
   JSContext* cx = nullptr;
   nsIGlobalObject* globalObject = nullptr;
 
   if (mIsMainThread) {
     // Now get the global and JSContext for this callback.
     nsGlobalWindow* win = xpc::WindowGlobalOrNull(realCallback);
     if (win) {
       // Make sure that if this is a window it's the current inner, since the
@@ -97,45 +97,45 @@ CallbackObject::CallSetup::CallSetup(JS:
     }
   } else {
     cx = workers::GetCurrentThreadJSContext();
     globalObject = workers::GetCurrentThreadWorkerPrivate()->GlobalScope();
   }
 
   mAutoEntryScript.construct(globalObject, mIsMainThread, cx);
 
-  // Unmark the callable, and stick it in a Rooted before it can go gray again.
+  // 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.
-  JS::ExposeObjectToActiveJS(aCallback);
-  mRootedCallable.construct(cx, aCallback);
+  mRootedCallable.construct(cx, aCallback->Callback());
 
   if (mIsMainThread) {
     // Check that it's ok to run this callback at all.
-    // Make sure to unwrap aCallback before passing it in to get the global of
-    // the callback object, not the wrapper.
+    // Make sure to use realCallback to get the global of the callback object,
+    // not the wrapper.
     bool allowed = nsContentUtils::GetSecurityManager()->
-      ScriptAllowed(js::GetGlobalForObjectCrossCompartment(js::UncheckedUnwrap(aCallback)));
+      ScriptAllowed(js::GetGlobalForObjectCrossCompartment(realCallback));
 
     if (!allowed) {
       return;
     }
   }
 
   // Enter the compartment of our callback, so we can actually work with it.
   //
   // Note that if the callback is a wrapper, this will not be the same
   // compartment that we ended up in with mAutoEntryScript above, because the
   // entry point is based off of the unwrapped callback (realCallback).
-  mAc.construct(cx, aCallback);
+  mAc.construct(cx, mRootedCallable.ref());
 
   // And now we're ready to go.
   mCx = cx;
 
   // Make sure the JS engine doesn't report exceptions we want to re-throw
   if (mExceptionHandling == eRethrowContentExceptions ||
       mExceptionHandling == eRethrowExceptions) {
     mSavedJSContextOptions = JS::ContextOptionsRef(cx);
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -124,17 +124,17 @@ protected:
      * A class that performs whatever setup we need to safely make a
      * call while this class is on the stack, After the constructor
      * returns, the call is safe to make if GetContext() returns
      * non-null.
      */
   public:
     // If aExceptionHandling == eRethrowContentExceptions then aCompartment
     // needs to be set to the caller's compartment.
-    CallSetup(JS::Handle<JSObject*> aCallable, ErrorResult& aRv,
+    CallSetup(CallbackObject* aCallback, ErrorResult& aRv,
               ExceptionHandling aExceptionHandling,
               JSCompartment* aCompartment = nullptr);
     ~CallSetup();
 
     JSContext* GetContext() const
     {
       return mCx;
     }
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -10573,17 +10573,17 @@ class CGCallback(CGClass):
         # method, insert our optional argument for deciding whether the
         # CallSetup should re-throw exceptions on aRv.
         args.append(Argument("ExceptionHandling", "aExceptionHandling",
                              "eReportExceptions"))
         # And now insert our template argument.
         argsWithoutThis = list(args)
         args.insert(0, Argument("const T&",  "thisObj"))
 
-        setupCall = ("CallSetup s(CallbackPreserveColor(), aRv, aExceptionHandling);\n"
+        setupCall = ("CallSetup s(this, aRv, aExceptionHandling);\n"
                      "if (!s.GetContext()) {\n"
                      "  aRv.Throw(NS_ERROR_UNEXPECTED);\n"
                      "  return${errorReturn};\n"
                      "}\n")
 
         bodyWithThis = string.Template(
             setupCall+
             "JS::Rooted<JSObject*> thisObjJS(s.GetContext(),\n"
@@ -10863,17 +10863,17 @@ class CallbackMember(CGNativeMember):
         # well as a JSContext.
         return [Argument("JSContext*", "cx"),
                 Argument("JS::Handle<JSObject*>", "aThisObj")] + args
 
     def getCallSetup(self):
         if self.needThisHandling:
             # It's been done for us already
             return ""
-        callSetup = "CallSetup s(CallbackPreserveColor(), aRv"
+        callSetup = "CallSetup s(this, aRv"
         if self.rethrowContentException:
             # getArgs doesn't add the aExceptionHandling argument but does add
             # aCompartment for us.
             callSetup += ", eRethrowContentExceptions, aCompartment"
         else:
             callSetup += ", aExceptionHandling"
         callSetup += ");"
         return string.Template(