Bug 1273661 part 3. Add the codegen bits and Trace implementation to allow Web IDL callbacks to not have to HoldJSObjects until the bindings have determined that someone is actually holding on to the callback object. r=smaug,terrence

dom/bindings/CallbackObject.cpp

@@ -47,6 +47,27 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(CallbackObject)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mIncumbentJSGlobal)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
+void
+CallbackObject::Trace(JSTracer* aTracer)
+{
+  JS::TraceEdge(aTracer, &mCallback, "CallbackObject.mCallback");
+  JS::TraceEdge(aTracer, &mCreationStack, "CallbackObject.mCreationStack");
+  JS::TraceEdge(aTracer, &mIncumbentJSGlobal,
+                "CallbackObject.mIncumbentJSGlobal");
+}
+
+void
+CallbackObject::HoldJSObjectsIfMoreThanOneOwner()
+{
+  MOZ_ASSERT(mRefCnt.get() > 0);
+  if (mRefCnt.get() > 1) {
+    mozilla::HoldJSObjects(this);
+  } else {
+    // We can just forget all our stuff.
+    ClearJSReferences();
+  }
+}
+
 CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
                                      ErrorResult& aRv,
                                      const char* aExecutionReason,

dom/bindings/CallbackObject.h

@@ -25,11 +25,13 @@
 #include "mozilla/ErrorResult.h"
 #include "mozilla/HoldDropJSObjects.h"
 #include "mozilla/MemoryReporting.h"
+#include "mozilla/OwningNonNull.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "nsWrapperCache.h"
 #include "nsJSEnvironment.h"
 #include "xpcpublic.h"
 #include "jsapi.h"
+#include "js/TracingAPI.h"
 
 namespace mozilla {
 namespace dom {
@@ -184,6 +186,13 @@ private:
     mozilla::HoldJSObjects(this);
   }
 
+  inline void ClearJSReferences()
+  {
+    mCallback = nullptr;
+    mCreationStack = nullptr;
+    mIncumbentJSGlobal = nullptr;
+  }
+
   CallbackObject(const CallbackObject&) = delete;
   CallbackObject& operator =(const CallbackObject&) = delete;
 
@@ -192,13 +201,19 @@ protected:
   {
     MOZ_ASSERT_IF(mIncumbentJSGlobal, mCallback);
     if (mCallback) {
-      mCallback = nullptr;
-      mCreationStack = nullptr;
-      mIncumbentJSGlobal = nullptr;
+      ClearJSReferences();
       mozilla::DropJSObjects(this);
     }
   }
 
+  // For use from subclasses that want to be usable with Rooted.
+  void Trace(JSTracer* aTracer);
+
+  // For use from subclasses that want to be traced for a bit then possibly
+  // switch to HoldJSObjects.  If we have more than one owner, this will
+  // HoldJSObjects; otherwise it will just forget all our JS references.
+  void HoldJSObjectsIfMoreThanOneOwner();
+
   // Struct used as a way to force a CallbackObject constructor to not call
   // HoldJSObjects. We're putting it here so that CallbackObject subclasses will
   // have access to it, but outside code will not.
@@ -506,6 +521,67 @@ ImplCycleCollectionUnlink(CallbackObjectHolder<T, U>& aField)
   aField.UnlinkSelf();
 }
 
+// T is expected to be a RefPtr or OwningNonNull around a CallbackObject
+// subclass.  This class is used in bindings to safely handle Fast* callbacks;
+// it ensures that the callback is traced, and that if something is holding onto
+// the callback when we're done with it HoldJSObjects is called.
+template<typename T>
+class RootedCallback : public JS::Rooted<T>
+{
+public:
+  explicit RootedCallback(JSContext* cx)
+    : JS::Rooted<T>(cx)
+  {}
+
+  // We need a way to make assignment from pointers (how we're normally used)
+  // work.
+  template<typename S>
+  void operator=(S* arg)
+  {
+    this->get().operator=(arg);
+  }
+
+  // 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
+  {
+    return this->get()->Callback();
+  }
+
+  ~RootedCallback()
+  {
+    // Ensure that our callback starts holding on to its own JS objects as
+    // needed.  Having to null-check here when T is OwningNonNull is a bit
+    // silly, but it's simpler than creating two separate RootedCallback
+    // instantiations for OwningNonNull and RefPtr.
+    if (IsInitialized(this->get())) {
+      this->get()->HoldJSObjectsIfMoreThanOneOwner();
+    }
+  }
+
+private:
+  template<typename U>
+  static bool IsInitialized(U& aArg); // Not implemented
+
+  template<typename U>
+  static bool IsInitialized(RefPtr<U>& aRefPtr)
+  {
+    return aRefPtr;
+  }
+
+  template<typename U>
+  static bool IsInitialized(OwningNonNull<U>& aOwningNonNull)
+  {
+    return aOwningNonNull.isInitialized();
+  }
+};
+
 } // namespace dom
 } // namespace mozilla
 

