Bug 1507950 - Allow calling controller.error() when the stream is not readable. r=arai
☠☠ backed out by 7eb42458e2d8 ☠ ☠
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 07 Dec 2018 20:03:46 +0000
changeset 508850 bc83a2fe5c178f5f555b5ff4566b387a028eccdc
parent 508849 345ad3e746e8e50d2365f6dac8ef5be6383d1afd
child 508851 47035a39723978394f8df24652bb198eed362540
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1507950
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 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
-