Bug 1342070 - Part 1: Only create result Promises in Promise#then if it's used or the creation is otherwise observable. r=anba
authorTill Schneidereit <till@tillschneidereit.net>
Tue, 01 Aug 2017 16:37:37 +0200
changeset 414099 c2d33789539668305c1e8ac2d65a05a590c8a548
parent 414098 8ebe4719c2414dc32568d67a2889c48adb686031
child 414100 f054209125461575f57ce0f1e6b962653c523572
push id33858
push userncsoregi@mozilla.com
push dateTue, 17 Apr 2018 21:55:44 +0000
treeherdermozilla-central@d6eb5597d744 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1342070
milestone61.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 1342070 - Part 1: Only create result Promises in Promise#then if it's used or the creation is otherwise observable. r=anba "Otherwise observable" here means either the receiver's "constructor" property is changed, or the constructor's @@species property.
js/src/builtin/Promise.cpp
js/src/builtin/Promise.h
js/src/jsapi.cpp
js/src/vm/JSObject-inl.h
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -2456,37 +2456,41 @@ IsPromiseSpecies(JSContext* cx, JSFuncti
 {
     return species->maybeNative() == Promise_static_species;
 }
 
 // ES2016, 25.4.5.3., steps 3-5.
 MOZ_MUST_USE bool
 js::OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise,
                         HandleValue onFulfilled, HandleValue onRejected,
-                        MutableHandleObject dependent, bool createDependent)
+                        MutableHandleObject dependent, CreateDependentPromise createDependent)
 {
     RootedObject promiseObj(cx, promise);
     if (promise->compartment() != cx->compartment()) {
         if (!cx->compartment()->wrap(cx, &promiseObj))
             return false;
     }
 
     RootedObject resultPromise(cx);
     RootedObject resolve(cx);
     RootedObject reject(cx);
 
-    if (createDependent) {
+    if (createDependent != CreateDependentPromise::Never) {
         // Step 3.
         RootedObject C(cx, SpeciesConstructor(cx, promiseObj, JSProto_Promise, IsPromiseSpecies));
         if (!C)
             return false;
 
-        // Step 4.
-        if (!NewPromiseCapability(cx, C, &resultPromise, &resolve, &reject, true))
-            return false;
+        if (createDependent == CreateDependentPromise::Always ||
+            !IsNativeFunction(C, PromiseConstructor))
+        {
+            // Step 4.
+            if (!NewPromiseCapability(cx, C, &resultPromise, &resolve, &reject, true))
+                return false;
+        }
     }
 
     // Step 5.
     if (!PerformPromiseThen(cx, promise, onFulfilled, onRejected, resultPromise, resolve, reject))
         return false;
 
     dependent.set(resultPromise);
     return true;
@@ -3019,19 +3023,17 @@ js::AsyncGeneratorEnqueue(JSContext* cx,
             return false;
     }
 
     // Step 9.
     result.setObject(*resultPromise);
     return true;
 }
 
-// ES2016, 25.4.5.3.
-bool
-js::Promise_then(JSContext* cx, unsigned argc, Value* vp)
+bool Promise_then_impl(JSContext* cx, unsigned argc, Value* vp, bool rvalUsed)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     // Step 1.
     RootedValue promiseVal(cx, args.thisv());
 
     RootedValue onFulfilled(cx, args.get(0));
     RootedValue onRejected(cx, args.get(1));
@@ -3058,24 +3060,47 @@ js::Promise_then(JSContext* cx, unsigned
             JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INCOMPATIBLE_PROTO,
                                       "Promise", "then", "value");
             return false;
         }
         promise = &unwrappedPromiseObj->as<PromiseObject>();
     }
 
     // Steps 3-5.
+    CreateDependentPromise createDependent = rvalUsed
+                                             ? CreateDependentPromise::Always
+                                             : CreateDependentPromise::SkipIfCtorUnobservable;
     RootedObject resultPromise(cx);
