Bug 1273251: Part 3 - Allow CallbackObject to contain a null callable. r=peterv
authorKris Maglione <maglione.k@gmail.com>
Mon, 14 Nov 2016 21:25:37 -0800
changeset 377336 5d378d9b9a910f2aa6e3fea032c707dcdccecd91
parent 377335 3782cf885346adb952de0f720bb48b7ea9557ed9
child 377337 27c422b6b825c6889e4ecb17737d5c16ef9fe859
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1273251
milestone53.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 1273251: Part 3 - Allow CallbackObject to contain a null callable. r=peterv MozReview-Commit-ID: FCXVHouhG3I
dom/base/CustomElementRegistry.cpp
dom/bindings/BindingUtils.h
dom/bindings/CallbackFunction.h
dom/bindings/CallbackObject.cpp
dom/bindings/CallbackObject.h
dom/bindings/Codegen.py
dom/bindings/ToJSValue.h
dom/console/Console.cpp
dom/events/DOMEventTargetHelper.cpp
dom/events/EventListenerService.cpp
dom/promise/Promise.cpp
dom/workers/WorkerPrivate.cpp
xpcom/base/CycleCollectedJSContext.cpp
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -572,17 +572,17 @@ CustomElementRegistry::Define(const nsAS
 
   AutoJSAPI jsapi;
   if (NS_WARN_IF(!jsapi.Init(mWindow))) {
     aRv.Throw(NS_ERROR_FAILURE);
     return;
   }
 
   JSContext *cx = jsapi.cx();
-  JS::Rooted<JSObject*> constructor(cx, aFunctionConstructor.Callable());
+  JS::Rooted<JSObject*> constructor(cx, aFunctionConstructor.CallableOrNull());
 
   /**
    * 1. If IsConstructor(constructor) is false, then throw a TypeError and abort
    *    these steps.
    */
   // For now, all wrappers are constructable if they are callable. So we need to
   // unwrap constructor to check it is really constructable.
   JS::Rooted<JSObject*> constructorUnwrapped(cx, js::CheckedUnwrap(constructor));
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1723,17 +1723,17 @@ GetOrCreateDOMReflectorNoWrap(JSContext*
   return
     GetOrCreateDOMReflectorNoWrapHelper<T>::GetOrCreate(cx, value, rval);
 }
 
 template <class T>
 inline JSObject*
 GetCallbackFromCallbackObject(T* aObj)
 {
-  return aObj->Callback();
+  return aObj->CallbackOrNull();
 }
 
 // Helper for getting the callback JSObject* of a smart ptr around a
 // CallbackObject or a reference to a CallbackObject or something like
 // that.
 template <class T, bool isSmartPtr=IsSmartPtr<T>::value>
 struct GetCallbackFromCallbackObjectHelper
 {
--- a/dom/bindings/CallbackFunction.h
+++ b/dom/bindings/CallbackFunction.h
@@ -35,19 +35,19 @@ public:
   // See CallbackObject for an explanation of the arguments.
   explicit CallbackFunction(JS::Handle<JSObject*> aCallable,
                             JS::Handle<JSObject*> aAsyncStack,
                             nsIGlobalObject* aIncumbentGlobal)
     : CallbackObject(aCallable, aAsyncStack, aIncumbentGlobal)
   {
   }
 
-  JS::Handle<JSObject*> Callable() const
+  JS::Handle<JSObject*> CallableOrNull() const
   {
-    return Callback();
+    return CallbackOrNull();
   }
 
   JS::Handle<JSObject*> CallablePreserveColor() const
   {
     return CallbackPreserveColor();
   }
 
   bool HasGrayCallable() const
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -95,18 +95,25 @@ CallbackObject::CallSetup::CallSetup(Cal
 
   // Compute the caller's subject principal (if necessary) early, before we
   // do anything that might perturb the relevant state.
   nsIPrincipal* webIDLCallerPrincipal = nullptr;
   if (aIsJSImplementedWebIDL) {
     webIDLCallerPrincipal = nsContentUtils::SubjectPrincipalOrSystemIfNativeCaller();
   }
 
+  JSObject* wrappedCallback = aCallback->CallbackPreserveColor();
+  if (!wrappedCallback) {
+    aRv.ThrowDOMException(NS_ERROR_DOM_NOT_SUPPORTED_ERR,
+      NS_LITERAL_CSTRING("Cannot execute callback from a nuked compartment."));
+    return;
+  }
+
   // First, find the real underlying callback.
-  JSObject* realCallback = js::UncheckedUnwrap(aCallback->CallbackPreserveColor());
+  JSObject* realCallback = js::UncheckedUnwrap(wrappedCallback);
   nsIGlobalObject* globalObject = nullptr;
 
   JSContext* cx;
   {
     // Bug 955660: we cannot do "proper" rooting here because we need the
     // global to get a context. Everything here is simple getters that cannot
     // GC, so just paper over the necessary dataflow inversion.
     JS::AutoSuppressGCAnalysis nogc;
@@ -159,23 +166,24 @@ CallbackObject::CallSetup::CallSetup(Cal
                            "incumbent global is being torn down."));
         return;
       }
       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.
+    // Unmark the callable (by invoking CallbackOrNull() 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 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());
+    mRootedCallable.emplace(cx, aCallback->CallbackOrNull());
   }
 
   // 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,
     // not the wrapper.
@@ -313,17 +321,17 @@ CallbackObjectHolderBase::ToXPCOMCallbac
   }
 
   // We don't init the AutoJSAPI with our callback because we don't want it
   // reporting errors to its global's onerror handlers.
   AutoJSAPI jsapi;
   jsapi.Init();
   JSContext* cx = jsapi.cx();
 
