Bug 1507950 - Allow calling controller.error() when the stream is not readable. r=arai
authorJason Orendorff <jorendorff@mozilla.com>
Sun, 09 Dec 2018 17:59:36 +0000
changeset 449677 a6eb6e3732aea5d375757a28ea5cfe3de430a6f9
parent 449652 efcada818361f5d8cc3ab78fb6039147e3da45ba
child 449678 c2ecb25b00d4ad095f23e66fb91507c7978fda8b
push idunknown
push userunknown
push dateunknown
reviewersarai
bugs1507950
milestone65.0a1
Bug 1507950 - Allow calling controller.error() when the stream is not readable. r=arai Differential Revision: https://phabricator.services.mozilla.com/D13748
js/src/builtin/Stream.cpp
js/src/jit-test/tests/web-platform/streams/readable-streams/bad-underlying-sources.js
js/src/jit-test/tests/web-platform/streams/readable-streams/garbage-collection.js
testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.dedicatedworker.html.ini
testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.html.ini
testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.serviceworker.https.html.ini
testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.sharedworker.html.ini
testing/web-platform/meta/streams/readable-streams/garbage-collection.dedicatedworker.html.ini
testing/web-platform/meta/streams/readable-streams/garbage-collection.html.ini
testing/web-platform/meta/streams/readable-streams/garbage-collection.serviceworker.https.html.ini
testing/web-platform/meta/streams/readable-streams/garbage-collection.sharedworker.html.ini
--- a/js/src/builtin/Stream.cpp
+++ b/js/src/builtin/Stream.cpp
@@ -1175,18 +1175,18 @@ static MOZ_MUST_USE JSObject* ReadableSt
   // Step 13/14.d: Return cancelPromise.
   RootedObject cancelPromise(cx, unwrappedTeeState->cancelPromise());
   if (!cx->compartment()->wrap(cx, &cancelPromise)) {
     return nullptr;
   }
   return cancelPromise;
 }
 
