Bug 1508438 - Part 1: Update step numbers in ReadableStreamTee_Cancel. r=arai
authorJason Orendorff <jorendorff@mozilla.com>
Tue, 20 Nov 2018 11:35:48 +0000
changeset 503629 2ed75fc212f1d4adf4fa5f9ab0ae886df5438f56
parent 503628 3f594d61b2cc489ea3e6e7472d9c37c5270c6531
child 503630 0038d6eebaef3b88964668e6d31a3244f03639cc
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1508438
milestone65.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 1508438 - Part 1: Update step numbers in ReadableStreamTee_Cancel. r=arai This also renames TeeState::promise to cancelPromise. Differential Revision: https://phabricator.services.mozilla.com/D12349
js/src/builtin/Stream.cpp
--- a/js/src/builtin/Stream.cpp
+++ b/js/src/builtin/Stream.cpp
@@ -354,32 +354,33 @@ class TeeState : public NativeObject
      * Memory layout for TeeState instances.
      *
      * The Reason1 and Reason2 slots store opaque values, which might be
      * wrapped objects from other compartments. Since we don't treat them as
      * objects in Streams-specific code, we don't have to worry about that
      * apart from ensuring that the values are properly wrapped before storing
      * them.
      *
-     * Promise is always created in TeeState::create below, so is guaranteed
-     * to be in the same compartment as the TeeState instance itself.
+     * CancelPromise is always created in TeeState::create below, so is
+     * guaranteed to be in the same compartment as the TeeState instance
+     * itself.
      *
      * Stream can be from another compartment. It is automatically wrapped
      * before storing it and unwrapped upon retrieval. That means that
      * TeeState consumers need to be able to deal with unwrapped
      * ReadableStream instances from non-current compartments.
      *
      * Branch1 and Branch2 are always created in the same compartment as the
      * TeeState instance, so cannot be from another compartment.
      */
     enum Slots {
         Slot_Flags = 0,
         Slot_Reason1,
         Slot_Reason2,
-        Slot_Promise,
+        Slot_CancelPromise,
         Slot_Stream,
         Slot_Branch1,
         Slot_Branch2,
         SlotCount
     };
 
   private:
     enum Flags {
@@ -421,18 +422,18 @@ class TeeState : public NativeObject
         return getFixedSlot(Slot_Reason1);
     }
 
     Value reason2() const {
         MOZ_ASSERT(canceled2());
         return getFixedSlot(Slot_Reason2);
     }
 