-  JS::Rooted<JSObject*> callback(cx, aCallback->Callback());
+  JS::Rooted<JSObject*> callback(cx, aCallback->CallbackOrNull());
 
   JSAutoCompartment ac(cx, callback);
   RefPtr<nsXPCWrappedJS> wrappedJS;
   nsresult rv =
     nsXPCWrappedJS::GetNewOrUsed(callback, aIID, getter_AddRefs(wrappedJS));
   if (NS_FAILED(rv) || !wrappedJS) {
     return nullptr;
   }
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -74,17 +74,25 @@ public:
   // for that purpose.
   explicit CallbackObject(JS::Handle<JSObject*> aCallback,
                           JS::Handle<JSObject*> aAsyncStack,
                           nsIGlobalObject* aIncumbentGlobal)
   {
     Init(aCallback, aAsyncStack, aIncumbentGlobal);
   }
 
-  JS::Handle<JSObject*> Callback() const
+  // This is guaranteed to be non-null from the time the CallbackObject is
+  // created until JavaScript has had a chance to run. It will only return null
+  // after a JavaScript caller has called nukeSandbox on a Sandbox object, and
+  // the cycle collector has had a chance to run.
+  //
+  // This means that any native callee which receives a CallbackObject as an
+  // argument can safely rely on the callback being non-null so long as it
+  // doesn't trigger any scripts before it accesses it.
+  JS::Handle<JSObject*> CallbackOrNull() const
   {
     mCallback.exposeToActiveJS();
     return CallbackPreserveColor();
   }
 
   JSObject* GetCreationStack() const
   {
     return mCreationStack;
@@ -108,17 +116,17 @@ public:
   {
     // Calling fromMarkedLocation() is safe because we trace our mCallback, and
     // because the value of mCallback cannot change after if has been set.
     return JS::Handle<JSObject*>::fromMarkedLocation(mCallback.address());
   }
 
   /*
    * If the callback is known to be non-gray, then this method can be
-   * used instead of Callback() to avoid the overhead of
+   * used instead of CallbackOrNull() to avoid the overhead of
    * ExposeObjectToActiveJS().
    */
   JS::Handle<JSObject*> CallbackKnownNotGray() const
   {
     MOZ_ASSERT(!JS::ObjectIsMarkedGray(mCallback));
     return CallbackPreserveColor();
   }
 
@@ -155,20 +163,24 @@ protected:
   explicit CallbackObject(CallbackObject* aCallbackObject)
   {
     Init(aCallbackObject->mCallback, aCallbackObject->mCreationStack,
          aCallbackObject->mIncumbentGlobal);
   }
 
   bool operator==(const CallbackObject& aOther) const
   {
-    JSObject* thisObj =
-      js::UncheckedUnwrap(CallbackPreserveColor());
-    JSObject* otherObj =
-      js::UncheckedUnwrap(aOther.CallbackPreserveColor());
+    JSObject* wrappedThis = CallbackPreserveColor();
+    JSObject* wrappedOther = aOther.CallbackPreserveColor();
+    if (!wrappedThis || !wrappedOther) {
+      return this == &aOther;
+    }
+
+    JSObject* thisObj = js::UncheckedUnwrap(wrappedThis);
+    JSObject* otherObj = js::UncheckedUnwrap(wrappedOther);
     return thisObj == otherObj;
   }
 
 private:
   inline void InitNoHold(JSObject* aCallback, JSObject* aCreationStack,
                          nsIGlobalObject* aIncumbentGlobal)
   {
     MOZ_ASSERT(aCallback && !mCallback);
@@ -557,20 +569,20 @@ public:
 
   // But nullptr can't use the above template, because it doesn't know which S
   // to select.  So we need a special overload for nullptr.
   void operator=(decltype(nullptr) arg)
   {
     this->get().operator=(arg);
   }
 
-  // Codegen relies on being able to do Callback() on us.
-  JS::Handle<JSObject*> Callback() const
+  // Codegen relies on being able to do CallbackOrNull() on us.
+  JS::Handle<JSObject*> CallbackOrNull() const
   {
-    return this->get()->Callback();
+    return this->get()->CallbackOrNull();
   }
 
   ~RootedCallback()
   {
     // Ensure that our callback starts holding on to its own JS objects as
     // needed.  We really do need to check that things are initialized even when
     // T is OwningNonNull, because we might be running before the OwningNonNull
     // ever got assigned to!
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4248,17 +4248,19 @@ class CastableObjectUnwrapper():
             self.substitution["codeOnFailure"] = fill(
                 """
                 // Be careful to not wrap random DOM objects here, even if
                 // they're wrapped in opaque security wrappers for some reason.
                 // XXXbz Wish we could check for a JS-implemented object
                 // that already has a content reflection...
                 if (!IsDOMObject(js::UncheckedUnwrap(${source}))) {
                   nsCOMPtr<nsIGlobalObject> contentGlobal;
-                  if (!GetContentGlobalForJSImplementedObject(cx, Callback(), getter_AddRefs(contentGlobal))) {
+                  JS::Handle<JSObject*> callback = CallbackOrNull();
+                  if (!callback ||
+                      !GetContentGlobalForJSImplementedObject(cx, callback, getter_AddRefs(contentGlobal))) {
                     $*{exceptionCode}
                   }
                   JS::Rooted<JSObject*> jsImplSourceObj(cx, ${source});
                   ${target} = new ${type}(jsImplSourceObj, contentGlobal);
                 } else {
                   $*{codeOnFailure}
                 }
                 """,
@@ -15116,21 +15118,21 @@ class CGJSImplClass(CGBindingImplClass):
         return fill(
             """
             JS::Rooted<JSObject*> obj(aCx, ${name}Binding::Wrap(aCx, this, aGivenProto));
             if (!obj) {
               return nullptr;
             }
 
             // Now define it on our chrome object
-            JSAutoCompartment ac(aCx, mImpl->Callback());
+            JSAutoCompartment ac(aCx, mImpl->CallbackOrNull());
             if (!JS_WrapObject(aCx, &obj)) {
               return nullptr;
             }
-            if (!JS_DefineProperty(aCx, mImpl->Callback(), "__DOM_IMPL__", obj, 0)) {
+            if (!JS_DefineProperty(aCx, mImpl->CallbackOrNull(), "__DOM_IMPL__", obj, 0)) {
               return nullptr;
             }
             return obj;
             """,
             name=self.descriptor.name)
 
     def getGetParentObjectReturnType(self):
         return "nsISupports*"
--- a/dom/bindings/ToJSValue.h
+++ b/dom/bindings/ToJSValue.h
@@ -127,17 +127,17 @@ ToJSValue(JSContext* aCx,
 MOZ_MUST_USE inline bool
 ToJSValue(JSContext* aCx,
           CallbackObject& aArgument,
           JS::MutableHandle<JS::Value> aValue)
 {
   // Make sure we're called in a compartment
   MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
 
-  aValue.setObject(*aArgument.Callback());
+  aValue.setObjectOrNull(aArgument.CallbackOrNull());
 
   return MaybeWrapValue(aCx, aValue);
 }
 
 // Accept objects that inherit from nsWrapperCache (e.g. most
 // DOM objects).
 template <class T>
 MOZ_MUST_USE
--- a/dom/console/Console.cpp
+++ b/dom/console/Console.cpp
@@ -2258,21 +2258,26 @@ Console::NotifyHandler(JSContext* aCx, c
   MOZ_ASSERT(aCallData);
 
   if (!mConsoleEventNotifier) {
     return;
   }
 
   JS::Rooted<JS::Value> value(aCx);
 
+  JS::Rooted<JSObject*> callable(aCx, mConsoleEventNotifier->CallableOrNull());
+  if (NS_WARN_IF(!callable)) {
+    return;
+  }
+
   // aCx and aArguments are in the same compartment because this method is
   // called directly when a Console.something() runs.
   // mConsoleEventNotifier->Callable() is the scope where value will be sent to.
   if (NS_WARN_IF(!PopulateConsoleNotificationInTheTargetScope(aCx, aArguments,
-                                                              mConsoleEventNotifier->Callable(),
+                                                              callable,
                                                               &value,
                                                               aCallData))) {
     return;
   }
 
   JS::Rooted<JS::Value> ignored(aCx);
   mConsoleEventNotifier->Call(value, &ignored);
 }
--- a/dom/events/DOMEventTargetHelper.cpp
+++ b/dom/events/DOMEventTargetHelper.cpp
@@ -316,17 +316,17 @@ DOMEventTargetHelper::SetEventHandler(ns
 
 void
 DOMEventTargetHelper::GetEventHandler(nsIAtom* aType,
                                       JSContext* aCx,
                                       JS::Value* aValue)
 {
   EventHandlerNonNull* handler = GetEventHandler(aType, EmptyString());
   if (handler) {
-    *aValue = JS::ObjectValue(*handler->Callable());
+    *aValue = JS::ObjectOrNullValue(handler->CallableOrNull());
   } else {
     *aValue = JS::NullValue();
   }
 }
 
 nsresult
 DOMEventTargetHelper::GetEventTargetParent(EventChainPreVisitor& aVisitor)
 {
--- a/dom/events/EventListenerService.cpp
+++ b/dom/events/EventListenerService.cpp
@@ -133,17 +133,17 @@ EventListenerInfo::GetJSVal(JSContext* a
     aAc.emplace(aCx, object);
     aJSVal.setObject(*object);
     return true;
   }
 
   nsCOMPtr<JSEventHandler> jsHandler = do_QueryInterface(mListener);
   if (jsHandler && jsHandler->GetTypedEventHandler().HasEventHandler()) {
     JS::Handle<JSObject*> handler =
-      jsHandler->GetTypedEventHandler().Ptr()->Callable();
+      jsHandler->GetTypedEventHandler().Ptr()->CallableOrNull();
     if (handler) {
       aAc.emplace(aCx, handler);
       aJSVal.setObject(*handler);
       return true;
     }
   }
   return false;
 }
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -196,26 +196,26 @@ Promise::Then(JSContext* aCx,
   JS::Rooted<JSObject*> promise(aCx, PromiseObj());
   if (!JS_WrapObject(aCx, &promise)) {
     aRv.NoteJSContextException(aCx);
     return;
   }
 
   JS::Rooted<JSObject*> resolveCallback(aCx);
   if (aResolveCallback) {
-    resolveCallback = aResolveCallback->Callback();
+    resolveCallback = aResolveCallback->CallbackOrNull();
     if (!JS_WrapObject(aCx, &resolveCallback)) {
       aRv.NoteJSContextException(aCx);
       return;
     }
   }
 
   JS::Rooted<JSObject*> rejectCallback(aCx);
   if (aRejectCallback) {
-    rejectCallback = aRejectCallback->Callback();
+    rejectCallback = aRejectCallback->CallbackOrNull();
     if (!JS_WrapObject(aCx, &rejectCallback)) {
       aRv.NoteJSContextException(aCx);
       return;
     }
   }
 
   JS::Rooted<JSObject*> retval(aCx);
   retval = JS::CallOriginalPromiseThen(aCx, promise, resolveCallback,
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -1230,17 +1230,17 @@ private:
   {
     // Silence bad assertions.
   }
 
   virtual bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
-    JS::Rooted<JS::Value> callable(aCx, JS::ObjectValue(*mHandler->Callable()));
+    JS::Rooted<JS::Value> callable(aCx, JS::ObjectOrNullValue(mHandler->CallableOrNull()));
     JS::HandleValueArray args = JS::HandleValueArray::empty();
     JS::Rooted<JS::Value> rval(aCx);
     if (!JS_CallFunctionValue(aCx, global, callable, args, &rval)) {
       // Just return false; WorkerRunnable::Run will report the exception.
       return false;
     }
 
     return true;
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -953,17 +953,18 @@ public:
   virtual ~PromiseJobRunnable()
   {
   }
 
 protected:
   NS_IMETHOD
   Run() override
   {
-    nsIGlobalObject* global = xpc::NativeGlobal(mCallback->CallbackPreserveColor());
+    JSObject* callback = mCallback->CallbackPreserveColor();
+    nsIGlobalObject* global = callback ? xpc::NativeGlobal(callback) : nullptr;
     if (global && !global->IsDying()) {
       mCallback->Call("promise callback");
     }
     return NS_OK;
   }
 
 private:
   RefPtr<PromiseJobCallback> mCallback;