From bc6a0c8f1ca45a127822de3471ed2dbaa5a97c99 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 25 Nov 2015 15:48:09 -0500 Subject: [PATCH] Bug 1170760 part 12. Rip out the promise-resolved-with-promise fast path. r=baku --- dom/bindings/Codegen.py | 22 ---------- dom/bindings/parser/WebIDL.py | 1 - dom/promise/Promise.cpp | 80 ++++++----------------------------- dom/webidl/Promise.webidl | 2 +- 4 files changed, 15 insertions(+), 90 deletions(-) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index e92167f7fa74..28852ab88840 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -9149,26 +9149,6 @@ class CGStaticMethodJitinfo(CGGeneric): IDLToCIdentifier(method.identifier.name)))) -class CGMethodIdentityTest(CGAbstractMethod): - """ - A class to generate a method-identity test for a given IDL operation. - """ - def __init__(self, descriptor, method): - self.method = method - name = "Is%sMethod" % MakeNativeName(method.identifier.name) - CGAbstractMethod.__init__(self, descriptor, name, 'bool', - [Argument('JS::Handle', 'aObj')]) - - def definition_body(self): - return dedent( - """ - MOZ_ASSERT(aObj); - return js::IsFunctionObject(aObj) && - js::FunctionObjectIsNative(aObj) && - FUNCTION_VALUE_TO_JITINFO(JS::ObjectValue(*aObj)) == &%s_methodinfo; - """ % IDLToCIdentifier(self.method.identifier.name)) - - def getEnumValueName(value): # Some enum values can be empty strings. Others might have weird # characters in them. Deal with the former by returning "_empty", @@ -11800,8 +11780,6 @@ class CGDescriptor(CGThing): cgThings.append(CGMemberJITInfo(descriptor, m)) if props.isCrossOriginMethod: crossOriginMethods.add(m.identifier.name) - if m.getExtendedAttribute("MethodIdentityTestable"): - cgThings.append(CGMethodIdentityTest(descriptor, m)) # If we've hit the maplike/setlike member itself, go ahead and # generate its convenience functions. elif m.isMaplikeOrSetlike(): diff --git a/dom/bindings/parser/WebIDL.py b/dom/bindings/parser/WebIDL.py index 0f3132cde696..a930242b8d30 100644 --- a/dom/bindings/parser/WebIDL.py +++ b/dom/bindings/parser/WebIDL.py @@ -4805,7 +4805,6 @@ class IDLMethod(IDLInterfaceMember, IDLScope): identifier == "CheckAnyPermissions" or identifier == "CheckAllPermissions" or identifier == "BinaryName" or - identifier == "MethodIdentityTestable" or identifier == "StaticClassOverride"): # Known attributes that we don't need to do anything with here pass diff --git a/dom/promise/Promise.cpp b/dom/promise/Promise.cpp index 6b199af795e0..d2b3313ac36c 100644 --- a/dom/promise/Promise.cpp +++ b/dom/promise/Promise.cpp @@ -278,48 +278,6 @@ private: NS_DECL_OWNINGTHREAD; }; -// Fast version of PromiseResolveThenableJob for use in the cases when we know we're -// calling the canonical Promise.prototype.then on an actual DOM Promise. In -// that case we can just bypass the jumping into and out of JS and call -// AppendCallbacks on that promise directly. -class FastPromiseResolveThenableJob final : public nsRunnable -{ -public: - FastPromiseResolveThenableJob(PromiseCallback* aResolveCallback, - PromiseCallback* aRejectCallback, - Promise* aNextPromise) - : mResolveCallback(aResolveCallback) - , mRejectCallback(aRejectCallback) - , mNextPromise(aNextPromise) - { - MOZ_ASSERT(aResolveCallback); - MOZ_ASSERT(aRejectCallback); - MOZ_ASSERT(aNextPromise); - MOZ_COUNT_CTOR(FastPromiseResolveThenableJob); - } - - virtual - ~FastPromiseResolveThenableJob() - { - NS_ASSERT_OWNINGTHREAD(FastPromiseResolveThenableJob); - MOZ_COUNT_DTOR(FastPromiseResolveThenableJob); - } - -protected: - NS_IMETHOD - Run() override - { - NS_ASSERT_OWNINGTHREAD(FastPromiseResolveThenableJob); - mNextPromise->AppendCallbacks(mResolveCallback, mRejectCallback); - return NS_OK; - } - -private: - RefPtr mResolveCallback; - RefPtr mRejectCallback; - RefPtr mNextPromise; -}; - // A struct implementing // . // While the spec holds on to these in some places, in practice those places @@ -2018,35 +1976,25 @@ Promise::ResolveInternal(JSContext* aCx, // This is the then() function of the thenable aValueObj. JS::Rooted thenObj(aCx, &then.toObject()); - // Add a fast path for the case when we're resolved with an actual - // Promise. This has two requirements: + // We used to have a fast path here for the case when the following + // requirements held: // // 1) valueObj is a Promise. // 2) thenObj is a JSFunction backed by our actual Promise::Then // implementation. // - // If those requirements are satisfied, then we know exactly what - // thenObj.call(valueObj) will do, so we can optimize a bit and avoid ever - // entering JS for this stuff. - Promise* nextPromise; - if (PromiseBinding::IsThenMethod(thenObj) && - NS_SUCCEEDED(UNWRAP_OBJECT(Promise, valueObj, nextPromise))) { - // If we were taking the codepath that involves PromiseResolveThenableJob and - // PromiseInit below, then eventually, in PromiseResolveThenableJob::Run, we - // would create some JSFunctions in the compartment of - // this->GetWrapper() and pass them to the PromiseInit. So by the time - // we'd see the resolution value it would be wrapped into the - // compartment of this->GetWrapper(). The global of that compartment is - // this->GetGlobalJSObject(), so use that as the global for - // ResolvePromiseCallback/RejectPromiseCallback. - JS::Rooted glob(aCx, GlobalJSObject()); - RefPtr resolveCb = new ResolvePromiseCallback(this, glob); - RefPtr rejectCb = new RejectPromiseCallback(this, glob); - RefPtr task = - new FastPromiseResolveThenableJob(resolveCb, rejectCb, nextPromise); - DispatchToMicroTask(task); - return; - } + // But now that we're doing subclassing in Promise.prototype.then we would + // also need the following requirements: + // + // 3) Getting valueObj.constructor has no side-effects. + // 4) Getting valueObj.constructor[@@species] has no side-effects. + // 5) valueObj.constructor[@@species] is a function and calling it has no + // side-effects (e.g. it's the canonical Promise constructor) and it + // provides some callback functions to call as arguments to its + // argument. + // + // Ensuring that stuff while not inside SpiderMonkey is painful, so let's + // drop the fast path for now. RefPtr thenCallback = new PromiseInit(nullptr, thenObj, mozilla::dom::GetIncumbentGlobal()); diff --git a/dom/webidl/Promise.webidl b/dom/webidl/Promise.webidl index b14d9c3a33df..ac5d9f86e34c 100644 --- a/dom/webidl/Promise.webidl +++ b/dom/webidl/Promise.webidl @@ -30,7 +30,7 @@ interface _Promise { // The [TreatNonCallableAsNull] annotation is required since then() should do // nothing instead of throwing errors when non-callable arguments are passed. - [NewObject, MethodIdentityTestable] + [NewObject] Promise then([TreatNonCallableAsNull] optional AnyCallback? fulfillCallback = null, [TreatNonCallableAsNull] optional AnyCallback? rejectCallback = null);