-    PromiseObject* promise() {
-        return &getFixedSlot(Slot_Promise).toObject().as<PromiseObject>();
+    PromiseObject* cancelPromise() {
+        return &getFixedSlot(Slot_CancelPromise).toObject().as<PromiseObject>();
     }
 
     ReadableStreamDefaultController* branch1() {
         ReadableStreamDefaultController* controller =
             &getFixedSlot(Slot_Branch1).toObject()
             .as<ReadableStreamDefaultController>();
         MOZ_ASSERT(controller->flags() & ReadableStreamController::Flag_TeeBranch);
         MOZ_ASSERT(controller->isTeeBranch1());
@@ -459,23 +460,23 @@ class TeeState : public NativeObject
     }
 
     static TeeState* create(JSContext* cx, Handle<ReadableStream*> unwrappedStream) {
         Rooted<TeeState*> state(cx, NewBuiltinClassInstance<TeeState>(cx));
         if (!state) {
             return nullptr;
         }
 
-        Rooted<PromiseObject*> promise(cx, PromiseObject::createSkippingExecutor(cx));
-        if (!promise) {
+        Rooted<PromiseObject*> cancelPromise(cx, PromiseObject::createSkippingExecutor(cx));
+        if (!cancelPromise) {
             return nullptr;
         }
 
         state->setFixedSlot(Slot_Flags, Int32Value(0));
-        state->setFixedSlot(Slot_Promise, ObjectValue(*promise));
+        state->setFixedSlot(Slot_CancelPromise, ObjectValue(*cancelPromise));
         RootedObject wrappedStream(cx, unwrappedStream);
         if (!cx->compartment()->wrap(cx, &wrappedStream)) {
             return nullptr;
         }
         state->setFixedSlot(Slot_Stream, ObjectValue(*wrappedStream));
 
         return state;
     }
@@ -867,17 +868,17 @@ CLASS_SPEC(ReadableStream, 0, SlotCount,
 
 
 /*** 3.3. General readable stream abstract operations ********************************************/
 
 // Streams spec, 3.3.1. AcquireReadableStreamBYOBReader ( stream )
 // Always inlined.
 
 // Streams spec, 3.3.2. AcquireReadableStreamDefaultReader ( stream )
-// Always inlined.
+// Always inlined. See CreateReadableStreamDefaultReader.
 
 // Streams spec, 3.3.3. CreateReadableStream ( startAlgorithm, pullAlgorithm, cancelAlgorithm [, highWaterMark [, sizeAlgorithm ] ] )
 // Not implemented.
 
 // Streams spec, 3.3.4. CreateReadableByteStream ( startAlgorithm, pullAlgorithm, cancelAlgorithm [, highWaterMark [, autoAllocateChunkSize ] ] )
 // Not implemented.
 
 // Streams spec, 3.3.5. InitializeReadableStream ( stream )
@@ -1051,34 +1052,37 @@ ReadableStreamTee_Pull(JSContext* cx, Ha
         return nullptr;
     }
 
     return JS::CallOriginalPromiseThen(cx, readPromise, onFulfilled, nullptr);
 }
 
 /**
  * Cancel one branch of a tee'd stream with the given |reason_|.
+ *
+ * Streams spec, 3.3.9. ReadableStreamTee steps 13 and 14: "Let
+ * cancel1Algorithm/cancel2Algorithm be the following steps, taking a reason
+ * argument:"
  */
 static MOZ_MUST_USE JSObject*
 ReadableStreamTee_Cancel(JSContext* cx,
                          Handle<TeeState*> unwrappedTeeState,
                          Handle<ReadableStreamDefaultController*> unwrappedBranch,
                          HandleValue reason)
 {
-    // Step 1: Let stream be F.[[stream]] and teeState be F.[[teeState]].
     Rooted<ReadableStream*> unwrappedStream(cx,
         UnwrapInternalSlot<ReadableStream>(cx, unwrappedTeeState, TeeState::Slot_Stream));
     if (!unwrappedStream) {
         return nullptr;
     }
 
     bool bothBranchesCanceled = false;
 
-    // Step 2: Set teeState.[[canceled1]] to true.
-    // Step 3: Set teeState.[[reason1]] to reason.
+    // Step 13/14.a: Set canceled1/canceled2 to true.
+    // Step 13/14.b: Set reason1/reason2 to reason.
     {
         RootedValue unwrappedReason(cx, reason);
         {
             AutoRealm ar(cx, unwrappedTeeState);
             if (!cx->compartment()->wrap(cx, &unwrappedReason)) {
                 return nullptr;
             }
         }
@@ -1087,67 +1091,70 @@ ReadableStreamTee_Cancel(JSContext* cx,
             bothBranchesCanceled = unwrappedTeeState->canceled2();
         } else {
             MOZ_ASSERT(unwrappedBranch->isTeeBranch2());
             unwrappedTeeState->setCanceled2(unwrappedReason);
             bothBranchesCanceled = unwrappedTeeState->canceled1();
         }
     }
 
-    // Step 4: If teeState.[[canceled1]] is true,
-    // Step 4: If teeState.[[canceled2]] is true,
+    // Step 13/14.c: If canceled2/canceled1 is true,
     if (bothBranchesCanceled) {
-        // Step a: Let compositeReason be
-        //         ! CreateArrayFromList(« teeState.[[reason1]], teeState.[[reason2]] »).
+        // Step 13/14.c.i: Let compositeReason be
+        //                 ! CreateArrayFromList(« reason1, reason2 »).
         RootedNativeObject compositeReason(cx, NewDenseFullyAllocatedArray(cx, 2));
         if (!compositeReason) {
             return nullptr;
         }
 
         compositeReason->setDenseInitializedLength(2);
 
         RootedValue reason1(cx, unwrappedTeeState->reason1());
         RootedValue reason2(cx, unwrappedTeeState->reason2());
         if (!cx->compartment()->wrap(cx, &reason1) || !cx->compartment()->wrap(cx, &reason2)) {
             return nullptr;
         }
         compositeReason->initDenseElement(0, reason1);
         compositeReason->initDenseElement(1, reason2);
         RootedValue compositeReasonVal(cx, ObjectValue(*compositeReason));
 
-        // Step b: Let cancelResult be ! ReadableStreamCancel(stream, compositeReason).
+        // Step 13/14.c.ii: Let cancelResult be
+        //                  ! ReadableStreamCancel(stream, compositeReason).
+        // In our implementation, this can fail with OOM. The best course then
+        // is to reject cancelPromise with an OOM error.
         RootedObject cancelResult(cx,
             ::ReadableStreamCancel(cx, unwrappedStream, compositeReasonVal));
         {
-            Rooted<PromiseObject*> promise(cx, unwrappedTeeState->promise());
-            AutoRealm ar(cx, promise);
+            Rooted<PromiseObject*> cancelPromise(cx, unwrappedTeeState->cancelPromise());
+            AutoRealm ar(cx, cancelPromise);
 
             if (!cancelResult) {
-                if (!RejectPromiseWithPendingError(cx, promise)) {
+                // Handle the OOM case mentioned above.
+                if (!RejectPromiseWithPendingError(cx, cancelPromise)) {
                     return nullptr;
                 }
             } else {
-                // Step c: Resolve teeState.[[promise]] with cancelResult.
+                // Step 13/14.c.iii: Resolve cancelPromise with cancelResult.
                 RootedValue resultVal(cx, ObjectValue(*cancelResult));
                 if (!cx->compartment()->wrap(cx, &resultVal)) {
                     return nullptr;
                 }
-                if (!PromiseObject::resolve(cx, promise, resultVal)) {
+                if (!PromiseObject::resolve(cx, cancelPromise, resultVal)) {
                     return nullptr;
                 }
             }
         }
     }
 
-    // Step 5: Return teeState.[[promise]].
-    RootedObject promise(cx, unwrappedTeeState->promise());
-    if (!cx->compartment()->wrap(cx, &promise)) {
+    // Step 13/14.d: Return cancelPromise.
+    RootedObject cancelPromise(cx, unwrappedTeeState->cancelPromise());
+    if (!cx->compartment()->wrap(cx, &cancelPromise)) {
         return nullptr;
     }
-    return promise;
+    return cancelPromise;
 }
 
 static MOZ_MUST_USE bool
 ReadableStreamDefaultControllerErrorIfNeeded(JSContext* cx,
                                              Handle<ReadableStreamDefaultController*> unwrappedController,
                                              HandleValue e);
 
 /**