-    if (!OriginalPromiseThen(cx, promise, onFulfilled, onRejected, &resultPromise, true))
+    if (!OriginalPromiseThen(cx, promise, onFulfilled, onRejected, &resultPromise,
+                             createDependent))
+    {
         return false;
-
-    args.rval().setObject(*resultPromise);
+    }
+
+    if (rvalUsed)
+        args.rval().setObject(*resultPromise);
+    else
+        args.rval().setUndefined();
     return true;
 }
 
+// ES2016, 25.4.5.3.
+bool
+Promise_then_noRetVal(JSContext* cx, unsigned argc, Value* vp)
+{
+    return Promise_then_impl(cx, argc, vp, false);
+}
+
+// ES2016, 25.4.5.3.
+bool
+js::Promise_then(JSContext* cx, unsigned argc, Value* vp)
+{
+    return Promise_then_impl(cx, argc, vp, true);
+}
+
 // ES2016, 25.4.5.3.1.
 static MOZ_MUST_USE bool
 PerformPromiseThen(JSContext* cx, Handle<PromiseObject*> promise, HandleValue onFulfilled_,
                    HandleValue onRejected_, HandleObject resultPromise,
                    HandleObject resolve, HandleObject reject)
 {
     // Step 1 (implicit).
     // Step 2 (implicit).
@@ -3732,19 +3757,28 @@ OffThreadPromiseRuntimeState::shutdown(J
 }
 
 static JSObject*
 CreatePromisePrototype(JSContext* cx, JSProtoKey key)
 {
     return GlobalObject::createBlankPrototype(cx, cx->global(), &PromiseObject::protoClass_);
 }
 
+const JSJitInfo promise_then_info = {
+  { (JSJitGetterOp)Promise_then_noRetVal },
+  { 0 }, /* unused */
+  { 0 }, /* unused */
+  JSJitInfo::IgnoresReturnValueNative,
+  JSJitInfo::AliasEverything,
+  JSVAL_TYPE_UNDEFINED,
+};
+
 static const JSFunctionSpec promise_methods[] = {
     JS_SELF_HOSTED_FN("catch", "Promise_catch", 1, 0),
-    JS_FN("then", Promise_then, 2, 0),
+    JS_FNINFO("then", Promise_then, &promise_then_info, 2, 0),
     JS_SELF_HOSTED_FN("finally", "Promise_finally", 1, 0),
     JS_FS_END
 };
 
 static const JSPropertySpec promise_properties[] = {
     JS_STRING_SYM_PS(toStringTag, "Promise", JSPROP_READONLY),
     JS_PS_END
 };
--- a/js/src/builtin/Promise.h
+++ b/js/src/builtin/Promise.h
@@ -98,30 +98,36 @@ class PromiseObject : public NativeObjec
  * promise.
  *
  * Asserts that all objects in the `promises` vector are, maybe wrapped,
  * instances of `Promise` or a subclass of `Promise`.
  */
 MOZ_MUST_USE JSObject*
 GetWaitForAllPromise(JSContext* cx, const JS::AutoObjectVector& promises);
 
+enum class CreateDependentPromise {
+    Always,
+    SkipIfCtorUnobservable,
+    Never
+};
+
 /**
  * Enqueues resolve/reject reactions in the given Promise's reactions lists
  * as though calling the original value of Promise.prototype.then.
  *
  * If the `createDependent` flag is not set, no dependent Promise will be
  * created. This is used internally to implement DOM functionality.
  * Note: In this case, the reactions pushed using this function contain a
  * `promise` field that can contain null. That field is only ever used by
  * devtools, which have to treat these reactions specially.
  */
 MOZ_MUST_USE bool
 OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise,
                     HandleValue onFulfilled, HandleValue onRejected,
-                    MutableHandleObject dependent, bool createDependent);
+                    MutableHandleObject dependent, CreateDependentPromise createDependent);
 
 /**
  * PromiseResolve ( C, x )
  *
  * The abstract operation PromiseResolve, given a constructor and a value,
  * returns a new promise resolved with that value.
  */
 MOZ_MUST_USE JSObject*
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -5214,17 +5214,18 @@ JS_PUBLIC_API(bool)
 JS::RejectPromise(JSContext* cx, JS::HandleObject promiseObj, JS::HandleValue rejectionValue)
 {
     return ResolveOrRejectPromise(cx, promiseObj, rejectionValue, true);
 }
 
 static bool
 CallOriginalPromiseThenImpl(JSContext* cx, JS::HandleObject promiseObj,
                             JS::HandleObject onResolvedObj_, JS::HandleObject onRejectedObj_,
-                            JS::MutableHandleObject resultObj, bool createDependent)
+                            JS::MutableHandleObject resultObj,
+                            CreateDependentPromise createDependent)
 {
     AssertHeapIsIdle();
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, promiseObj, onResolvedObj_, onRejectedObj_);
 
     MOZ_ASSERT_IF(onResolvedObj_, IsCallable(onResolvedObj_));
     MOZ_ASSERT_IF(onRejectedObj_, IsCallable(onRejectedObj_));
 
