Bug 1289318 - Part 8: Combine Promise state and rejection handling info into a single flags field. r?efaust draft
authorTill Schneidereit <till@tillschneidereit.net>
Sat, 06 Aug 2016 18:26:22 +0200
changeset 400701 a7b0f6d3a055a4f4a94171cebb306b9db84e9b2c
parent 400700 4ba2419ce2ab2a609fc7d5f20ce785b44fcf7859
child 400702 228ccb3b4da510eca5fb15beb9f4aed6ee08bdc8
child 401141 e23bac24c9c438be048aeee991b5e51030a0afd2
push id26245
push userbmo:till@tillschneidereit.net
push dateMon, 15 Aug 2016 14:02:13 +0000
reviewersefaust
bugs1289318
milestone51.0a1
Bug 1289318 - Part 8: Combine Promise state and rejection handling info into a single flags field. r?efaust The rejection handling state will be required even without devtools being open once projection rejection tracking events[1] are implemented, so it should always be tracked on the Promise itself. The other debugging state will be moved into a debug-only object referenced via a slot on Promise. MozReview-Commit-ID: LM10qruLDxz
js/src/builtin/Promise.cpp
js/src/builtin/Promise.h
js/src/builtin/Promise.js
js/src/builtin/SelfHostingDefines.h
js/src/builtin/TestingFunctions.cpp
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -150,17 +150,21 @@ ResolvePromise(JSContext* cx, Handle<Pro
     RootedValue reactionsVal(cx, promise->getFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT));
 
     // Step 3-5.
     // The same slot is used for the reactions list and the result, so setting
     // the result also removes the reactions list.
     promise->setFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT, valueOrReason);
 
     // Step 6.
-    promise->setFixedSlot(PROMISE_STATE_SLOT, Int32Value(int32_t(state)));
+    int32_t flags = promise->getFixedSlot(PROMISE_FLAGS_SLOT).toInt32();
+    flags |= PROMISE_FLAG_RESOLVED;
+    if (state == JS::PromiseState::Fulfilled)
+        flags |= PROMISE_FLAG_FULFILLED;
+    promise->setFixedSlot(PROMISE_FLAGS_SLOT, Int32Value(flags));
 
     // Also null out the resolve/reject functions so they can be GC'd.
     promise->setFixedSlot(PROMISE_RESOLVE_FUNCTION_SLOT, UndefinedValue());
 
     // Now that everything else is done, do the things the debugger needs.
     // Step 7 of RejectPromise implemented in onSettled.
     promise->onSettled(cx);
 