-static MOZ_MUST_USE bool ReadableStreamDefaultControllerErrorIfNeeded(
-    JSContext* cx, Handle<ReadableStreamDefaultController*> unwrappedController,
+static MOZ_MUST_USE bool ReadableStreamControllerError(
+    JSContext* cx, Handle<ReadableStreamController*> unwrappedController,
     HandleValue e);
 
 /**
  * Streams spec, 3.3.9. step 18:
  * Upon rejection of reader.[[closedPromise]] with reason r,
  */
 static bool TeeReaderClosedHandler(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
@@ -1195,27 +1195,29 @@ static bool TeeReaderClosedHandler(JSCon
 
   // Step a: If closedOrErrored is false, then:
   if (!teeState->closedOrErrored()) {
     // Step a.iii: Set closedOrErrored to true.
     // Reordered to ensure that internal errors in the other steps don't
     // leave the teeState in an undefined state.
     teeState->setClosedOrErrored();
 
-    // Step a.i: Perform ! ReadableStreamDefaultControllerErrorIfNeeded(
-    //                          branch1.[[readableStreamController]], r).
+    // Step a.i: Perform
+    //           ! ReadableStreamDefaultControllerError(
+    //               branch1.[[readableStreamController]], r).
     Rooted<ReadableStreamDefaultController*> branch1(cx, teeState->branch1());
-    if (!ReadableStreamDefaultControllerErrorIfNeeded(cx, branch1, reason)) {
+    if (!ReadableStreamControllerError(cx, branch1, reason)) {
       return false;
     }
 
-    // Step a.ii: Perform ! ReadableStreamDefaultControllerErrorIfNeeded(
-    //                          branch2.[[readableStreamController]], r).
+    // Step a.ii: Perform
+    //            ! ReadableStreamDefaultControllerError(
+    //                branch2.[[readableStreamController]], r).
     Rooted<ReadableStreamDefaultController*> branch2(cx, teeState->branch2());
-    if (!ReadableStreamDefaultControllerErrorIfNeeded(cx, branch2, reason)) {
+    if (!ReadableStreamControllerError(cx, branch2, reason)) {
       return false;
     }
   }
 
   args.rval().setUndefined();
   return true;
 }
 
@@ -2226,46 +2228,33 @@ static bool ControllerStartHandler(JSCon
   //          ! ReadableByteStreamControllerCallPullIfNeeded((controller).
   if (!ReadableStreamControllerCallPullIfNeeded(cx, controller)) {
     return false;
   }
   args.rval().setUndefined();
   return true;
 }
 
-static MOZ_MUST_USE bool ReadableStreamControllerError(
-    JSContext* cx, Handle<ReadableStreamController*> unwrappedController,
-    HandleValue e);
-
 /**
- * Streams spec, 3.8.3, step 11.b.
+ * Streams spec, 3.9.11, step 12.a.
  * and
- * Streams spec, 3.10.3, step 16.b.
+ * Streams spec, 3.12.26, step 17.a.
  */
 static bool ControllerStartFailedHandler(JSContext* cx, unsigned argc,
                                          Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   Rooted<ReadableStreamController*> controller(
       cx, TargetFromHandler<ReadableStreamController>(args));
 
-  // 3.8.3, Step 11.b.i:
-  // Perform ! ReadableStreamDefaultControllerErrorIfNeeded(controller, r).
-  if (controller->is<ReadableStreamDefaultController>()) {
-    Rooted<ReadableStreamDefaultController*> defaultController(
-        cx, &controller->as<ReadableStreamDefaultController>());
-    return ReadableStreamDefaultControllerErrorIfNeeded(cx, defaultController,
-                                                        args.get(0));
-  }
-
-  // 3.10.3, Step 16.b.i: If stream.[[state]] is "readable", perform
-  //                      ! ReadableByteStreamControllerError(controller, r).
-  if (controller->stream()->readable()) {
-    if (!ReadableStreamControllerError(cx, controller, args.get(0))) {
-      return false;
-    }
+  // 3.9.11, Step 12.a: Perform
+  //      ! ReadableStreamDefaultControllerError(controller, r).
+  // 3.12.26, Step 17.a: Perform
+  //      ! ReadableByteStreamControllerError(controller, r).
+  if (!ReadableStreamControllerError(cx, controller, args.get(0))) {
+    return false;
   }
 
   args.rval().setUndefined();
   return true;
 }
 
 /**
  * Streams spec, 3.8.3.
@@ -2424,38 +2413,29 @@ static bool ReadableStreamDefaultControl
 
 /**
  * Streams spec, 3.8.4.4. error ( e )
  */
 static bool ReadableStreamDefaultController_error(JSContext* cx, unsigned argc,
                                                   Value* vp) {
   // Step 1: If ! IsReadableStreamDefaultController(this) is false, throw a
   //         TypeError exception.
-
   CallArgs args = CallArgsFromVp(argc, vp);
   Rooted<ReadableStreamDefaultController*> unwrappedController(
       cx, UnwrapAndTypeCheckThis<ReadableStreamDefaultController>(cx, args,
                                                                   "enqueue"));
   if (!unwrappedController) {
     return false;
   }
 
-  // Step 2: Let stream be this.[[controlledReadableStream]].
-  // Step 3: If stream.[[state]] is not "readable", throw a TypeError exception.
-  if (!unwrappedController->stream()->readable()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_READABLESTREAMCONTROLLER_NOT_READABLE,
-                              "error");
-    return false;
-  }
-
-  // Step 4: Perform ! ReadableStreamDefaultControllerError(this, e).
+  // Step 2: Perform ! ReadableStreamDefaultControllerError(this, e).
   if (!ReadableStreamControllerError(cx, unwrappedController, args.get(0))) {
     return false;
   }
+
   args.rval().setUndefined();
   return true;
 }
 
 static const JSPropertySpec ReadableStreamDefaultController_properties[] = {
     JS_PSG("desiredSize", ReadableStreamDefaultController_desiredSize, 0),
     JS_PS_END};
 
@@ -2722,22 +2702,20 @@ static bool ControllerPullFailedHandler(
   HandleValue e = args.get(0);
 
   Rooted<ReadableStreamController*> controller(
       cx, UnwrapCalleeSlot<ReadableStreamController>(cx, args, 0));
   if (!controller) {
     return false;
   }
 
-  // Step a: If controller.[[controlledReadableStream]].[[state]] is "readable",
-  //         perform ! ReadableByteStreamControllerError(controller, e).
-  if (controller->stream()->readable()) {
-    if (!ReadableStreamControllerError(cx, controller, e)) {
-      return false;
-    }
+  // Step a: Perform ! ReadableStreamDefaultControllerError(controller, e).
+  //         (ReadableByteStreamControllerError in 3.12.3.)
+  if (!ReadableStreamControllerError(cx, controller, e)) {
+    return false;
   }
 
   args.rval().setUndefined();
   return true;
 }
 
 static bool ReadableStreamControllerShouldCallPull(
     ReadableStreamController* unwrappedController);
@@ -2977,71 +2955,69 @@ static MOZ_MUST_USE bool ReadableStreamD
   AssertSameCompartment(cx, chunk);
 
   // Step 1: Let stream be controller.[[controlledReadableStream]].
   Rooted<ReadableStream*> unwrappedStream(cx, unwrappedController->stream());
 
   // Step 2: Assert: controller.[[closeRequested]] is false.
   MOZ_ASSERT(!unwrappedController->closeRequested());
 
-  // Step 3: Assert: stream.[[state]] is "readable".
-  MOZ_ASSERT(unwrappedStream->readable());
-
-  // Step 4: If ! IsReadableStreamLocked(stream) is true and
+  // Step 3: If ! IsReadableStreamLocked(stream) is true and
   //         ! ReadableStreamGetNumReadRequests(stream) > 0, perform
   //         ! ReadableStreamFulfillReadRequest(stream, chunk, false).
   if (unwrappedStream->locked() &&
       ReadableStreamGetNumReadRequests(unwrappedStream) > 0) {
     if (!ReadableStreamFulfillReadOrReadIntoRequest(cx, unwrappedStream, chunk,
                                                     false)) {
       return false;
     }
   } else {
-    // Step 5: Otherwise,
-    // Step a: Let chunkSize be 1.
+    // Step 4: Otherwise,
+    // Step a: Let result be the result of performing
+    //         controller.[[strategySizeAlgorithm]], passing in chunk, and
+    //         interpreting the result as an ECMAScript completion value.
+    // Step c: (on success) Let chunkSize be result.[[Value]].
     RootedValue chunkSize(cx, NumberValue(1));
     bool success = true;
-
-    // Step b: If controller.[[strategySize]] is not undefined,
     RootedValue strategySize(cx, unwrappedController->strategySize());
     if (!strategySize.isUndefined()) {
-      // Step i: Set chunkSize to
-      //         Call(stream.[[strategySize]], undefined, chunk).
       if (!cx->compartment()->wrap(cx, &strategySize)) {
         return false;
       }
       success = Call(cx, strategySize, UndefinedHandleValue, chunk, &chunkSize);
     }
 
-    // Step c: Let enqueueResult be
+    // Step d: Let enqueueResult be
     //         EnqueueValueWithSize(controller, chunk, chunkSize).
     if (success) {
       success = EnqueueValueWithSize(cx, unwrappedController, chunk, chunkSize);
     }
 
     if (!success) {
-      // Step b.ii: If chunkSize is an abrupt completion,
+      // Step b: If result is an abrupt completion,
       // and
-      // Step d: If enqueueResult is an abrupt completion,
+      // Step e: If enqueueResult is an abrupt completion,
       RootedValue exn(cx);
       if (!cx->isExceptionPending() || !GetAndClearException(cx, &exn)) {
         // Uncatchable error. Die immediately without erroring the
         // stream.
         return false;
       }
 
-      // Step b.ii.1: Perform
-      //              ! ReadableStreamDefaultControllerErrorIfNeeded(
-      //                  controller, chunkSize.[[Value]]).
-      if (!ReadableStreamDefaultControllerErrorIfNeeded(cx, unwrappedController,
-                                                        exn)) {
+      // Step b.i: Perform ! ReadableStreamDefaultControllerError(
+      //           controller, result.[[Value]]).
+      // Step e.i: Perform ! ReadableStreamDefaultControllerError(
+      //           controller, enqueueResult.[[Value]]).
+      if (!ReadableStreamControllerError(cx, unwrappedController, exn)) {
         return false;
       }
 
-      // Step b.ii.2: Return chunkSize.
+      // Step b.ii: Return result.
+      // Step e.ii: Return enqueueResult.
+      // (I.e., propagate the exception.)
       cx->setPendingException(exn);
       return false;
     }
   }
 
   // Step 6: Perform
   //         ! ReadableStreamDefaultControllerCallPullIfNeeded(controller).
   // Step 7: Return.
@@ -3059,18 +3035,20 @@ static MOZ_MUST_USE bool ReadableStreamC
     JSContext* cx, Handle<ReadableStreamController*> unwrappedController,
     HandleValue e) {
   MOZ_ASSERT(!cx->isExceptionPending());
   AssertSameCompartment(cx, e);
 
   // Step 1: Let stream be controller.[[controlledReadableStream]].
   Rooted<ReadableStream*> unwrappedStream(cx, unwrappedController->stream());
 
-  // Step 2: Assert: stream.[[state]] is "readable".
-  MOZ_ASSERT(unwrappedStream->readable());
+  // Step 2: If stream.[[state]] is not "readable", return.
+  if (!unwrappedStream->readable()) {
+    return true;
+  }
 
   // Step 3 of 3.12.10:
   // Perform ! ReadableByteStreamControllerClearPendingPullIntos(controller).
   if (unwrappedController->is<ReadableByteStreamController>()) {
     Rooted<ReadableByteStreamController*> unwrappedByteStreamController(
         cx, &unwrappedController->as<ReadableByteStreamController>());
     if (!ReadableByteStreamControllerClearPendingPullIntos(
             cx, unwrappedByteStreamController)) {
@@ -3083,33 +3061,16 @@ static MOZ_MUST_USE bool ReadableStreamC
     return false;
   }
 
   // Step 4 (or 5): Perform ! ReadableStreamError(stream, e).
   return ReadableStreamErrorInternal(cx, unwrappedStream, e);
 }
 
 /**
- * Streams spec, 3.9.7.
- *      ReadableStreamDefaultControllerErrorIfNeeded ( controller, e ) nothrow
- */
-static MOZ_MUST_USE bool ReadableStreamDefaultControllerErrorIfNeeded(
-    JSContext* cx, Handle<ReadableStreamDefaultController*> unwrappedController,
-    HandleValue e) {
-  MOZ_ASSERT(!cx->isExceptionPending());
-
-  // Step 1: If controller.[[controlledReadableStream]].[[state]] is "readable",
-  //         perform ! ReadableStreamDefaultControllerError(controller, e).
-  if (unwrappedController->stream()->readable()) {
-    return ReadableStreamControllerError(cx, unwrappedController, e);
-  }
-  return true;
-}
-
-/**
  * Streams spec, 3.9.8.
  *      ReadableStreamDefaultControllerGetDesiredSize ( controller )
  * Streams spec 3.12.14.
  *      ReadableByteStreamControllerGetDesiredSize ( controller )
  */
 static MOZ_MUST_USE double ReadableStreamControllerGetDesiredSizeUnchecked(
     ReadableStreamController* controller) {
   // Steps 1-4 done at callsites, so only assert that they have been done.
@@ -4821,25 +4782,16 @@ JS_PUBLIC_API bool JS::ReadableStreamErr
   cx->check(error);
 
   Rooted<ReadableStream*> unwrappedStream(
       cx, APIUnwrapAndDowncast<ReadableStream>(cx, streamObj));
   if (!unwrappedStream) {
     return false;
   }
 
-  // Step 3: If stream.[[state]] is not "readable", throw a TypeError exception.
-  if (!unwrappedStream->readable()) {
-    JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
-                              JSMSG_READABLESTREAMCONTROLLER_NOT_READABLE,
-                              "error");
-    return false;
-  }
-
-  // Step 4: Perform ! ReadableStreamDefaultControllerError(this, e).
   Rooted<ReadableStreamController*> unwrappedController(
       cx, unwrappedStream->controller());
   return ReadableStreamControllerError(cx, unwrappedController, error);
 }
 
 JS_PUBLIC_API bool JS::ReadableStreamReaderIsClosed(JSContext* cx,
                                                     HandleObject readerObj,
                                                     bool* result) {
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/web-platform/streams/readable-streams/bad-underlying-sources.js
@@ -0,0 +1,2 @@
+load(libdir + "web-platform-testharness.js");
+load_web_platform_test("streams/readable-streams/bad-underlying-sources.js");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/web-platform/streams/readable-streams/garbage-collection.js
@@ -0,0 +1,2 @@
+load(libdir + "web-platform-testharness.js");
+load_web_platform_test("streams/readable-streams/garbage-collection.js");
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.dedicatedworker.html.ini
+++ /dev/null
@@ -1,7 +0,0 @@
-[bad-underlying-sources.dedicatedworker.html]
-  [Underlying source: calling error twice should not throw]
-    expected: FAIL
-
-  [Underlying source: calling error after close should not throw]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.html.ini
+++ /dev/null
@@ -1,7 +0,0 @@
-[bad-underlying-sources.html]
-  [Underlying source: calling error twice should not throw]
-    expected: FAIL
-
-  [Underlying source: calling error after close should not throw]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.serviceworker.https.html.ini
+++ /dev/null
@@ -1,7 +0,0 @@
-[bad-underlying-sources.serviceworker.https.html]
-  [Underlying source: calling error twice should not throw]
-    expected: FAIL
-
-  [Underlying source: calling error after close should not throw]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/bad-underlying-sources.sharedworker.html.ini
+++ /dev/null
@@ -1,7 +0,0 @@
-[bad-underlying-sources.sharedworker.html]
-  [Underlying source: calling error twice should not throw]
-    expected: FAIL
-
-  [Underlying source: calling error after close should not throw]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/garbage-collection.dedicatedworker.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[garbage-collection.dedicatedworker.html]
-  [ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/garbage-collection.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[garbage-collection.html]
-  [ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/garbage-collection.serviceworker.https.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[garbage-collection.serviceworker.https.html]
-  [ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream]
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/streams/readable-streams/garbage-collection.sharedworker.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[garbage-collection.sharedworker.html]
-  [ReadableStreamController methods should continue working properly when scripts lose their reference to the readable stream]
-    expected: FAIL
-