Issue #2089 - Only create result Promises in Promise#then and Promise#catch if it's used or the creation is otherwise observable

Based-on: m-c 1342070

js/src/builtin/Promise.cpp

@@ -2588,8 +2588,8 @@ js::PromiseResolve(JSContext* cx, HandleObject constructor, HandleValue value)
 /**
  * ES2016, 25.4.4.4, Promise.reject.
  */
-bool
+static bool
-js::Promise_reject(JSContext* cx, unsigned argc, Value* vp)
+Promise_reject(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedValue thisVal(cx, args.thisv());
@@ -2617,8 +2617,8 @@ PromiseObject::unforgeableReject(JSContext* cx, HandleValue value)
 /**
  * ES2016, 25.4.4.5, Promise.resolve.
  */
-bool
+static bool
-js::Promise_static_resolve(JSContext* cx, unsigned argc, Value* vp)
+Promise_static_resolve(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedValue thisVal(cx, args.thisv());
@@ -2705,7 +2705,7 @@ IsPromiseSpecies(JSContext* cx, JSFunction* species)
 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()) {
@@ -2717,16 +2717,21 @@ js::OriginalPromiseThen(JSContext* cx, Handle<PromiseObject*> promise,
     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 (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))
@@ -3225,18 +3230,77 @@ js::AsyncGeneratorEnqueue(JSContext* cx, HandleValue asyncGenVal,
     return true;
 }
 
-// ES2016, 25.4.5.3.
+static bool Promise_then(JSContext* cx, unsigned argc, Value* vp);
-bool
+static bool Promise_then_impl(JSContext* cx, HandleValue promiseVal, HandleValue onFulfilled,
-js::Promise_then(JSContext* cx, unsigned argc, Value* vp)
+                              HandleValue onRejected, MutableHandleValue rval, bool rvalUsed);
+
+static bool
+Promise_catch_impl(JSContext* cx, unsigned argc, Value* vp, bool rvalUsed)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     // Step 1.
-    RootedValue promiseVal(cx, args.thisv());
+    RootedValue thenVal(cx);
+    if (!GetProperty(cx, args.thisv(), cx->names().then, &thenVal))
+        return false;
 
-    RootedValue onFulfilled(cx, args.get(0));
+    if (IsNativeFunction(thenVal, &Promise_then)) {
-    RootedValue onRejected(cx, args.get(1));
+        return Promise_then_impl(cx, args.thisv(), UndefinedHandleValue, args.get(0),
+                                 args.rval(), rvalUsed);
+    }
 
+    FixedInvokeArgs<2> iargs(cx);
+    iargs[0].setUndefined();
+    iargs[1].set(args.get(0));
+
+    return Call(cx, thenVal, args.thisv(), iargs, args.rval());
+}
+
+static MOZ_ALWAYS_INLINE bool
+IsPromiseThenOrCatchRetValImplicitlyUsed(JSContext* cx)
+{
+    // The returned promise of Promise#then and Promise#catch contains
+    // stack info if async stack is enabled.  Even if their return value is not
+    // used explicitly in the script, the stack info is observable in devtools
+    // and profilers.  We shouldn't apply the optimization not to allocate the
+    // returned Promise object if the it's implicitly used by them.
+    //
+    // FIXME: Once bug 1280819 gets fixed, we can use ShouldCaptureDebugInfo.
+    if (!cx->options().asyncStack())
+        return false;
+
+    // If devtools is opened, the current compartment will become debuggee.
+    if (cx->compartment()->isDebuggee())
+        return true;
+
+    // The profiler also makes the call stack observable
+    if (JS::IsProfileTimelineRecordingEnabled())
+        return true;
+
+    // The stack is also observable from Error#stack, but we don't care since
+    // it's nonstandard feature.
+    return false;
+}
+
+// ES2016, 25.4.5.3.
+static bool
+Promise_catch_noRetVal(JSContext* cx, unsigned argc, Value* vp)
+{
+    return Promise_catch_impl(cx, argc, vp, IsPromiseThenOrCatchRetValImplicitlyUsed(cx));
+}
+
+// ES2016, 25.4.5.3.
+static bool
+Promise_catch(JSContext* cx, unsigned argc, Value* vp)
+{
+    return Promise_catch_impl(cx, argc, vp, true);
+}
+
+static bool
+Promise_then_impl(JSContext* cx, HandleValue promiseVal, HandleValue onFulfilled,
+                  HandleValue onRejected, MutableHandleValue rval, bool rvalUsed)
+{
+    // Step 1 (implicit).
     // Step 2.
     if (!promiseVal.isObject()) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_NOT_NONNULL_OBJECT,
@@ -3264,14 +3328,40 @@ js::Promise_then(JSContext* cx, unsigned argc, Value* vp)
     }
 
     // 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)
+        rval.setObject(*resultPromise);
+    else
+        rval.setUndefined();
     return true;
 }
 
+// ES2016, 25.4.5.3.
+bool
+Promise_then_noRetVal(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+    return Promise_then_impl(cx, args.thisv(), args.get(0), args.get(1), args.rval(),
+                             IsPromiseThenOrCatchRetValImplicitlyUsed(cx));
+}
+
+// ES2016, 25.4.5.3.
+static bool
+Promise_then(JSContext* cx, unsigned argc, Value* vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+    return Promise_then_impl(cx, args.thisv(), args.get(0), args.get(1), args.rval(), true);
+}
+
 // ES2016, 25.4.5.3.1.
 static MOZ_MUST_USE bool
 PerformPromiseThen(JSContext* cx, Handle<PromiseObject*> promise, HandleValue onFulfilled_,
@@ -3723,9 +3813,27 @@ 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,
+};
+
+const JSJitInfo promise_catch_info = {
+  { (JSJitGetterOp)Promise_catch_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_FNINFO("then", Promise_then, &promise_then_info, 2, 0),
-    JS_FN("then", Promise_then, 2, 0),
+    JS_FNINFO("catch", Promise_catch, &promise_catch_info, 1, 0),
     JS_SELF_HOSTED_FN("finally", "Promise_finally", 1, 0),
     JS_FS_END
 };

js/src/builtin/Promise.h

@@ -106,6 +106,12 @@ class PromiseObject : public NativeObject
 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.
@@ -119,7 +125,7 @@ GetWaitForAllPromise(JSContext* cx, const JS::AutoObjectVector& promises);
 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 )
@@ -205,13 +211,6 @@ class PromiseTask : public JS::AsyncTask
     bool executeAndFinish(JSContext* cx);
 };
 
-bool
-Promise_static_resolve(JSContext* cx, unsigned argc, Value* vp);
-bool
-Promise_reject(JSContext* cx, unsigned argc, Value* vp);
-bool
-Promise_then(JSContext* cx, unsigned argc, Value* vp);
-
 } // namespace js
 
 #endif /* builtin_Promise_h */

js/src/jsapi.cpp

@@ -4994,7 +4994,8 @@ JS::RejectPromise(JSContext* cx, JS::HandleObject promiseObj, JS::HandleValue re
 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(cx);
     CHECK_REQUEST(cx);
@@ -5044,8 +5045,11 @@ 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;
 }
 
@@ -5054,7 +5058,8 @@ 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;
 }

js/src/jsobjinlines.h

@@ -566,6 +566,12 @@ IsNativeFunction(const js::Value& v, JSNative native)
     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.