@@ -425,24 +429,23 @@ PromiseObject::create(JSContext* cx, Han
         if (wrappedProto)
             ac.emplace(cx, usedProto);
 
         promise = NewObjectWithClassProto<PromiseObject>(cx, usedProto);
         if (!promise)
             return nullptr;
 
         // Step 4.
-        promise->setFixedSlot(PROMISE_STATE_SLOT, Int32Value(PROMISE_STATE_PENDING));
+        promise->setFixedSlot(PROMISE_FLAGS_SLOT, Int32Value(0));
 
         // Steps 5-6.
         // Omitted, we allocate our single list of reaction records lazily.
 
         // Step 7.
-        promise->setFixedSlot(PROMISE_IS_HANDLED_SLOT,
-                              Int32Value(PROMISE_IS_HANDLED_STATE_UNHANDLED));
+        // Implicit, the handled flag is unset by default.
 
         // Store an allocation stack so we can later figure out what the
         // control flow was for some unexpected results. Frightfully expensive,
         // but oh well.
         RootedObject stack(cx);
         if (cx->options().asyncStack() || cx->compartment()->isDebuggee()) {
             if (!JS::CaptureCurrentStack(cx, &stack, JS::StackCapture(JS::AllFrames())))
                 return nullptr;
@@ -675,17 +678,17 @@ PromiseConstructor(JSContext* cx, unsign
     return true;
 }
 
 
 
 bool
 PromiseObject::resolve(JSContext* cx, HandleValue resolutionValue)
 {
-    if (this->getFixedSlot(PROMISE_STATE_SLOT).toInt32() != unsigned(JS::PromiseState::Pending))
+    if (state() != JS::PromiseState::Pending)
         return true;
 
     RootedValue funVal(cx, this->getReservedSlot(PROMISE_RESOLVE_FUNCTION_SLOT));
     // TODO: ensure that this holds for xray'd promises. (It probably doesn't)
     MOZ_ASSERT(funVal.toObject().is<JSFunction>());
 
     FixedInvokeArgs<1> args(cx);
 
@@ -693,17 +696,17 @@ PromiseObject::resolve(JSContext* cx, Ha
 
     RootedValue dummy(cx);
     return Call(cx, funVal, UndefinedHandleValue, args, &dummy);
 }
 
 bool
 PromiseObject::reject(JSContext* cx, HandleValue rejectionValue)
 {
-    if (this->getFixedSlot(PROMISE_STATE_SLOT).toInt32() != unsigned(JS::PromiseState::Pending))
+    if (state() != JS::PromiseState::Pending)
         return true;
 
     RootedValue resolveVal(cx, this->getReservedSlot(PROMISE_RESOLVE_FUNCTION_SLOT));
     RootedFunction resolve(cx, &resolveVal.toObject().as<JSFunction>());
     RootedValue funVal(cx, resolve->getExtendedSlot(ResolutionFunctionSlot_OtherFunction));
     MOZ_ASSERT(funVal.toObject().is<JSFunction>());
 
     FixedInvokeArgs<1> args(cx);
@@ -723,22 +726,18 @@ PromiseObject::onSettled(JSContext* cx)
         if (!JS::CaptureCurrentStack(cx, &stack, JS::StackCapture(JS::AllFrames()))) {
             cx->clearPendingException();
             return;
         }
     }
     promise->setFixedSlot(PROMISE_RESOLUTION_SITE_SLOT, ObjectOrNullValue(stack));
     promise->setFixedSlot(PROMISE_RESOLUTION_TIME_SLOT, DoubleValue(MillisecondsSinceStartup()));
 
-    if (promise->state() == JS::PromiseState::Rejected &&
-        promise->getFixedSlot(PROMISE_IS_HANDLED_SLOT).toInt32() !=
-            PROMISE_IS_HANDLED_STATE_HANDLED)
-    {
+    if (promise->state() == JS::PromiseState::Rejected && promise->markedAsUncaught())
         cx->runtime()->addUnhandledRejectedPromise(cx, promise);
-    }
 
     JS::dbg::onPromiseSettled(cx, promise);
 }
 
 enum ReactionJobSlots {
     ReactionJobSlot_Handler = 0,
     ReactionJobSlot_JobData,
 };
--- a/js/src/builtin/Promise.h
+++ b/js/src/builtin/Promise.h
@@ -12,26 +12,31 @@
 
 namespace js {
 
 class AutoSetNewObjectMetadata;
 
 class PromiseObject : public NativeObject
 {
   public:
-    static const unsigned RESERVED_SLOTS = 12;
+    static const unsigned RESERVED_SLOTS = 8;
     static const Class class_;
     static const Class protoClass_;
     static PromiseObject* create(JSContext* cx, HandleObject executor,
                                  HandleObject proto = nullptr);
 
     JS::PromiseState state() {
-        int32_t state = getFixedSlot(PROMISE_STATE_SLOT).toInt32();
-        MOZ_ASSERT(state >= 0 && state <= int32_t(JS::PromiseState::Rejected));
-        return JS::PromiseState(state);
+        int32_t flags = getFixedSlot(PROMISE_FLAGS_SLOT).toInt32();
+        if (!(flags & PROMISE_FLAG_RESOLVED)) {
+            MOZ_ASSERT(!(flags & PROMISE_FLAG_FULFILLED));
+            return JS::PromiseState::Pending;
+        }
+        if (flags & PROMISE_FLAG_FULFILLED)
+            return JS::PromiseState::Fulfilled;
+        return JS::PromiseState::Rejected;
     }
     Value value()  {
         MOZ_ASSERT(state() == JS::PromiseState::Fulfilled);
         return getFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT);
     }
     Value reason() {
         MOZ_ASSERT(state() == JS::PromiseState::Rejected);
         return getFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT);
@@ -53,22 +58,22 @@ class PromiseObject : public NativeObjec
     double lifetime();
     double timeToResolution() {
         MOZ_ASSERT(state() != JS::PromiseState::Pending);
         return resolutionTime() - allocationTime();
     }
     MOZ_MUST_USE bool dependentPromises(JSContext* cx, MutableHandle<GCVector<Value>> values);
     uint64_t getID();
     bool markedAsUncaught() {
-        return getFixedSlot(PROMISE_IS_HANDLED_SLOT).toInt32() != PROMISE_IS_HANDLED_STATE_HANDLED;
+        return !(getFixedSlot(PROMISE_FLAGS_SLOT).toInt32() & PROMISE_FLAG_HANDLED);
     }
     void markAsReported() {
-        MOZ_ASSERT(getFixedSlot(PROMISE_IS_HANDLED_SLOT).toInt32() ==
-                   PROMISE_IS_HANDLED_STATE_UNHANDLED);
-        setFixedSlot(PROMISE_IS_HANDLED_SLOT, Int32Value(PROMISE_IS_HANDLED_STATE_REPORTED));
+        MOZ_ASSERT(markedAsUncaught());
+        int32_t flags = getFixedSlot(PROMISE_FLAGS_SLOT).toInt32();
+        setFixedSlot(PROMISE_FLAGS_SLOT, Int32Value(flags | PROMISE_FLAG_REPORTED));
     }
 };
 
 /**
  * Tells the embedding to enqueue a Promise reaction job, based on six
  * parameters:
  * reaction handler - The callback to invoke for this job.
    argument - The first and only argument to pass to the handler.
--- a/js/src/builtin/Promise.js
+++ b/js/src/builtin/Promise.js
@@ -731,16 +731,17 @@ function PerformPromiseThen(promise, onF
                                              resultCapability.resolve,
                                              resultCapability.reject,
                                              onFulfilled,
                                              onRejected,
                                              incumbentGlobal);
 
     // Step 7.
     let state = GetPromiseState(promise);
+    let flags = UnsafeGetInt32FromReservedSlot(promise, PROMISE_FLAGS_SLOT);
     if (state === PROMISE_STATE_PENDING) {
         // Steps 7.a,b.
         // We only have a single list for fulfill and reject reactions.
         let reactions = UnsafeGetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT);
         if (!reactions)
             UnsafeSetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT, [reaction]);
         else
             _DefineDataProperty(reactions, reactions.length, reaction);
@@ -759,37 +760,41 @@ function PerformPromiseThen(promise, onF
     else {
         // Step 9.a.
         assert(state === PROMISE_STATE_REJECTED, "Invalid Promise state " + state);
 
         // Step 9.b.
         let reason = UnsafeGetReservedSlot(promise, PROMISE_REACTIONS_OR_RESULT_SLOT);
 
         // Step 9.c.
-        if (UnsafeGetInt32FromReservedSlot(promise, PROMISE_IS_HANDLED_SLOT) !==
-            PROMISE_IS_HANDLED_STATE_HANDLED)
-        {
+        if (!(flags & PROMISE_FLAG_HANDLED))
             HostPromiseRejectionTracker(promise, PROMISE_REJECTION_TRACKER_OPERATION_HANDLE);
-        }
 
         // Step 9.d.
         EnqueuePromiseReactionJob(reaction, PROMISE_JOB_TYPE_REJECT, reason);
     }
 
     // Step 10.
-    UnsafeSetReservedSlot(promise, PROMISE_IS_HANDLED_SLOT, PROMISE_IS_HANDLED_STATE_HANDLED);
+    UnsafeSetReservedSlot(promise, PROMISE_FLAGS_SLOT, flags | PROMISE_FLAG_HANDLED);
 
     // Step 11.
     return resultCapability.promise;
 }
 
 /// Utility functions below.
 function IsPromiseReaction(record) {
     return std_Reflect_getPrototypeOf(record) === PromiseReactionRecordProto;
 }
 
 function IsPromiseCapability(capability) {
     return std_Reflect_getPrototypeOf(capability) === PromiseCapabilityRecordProto;
 }
 
 function GetPromiseState(promise) {
-    return UnsafeGetInt32FromReservedSlot(promise, PROMISE_STATE_SLOT);
+    let flags = UnsafeGetInt32FromReservedSlot(promise, PROMISE_FLAGS_SLOT);
+    if (!(flags & PROMISE_FLAG_RESOLVED)) {
+        assert(!(flags & PROMISE_STATE_FULFILLED), "Fulfilled promises are resolved, too");
+        return PROMISE_STATE_PENDING;
+    }
+    if (flags & PROMISE_FLAG_FULFILLED)
+        return PROMISE_STATE_FULFILLED;
+    return PROMISE_STATE_REJECTED;
 }
--- a/js/src/builtin/SelfHostingDefines.h
+++ b/js/src/builtin/SelfHostingDefines.h
@@ -52,33 +52,33 @@
 #define ITERATOR_SLOT_ITEM_KIND 2
 // Used for ListIterator.
 #define ITERATOR_SLOT_NEXT_METHOD 2
 
 #define ITEM_KIND_KEY 0
 #define ITEM_KIND_VALUE 1
 #define ITEM_KIND_KEY_AND_VALUE 2
 
-#define PROMISE_STATE_SLOT               0
+#define PROMISE_FLAGS_SLOT               0
 #define PROMISE_REACTIONS_OR_RESULT_SLOT 1
 #define PROMISE_RESOLVE_FUNCTION_SLOT    2
 #define PROMISE_ALLOCATION_SITE_SLOT     3
 #define PROMISE_RESOLUTION_SITE_SLOT     4
 #define PROMISE_ALLOCATION_TIME_SLOT     5
 #define PROMISE_RESOLUTION_TIME_SLOT     6
 #define PROMISE_ID_SLOT                  7
-#define PROMISE_IS_HANDLED_SLOT          8
 
 #define PROMISE_STATE_PENDING   0
 #define PROMISE_STATE_FULFILLED 1
 #define PROMISE_STATE_REJECTED  2
 
-#define PROMISE_IS_HANDLED_STATE_HANDLED   0
-#define PROMISE_IS_HANDLED_STATE_UNHANDLED 1
-#define PROMISE_IS_HANDLED_STATE_REPORTED  2
+#define PROMISE_FLAG_RESOLVED  0x1
+#define PROMISE_FLAG_FULFILLED 0x2
+#define PROMISE_FLAG_HANDLED   0x4
+#define PROMISE_FLAG_REPORTED  0x8
 
 #define PROMISE_HANDLER_IDENTITY 0
 #define PROMISE_HANDLER_THROWER  1
 
 #define PROMISE_REJECTION_TRACKER_OPERATION_REJECT false
 #define PROMISE_REJECTION_TRACKER_OPERATION_HANDLE true
 
 #define PROMISE_JOB_TYPE_FULFILL "fulfillHandler"
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1448,18 +1448,20 @@ SettlePromiseNow(JSContext* cx, unsigned
     if (!args.requireAtLeast(cx, "settlePromiseNow", 1))
         return false;
     if (!args[0].isObject() || !args[0].toObject().is<PromiseObject>()) {
         JS_ReportError(cx, "first argument must be a Promise object");
         return false;
     }
 
     RootedNativeObject promise(cx, &args[0].toObject().as<NativeObject>());
-    promise->setReservedSlot(PROMISE_STATE_SLOT, Int32Value(PROMISE_STATE_FULFILLED));
-    promise->setReservedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT, UndefinedValue());
+    int32_t flags = promise->getFixedSlot(PROMISE_FLAGS_SLOT).toInt32();
+    promise->setFixedSlot(PROMISE_FLAGS_SLOT,
+                          Int32Value(flags | PROMISE_FLAG_RESOLVED | PROMISE_FLAG_FULFILLED));
+    promise->setFixedSlot(PROMISE_REACTIONS_OR_RESULT_SLOT, UndefinedValue());
 
     JS::dbg::onPromiseSettled(cx, promise);
     return true;
 }
 
 #else
 
 static const js::Class FakePromiseClass = {