dom/bindings/Codegen.py

@@ -4186,6 +4186,37 @@ class CGCallbackTempRoot(CGGeneric):
         CGGeneric.__init__(self, define=define)
 
 
+def getCallbackConversionInfo(type, idlObject, isMember, isCallbackReturnValue,
+                              isOptional):
+    """
+    Returns a tuple containing the declType, declArgs, and basic
+    conversion for the given callback type, with the given callback
+    idl object in the given context (isMember/isCallbackReturnValue/isOptional).
+    """
+    name = idlObject.identifier.name
+
+    # We can't use fast callbacks if isOptional because then we get an
+    # Optional<RootedCallback> thing, which is not transparent to consumers.
+    useFastCallback = (not isMember and not isCallbackReturnValue and
+                       not isOptional)
+    if useFastCallback:
+        name = "binding_detail::Fast%s" % name
+
+    if type.nullable() or isCallbackReturnValue:
+        declType = CGGeneric("RefPtr<%s>" % name)
+    else:
+        declType = CGGeneric("OwningNonNull<%s>" % name)
+
+    if useFastCallback:
+        declType = CGTemplatedType("RootedCallback", declType)
+        declArgs = "cx"
+    else:
+        declArgs = None
+
+    conversion = indent(CGCallbackTempRoot(name).define())
+    return (declType, declArgs, conversion)
+
+
 class JSToNativeConversionInfo():
     """
     An object representing information about a JS-to-native conversion.
@@ -5088,17 +5119,16 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
         assert descriptor.nativeType != 'JSObject'
 
         if descriptor.interface.isCallback():
-            name = descriptor.interface.identifier.name
-            if type.nullable() or isCallbackReturnValue:
-                declType = CGGeneric("RefPtr<%s>" % name)
-            else:
-                declType = CGGeneric("OwningNonNull<%s>" % name)
-            conversion = indent(CGCallbackTempRoot(name).define())
-
+            (declType, declArgs,
+             conversion) = getCallbackConversionInfo(type, descriptor.interface,
+                                                     isMember,
+                                                     isCallbackReturnValue,
+                                                     isOptional)
             template = wrapObjectTemplate(conversion, type,
                                           "${declName} = nullptr;\n",
                                           failureCode)
             return JSToNativeConversionInfo(template, declType=declType,
+                                            declArgs=declArgs,
                                             dealWithOptional=isOptional)
 
         # This is an interface that we implement as a concrete class
@@ -5578,11 +5608,10 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
 
         callback = type.unroll().callback
         name = callback.identifier.name
-        if type.nullable():
-            declType = CGGeneric("RefPtr<%s>" % name)
-        else:
-            declType = CGGeneric("OwningNonNull<%s>" % name)
-        conversion = indent(CGCallbackTempRoot(name).define())
+        (declType, declArgs,
+         conversion) = getCallbackConversionInfo(type, callback, isMember,
+                                                 isCallbackReturnValue,
+                                                 isOptional)
 
         if allowTreatNonCallableAsNull and type.treatNonCallableAsNull():
             haveCallable = "JS::IsCallable(&${val}.toObject())"
@@ -5619,6 +5648,7 @@ def getJSToNativeConversionInfo(type, descriptorProvider, failureCode=None,
                 "${declName} = nullptr;\n",
                 failureCode)
         return JSToNativeConversionInfo(template, declType=declType,
+                                        declArgs=declArgs,
                                         dealWithOptional=isOptional)
 
     if type.isAny():
@@ -13536,9 +13566,15 @@ class CGBindingRoot(CGThing):
         cgthings.extend(CGCallbackFunction(c, config.getDescriptorProvider(False))
                         for c in mainCallbacks)
 
+        cgthings.extend([CGNamespace('binding_detail', CGFastCallback(c))
+                         for c in mainCallbacks])
+
         cgthings.extend(CGCallbackFunction(c, config.getDescriptorProvider(True))
                         for c in workerCallbacks if c not in mainCallbacks)
 
+        cgthings.extend([CGNamespace('binding_detail', CGFastCallback(c))
+                         for c in workerCallbacks if c not in mainCallbacks])
+
         # Do codegen for all the descriptors
         cgthings.extend([CGDescriptor(x) for x in descriptors])
 
@@ -13546,6 +13582,10 @@ class CGBindingRoot(CGThing):
         cgthings.extend([CGCallbackInterface(x) for x in callbackDescriptors if
                          not x.workers])
 
+        cgthings.extend([CGNamespace('binding_detail',
+                                     CGFastCallback(x.interface))
+                         for x in callbackDescriptors if not x.workers])
+
         # Do codegen for JS implemented classes
         def getParentDescriptor(desc):
             if not desc.interface.parent:
@@ -14819,6 +14859,18 @@ class CGCallback(CGClass):
                     "%s(aCx, aCallback, aIncumbentGlobal)" % self.baseName,
                 ],
                 body=body),
+            ClassConstructor(
+                [Argument("JSContext*", "aCx"),
+                 Argument("JS::Handle<JSObject*>", "aCallback"),
+                 Argument("nsIGlobalObject*", "aIncumbentGlobal"),
+                 Argument("const FastCallbackConstructor&", "")],
+                bodyInHeader=True,
+                visibility="public",
+                explicit=True,
+                baseConstructors=[
+                    "%s(aCx, aCallback, aIncumbentGlobal, FastCallbackConstructor())" % self.baseName,
+                ],
+                body=body),
             ClassConstructor(
                 [Argument("JS::Handle<JSObject*>", "aCallback"),
                  Argument("JS::Handle<JSObject*>", "aAsyncStack"),
@@ -14984,6 +15036,47 @@ class CGCallbackFunction(CGCallback):
                 baseConstructors=["CallbackFunction(aOther)"])]
 
 
+class CGFastCallback(CGClass):
+    def __init__(self, idlObject):
+        self._deps = idlObject.getDeps()
+        baseName = idlObject.identifier.name
+        constructor = ClassConstructor(
+            [Argument("JSContext*", "aCx"),
+             Argument("JS::Handle<JSObject*>", "aCallback"),
+             Argument("nsIGlobalObject*", "aIncumbentGlobal")],
+            bodyInHeader=True,
+            visibility="public",
+            explicit=True,
+            baseConstructors=[
+                "%s(aCx, aCallback, aIncumbentGlobal, FastCallbackConstructor())" %
+                baseName,
+            ],
+            body="")
+
+        traceMethod = ClassMethod("Trace", "void",
+                                  [Argument("JSTracer*", "aTracer")],
+                                  inline=True,
+                                  bodyInHeader=True,
+                                  visibility="public",
+                                  body="%s::Trace(aTracer);\n" % baseName)
+        holdMethod = ClassMethod("HoldJSObjectsIfMoreThanOneOwner", "void",
+                                 [],
+                                 inline=True,
+                                 bodyInHeader=True,
+                                 visibility="public",
+                                 body=(
+                                     "%s::HoldJSObjectsIfMoreThanOneOwner();\n" %
+                                     baseName))
+
+        CGClass.__init__(self, "Fast%s" % baseName,
+                         bases=[ClassBase(baseName)],
+                         constructors=[constructor],
+                         methods=[traceMethod, holdMethod])
+
+    def deps(self):
+        return self._deps
+
+
 class CGCallbackInterface(CGCallback):
     def __init__(self, descriptor, typedArraysAreStructs=False):
         iface = descriptor.interface