From bfc23d53a8c7273e478198f8bcd3ebe80362c55d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 18 May 2016 12:23:35 -0400 Subject: [PATCH] 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 | 21 ++++++ dom/bindings/CallbackObject.h | 82 +++++++++++++++++++++- dom/bindings/Codegen.py | 117 ++++++++++++++++++++++++++++---- 3 files changed, 205 insertions(+), 15 deletions(-) diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp index e445475d9857..42dd2280695e 100644 --- a/dom/bindings/CallbackObject.cpp +++ b/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, diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h index 970e442b3f80..20b8cf5e4113 100644 --- a/dom/bindings/CallbackObject.h +++ b/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& 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 +class RootedCallback : public JS::Rooted +{ +public: + explicit RootedCallback(JSContext* cx) + : JS::Rooted(cx) + {} + + // We need a way to make assignment from pointers (how we're normally used) + // work. + template + 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 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 + static bool IsInitialized(U& aArg); // Not implemented + + template + static bool IsInitialized(RefPtr& aRefPtr) + { + return aRefPtr; + } + + template + static bool IsInitialized(OwningNonNull& aOwningNonNull) + { + return aOwningNonNull.isInitialized(); + } +}; + } // namespace dom } // namespace mozilla diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 2d0af83e249b..c452d3e141d5 100644 --- a/dom/bindings/Codegen.py +++ b/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 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", "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", "aCallback"), Argument("JS::Handle", "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", "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