@@ -5263,27 +5264,31 @@ CallOriginalPromiseThenImpl(JSContext* c
     return true;
 }
 
 JS_PUBLIC_API(JSObject*)
 JS::CallOriginalPromiseThen(JSContext* cx, JS::HandleObject promiseObj,
                             JS::HandleObject onResolvedObj, JS::HandleObject onRejectedObj)
 {
     RootedObject resultPromise(cx);
-    if (!CallOriginalPromiseThenImpl(cx, promiseObj, onResolvedObj, onRejectedObj, &resultPromise, true))
+    if (!CallOriginalPromiseThenImpl(cx, promiseObj, onResolvedObj, onRejectedObj, &resultPromise,
+                                     CreateDependentPromise::Always))
+    {
         return nullptr;
+    }
     return resultPromise;
 }
 
 JS_PUBLIC_API(bool)
 JS::AddPromiseReactions(JSContext* cx, JS::HandleObject promiseObj,
                         JS::HandleObject onResolvedObj, JS::HandleObject onRejectedObj)
 {
     RootedObject resultPromise(cx);
-    bool result = CallOriginalPromiseThenImpl(cx, promiseObj, onResolvedObj, onRejectedObj, &resultPromise, false);
+    bool result = CallOriginalPromiseThenImpl(cx, promiseObj, onResolvedObj, onRejectedObj,
+                                              &resultPromise, CreateDependentPromise::Never);
     MOZ_ASSERT(!resultPromise);
     return result;
 }
 
 /**
  * Unforgeable version of Promise.all for internal use.
  *
  * Takes a dense array of Promise objects and returns a promise that's
--- a/js/src/vm/JSObject-inl.h
+++ b/js/src/vm/JSObject-inl.h
@@ -525,16 +525,21 @@ IsNativeFunction(const js::Value& v, JSF
 
 static MOZ_ALWAYS_INLINE bool
 IsNativeFunction(const js::Value& v, JSNative native)
 {
     JSFunction* fun;
     return IsFunctionObject(v, &fun) && fun->maybeNative() == native;
 }
 
+static MOZ_ALWAYS_INLINE bool
+IsNativeFunction(const JSObject* obj, JSNative native)
+{
+    return obj->is<JSFunction>() && obj->as<JSFunction>().maybeNative() == native;
+}
 
 // Return whether looking up a method on 'obj' definitely resolves to the
 // original specified native function. The method may conservatively return
 // 'false' in the case of proxies or other non-native objects.
 static MOZ_ALWAYS_INLINE bool
 HasNativeMethodPure(JSObject* obj, PropertyName* name, JSNative native, JSContext* cx)
 {
     Value v;