Bug 1752695 - Reject pending reads when releasing reader. r=mgaudet D…

…ONTBUILD

This implements the changes from whatwg/streams#1168.

Differential Revision: https://phabricator.services.mozilla.com/D137382

dom/streams/ReadableByteStreamController.h

@@ -29,7 +29,10 @@
 
 namespace mozilla::dom {
 
-enum ReaderType { Default, BYOB };
+// https://streams.spec.whatwg.org/#pull-into-descriptor-reader-type
+// Indicates what type of readable stream reader initiated this request,
+// or None if the initiating reader was released.
+enum ReaderType { Default, BYOB, None };
 
 struct PullIntoDescriptor;
 struct ReadableByteStreamQueueEntry;
@@ -75,6 +78,7 @@ class ReadableByteStreamController final : public ReadableStreamController,
   MOZ_CAN_RUN_SCRIPT virtual void PullSteps(JSContext* aCx,
                                             ReadRequest* aReadRequest,
                                             ErrorResult& aRv) override;
+  virtual void ReleaseSteps() override;
 
   // Internal Slot Accessors
   Maybe<uint64_t> AutoAllocateChunkSize() { return mAutoAllocateChunkSize; }

dom/streams/ReadableStream.cpp

@@ -334,28 +334,28 @@ void ReadableStreamClose(JSContext* aCx, ReadableStream* aStream,
 
   // Step 6.
   if (reader->IsDefault()) {
-    // Step 6.1
-    ReadableStreamDefaultReader* defaultReader = reader->AsDefault();
-
+    // Step 6.1. Let readRequests be reader.[[readRequests]].
     // Move LinkedList out of DefaultReader onto stack to avoid the potential
     // for concurrent modification, which could invalidate the iterator.
     //
     // See https://bugs.chromium.org/p/chromium/issues/detail?id=1045874 as an
     // example of the kind of issue that could occur.
-    LinkedList<RefPtr<ReadRequest>> requestsToClose =
-        std::move(defaultReader->ReadRequests());
+    LinkedList<RefPtr<ReadRequest>> readRequests =
+        std::move(reader->AsDefault()->ReadRequests());
+
+    // Step 6.2. Set reader.[[readRequests]] to an empty list.
+    // Note: The std::move already cleared this anyway.
+    reader->AsDefault()->ReadRequests().clear();
 
+    // Step 6.3. For each readRequest of readRequests,
     // Drain the local list and destroy elements along the way.
-    while (RefPtr<ReadRequest> readRequest = requestsToClose.popFirst()) {
-      // Step 6.1.1.
+    while (RefPtr<ReadRequest> readRequest = readRequests.popFirst()) {
+      // Step 6.3.1. Perform readRequest’s close steps.
       readRequest->CloseSteps(aCx, aRv);
       if (aRv.Failed()) {
         return;
       }
     }
-
-    // Step 6.2 (this may be superflous post-std::move)
-    defaultReader->ReadRequests().clear();
   }
 }
 
@@ -400,19 +400,23 @@ already_AddRefed<Promise> ReadableStreamCancel(JSContext* aCx,
 
   // Step 6.
   if (reader && reader->IsBYOB()) {
-    // Step 6.1.
-    LinkedList<RefPtr<ReadIntoRequest>> readIntoRequestsToClose =
+    // Step 6.1. Let readIntoRequests be reader.[[readIntoRequests]].
+    LinkedList<RefPtr<ReadIntoRequest>> readIntoRequests =
         std::move(reader->AsBYOB()->ReadIntoRequests());
+
+    // Step 6.2. Set reader.[[readIntoRequests]] to an empty list.
+    // Note: The std::move already cleared this anyway.
+    reader->AsBYOB()->ReadIntoRequests().clear();
+
+    // Step 6.3. For each readIntoRequest of readIntoRequests,
     while (RefPtr<ReadIntoRequest> readIntoRequest =
-               readIntoRequestsToClose.popFirst()) {
+               readIntoRequests.popFirst()) {
+      // Step 6.3.1.Perform readIntoRequest’s close steps, given undefined.
       readIntoRequest->CloseSteps(aCx, JS::UndefinedHandleValue, aRv);
       if (aRv.Failed()) {
         return nullptr;
       }
     }
-
-    // Step 6.2.
-    reader->AsBYOB()->ReadIntoRequests().clear();
   }
 
   // Step 7.
@@ -561,39 +565,26 @@ void ReadableStreamError(JSContext* aCx, ReadableStream* aStream,
 
   // Step 8.
   if (reader->IsDefault()) {
-    // Step 8.1:
-    ReadableStreamDefaultReader* defaultReader = reader->AsDefault();
-
-    LinkedList<RefPtr<ReadRequest>> readRequestsToError =
-        std::move(defaultReader->ReadRequests());
-    while (RefPtr<ReadRequest> readRequest = readRequestsToError.popFirst()) {
-      readRequest->ErrorSteps(aCx, aValue, aRv);
-      if (aRv.Failed()) {
-        return;
-      }
+    // Step 8.1. Perform ! ReadableStreamDefaultReaderErrorReadRequests(reader,
+    // e).
+    RefPtr<ReadableStreamDefaultReader> defaultReader = reader->AsDefault();
+    ReadableStreamDefaultReaderErrorReadRequests(aCx, defaultReader, aValue,
+                                                 aRv);
+    if (aRv.Failed()) {
+      return;
     }
-
-    // Step 8.2
-    defaultReader->ReadRequests().clear();
   } else {
-    // Step 9.
-    // Step 9.1.
+    // Step 9. Otherwise,
+    // Step 9.1. Assert: reader implements ReadableStreamBYOBReader.
     MOZ_ASSERT(reader->IsBYOB());
-    ReadableStreamBYOBReader* byobReader = reader->AsBYOB();
-    // Step 9.2.
-    LinkedList<RefPtr<ReadIntoRequest>> requestsToError =
-        std::move(byobReader->ReadIntoRequests());
 
-    while (RefPtr<ReadIntoRequest> readIntoRequest =
-               requestsToError.popFirst()) {
-      readIntoRequest->ErrorSteps(aCx, aValue, aRv);
-      if (aRv.Failed()) {
-        return;
-      }
+    // Step 9.2. Perform ! ReadableStreamBYOBReaderErrorReadIntoRequests(reader,
+    // e).
+    RefPtr<ReadableStreamBYOBReader> byobReader = reader->AsBYOB();
+    ReadableStreamBYOBReaderErrorReadIntoRequests(aCx, byobReader, aValue, aRv);
+    if (aRv.Failed()) {
+      return;
     }
-
-    // Step 9.3
-    byobReader->ReadIntoRequests().clear();
   }
 
   // Not in Specification: Allow notifying native underlying sources that a

dom/streams/ReadableStreamBYOBReader.cpp

@@ -271,18 +271,65 @@ already_AddRefed<Promise> ReadableStreamBYOBReader::Read(
   return promise.forget();
 }
 
+// https://streams.spec.whatwg.org/#abstract-opdef-readablestreambyobreadererrorreadintorequests
+void ReadableStreamBYOBReaderErrorReadIntoRequests(
+    JSContext* aCx, ReadableStreamBYOBReader* aReader,
+    JS::Handle<JS::Value> aError, ErrorResult& aRv) {
+  // Step 1. Let readIntoRequests be reader.[[readIntoRequests]].
+  LinkedList<RefPtr<ReadIntoRequest>> readIntoRequests =
+      std::move(aReader->ReadIntoRequests());
+
+  // Step 2. Set reader.[[readIntoRequests]] to a new empty list.
+  // Note: The std::move already cleared this anyway.
+  aReader->ReadIntoRequests().clear();
+
+  // Step 3. For each readIntoRequest of readIntoRequests,
+  while (RefPtr<ReadIntoRequest> readIntoRequest =
+             readIntoRequests.popFirst()) {
+    // Step 3.1. Perform readIntoRequest’s error steps, given e.
+    readIntoRequest->ErrorSteps(aCx, aError, aRv);
+    if (aRv.Failed()) {
+      return;
+    }
+  }
+}
+
+// https://streams.spec.whatwg.org/#abstract-opdef-readablestreambyobreaderrelease
+void ReadableStreamBYOBReaderRelease(JSContext* aCx,
+                                     ReadableStreamBYOBReader* aReader,
+                                     ErrorResult& aRv) {
+  // Step 1. Perform ! ReadableStreamReaderGenericRelease(reader).
+  ReadableStreamReaderGenericRelease(aReader, aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
+  // Step 2. Let e be a new TypeError exception.
+  ErrorResult rv;
+  rv.ThrowTypeError("Releasing lock");
+  JS::Rooted<JS::Value> error(aCx);
+  MOZ_ALWAYS_TRUE(ToJSValue(aCx, std::move(rv), &error));
+
+  // Step 3. Perform ! ReadableStreamBYOBReaderErrorReadIntoRequests(reader, e).
+  ReadableStreamBYOBReaderErrorReadIntoRequests(aCx, aReader, error, aRv);
+}
+
 // https://streams.spec.whatwg.org/#byob-reader-release-lock
 void ReadableStreamBYOBReader::ReleaseLock(ErrorResult& aRv) {
-  if (!GetStream()) {
+  // Step 1. If this.[[stream]] is undefined, return.
+  if (!mStream) {
     return;
   }
 
-  if (!ReadIntoRequests().isEmpty()) {
-    aRv.ThrowTypeError("ReadIntoRequests not empty");
-    return;
+  AutoJSAPI jsapi;
+  if (!jsapi.Init(mGlobal)) {
+    return aRv.ThrowUnknownError("Internal error");
   }
+  JSContext* cx = jsapi.cx();
 
-  ReadableStreamReaderGenericRelease(this, aRv);
+  // Step 2. Perform ! ReadableStreamBYOBReaderRelease(this).
+  RefPtr<ReadableStreamBYOBReader> thisRefPtr = this;
+  ReadableStreamBYOBReaderRelease(cx, thisRefPtr, aRv);
 }
 
 // https://streams.spec.whatwg.org/#acquire-readable-stream-byob-reader

dom/streams/ReadableStreamBYOBReader.h

@@ -77,6 +77,14 @@ MOZ_CAN_RUN_SCRIPT void ReadableStreamBYOBReaderRead(
     JSContext* aCx, ReadableStreamBYOBReader* aReader, JS::HandleObject aView,
     ReadIntoRequest* aReadIntoRequest, ErrorResult& aRv);
 
+void ReadableStreamBYOBReaderErrorReadIntoRequests(
+    JSContext* aCx, ReadableStreamBYOBReader* aReader,
+    JS::Handle<JS::Value> aError, ErrorResult& aRv);
+
+void ReadableStreamBYOBReaderRelease(JSContext* aCx,
+                                     ReadableStreamBYOBReader* aReader,
+                                     ErrorResult& aRv);
+
 }  // namespace dom
 }  // namespace mozilla
 

dom/streams/ReadableStreamController.h

@@ -39,6 +39,9 @@ class ReadableStreamController : public nsISupports {
   virtual void PullSteps(JSContext* aCx, ReadRequest* aReadRequest,
                          ErrorResult& aRv) = 0;
 
+  // No JS implementable UnderlyingSource callback exists for this.
+  virtual void ReleaseSteps() = 0;
+
  protected:
   nsCOMPtr<nsIGlobalObject> mGlobal;
   virtual ~ReadableStreamController() = default;

dom/streams/ReadableStreamDefaultController.cpp

@@ -715,5 +715,10 @@ void ReadableStreamDefaultController::PullSteps(JSContext* aCx,
   }
 }
 
+// https://streams.spec.whatwg.org/#abstract-opdef-readablestreamdefaultcontroller-releasesteps
+void ReadableStreamDefaultController::ReleaseSteps() {
+  // Step 1. Return.
+}
+
 }  // namespace dom
 }  // namespace mozilla

dom/streams/ReadableStreamDefaultController.h

@@ -73,6 +73,8 @@ class ReadableStreamDefaultController final : public ReadableStreamController,
                                             ReadRequest* aReadRequest,
                                             ErrorResult& aRv) override;
 
+  virtual void ReleaseSteps() override;
+
   // Internal Slot Accessors
   UnderlyingSourceCancelCallbackHelper* GetCancelAlgorithm() const {
     return mCancelAlgorithm;

dom/streams/ReadableStreamDefaultReader.cpp

@@ -267,16 +267,23 @@ already_AddRefed<Promise> ReadableStreamDefaultReader::Read(JSContext* aCx,
 // https://streams.spec.whatwg.org/#readable-stream-reader-generic-release
 void ReadableStreamReaderGenericRelease(ReadableStreamGenericReader* aReader,
                                         ErrorResult& aRv) {
-  // Step 1.
-  MOZ_ASSERT(aReader->GetStream());
-  // Step 2.
-  MOZ_ASSERT(aReader->GetStream()->GetReader() == aReader);
-  // Step 3.
+  // Step 1. Let stream be reader.[[stream]].
+  RefPtr<ReadableStream> stream = aReader->GetStream();
+
+  // Step 2. Assert: stream is not undefined.
+  MOZ_ASSERT(stream);
+
+  // Step 3. Assert: stream.[[reader]] is reader.
+  MOZ_ASSERT(stream->GetReader() == aReader);
+
+  // Step 4. If stream.[[state]] is "readable", reject reader.[[closedPromise]]
+  // with a TypeError exception.
   if (aReader->GetStream()->State() == ReadableStream::ReaderState::Readable) {
     aReader->ClosedPromise()->MaybeRejectWithTypeError(
         "Releasing lock on readable stream");
   } else {
-    // Step 4.
+    // Step 5. Otherwise, set reader.[[closedPromise]] to a promise rejected
+    // with a TypeError exception.
     RefPtr<Promise> promise = Promise::Create(aReader->GetParentObject(), aRv);
     if (aRv.Failed()) {
       return;
@@ -285,32 +292,77 @@ void ReadableStreamReaderGenericRelease(ReadableStreamGenericReader* aReader,
     aReader->SetClosedPromise(promise.forget());
   }
 
-  // Step 5.
+  // Step 6. Set reader.[[closedPromise]].[[PromiseIsHandled]] to true.
   aReader->ClosedPromise()->SetSettledPromiseIsHandled();
 
-  // Step 6.
-  aReader->GetStream()->SetReader(nullptr);
+  // Step 7. Perform ! stream.[[controller]].[[ReleaseSteps]]().
+  stream->Controller()->ReleaseSteps();
+
+  // Step 8. Set stream.[[reader]] to undefined.
+  stream->SetReader(nullptr);
 
-  // Step 7.
+  // Step 9. Set reader.[[stream]] to undefined.
   aReader->SetStream(nullptr);
 }
 
+// https://streams.spec.whatwg.org/#abstract-opdef-readablestreamdefaultreadererrorreadrequests
+void ReadableStreamDefaultReaderErrorReadRequests(
+    JSContext* aCx, ReadableStreamDefaultReader* aReader,
+    JS::Handle<JS::Value> aError, ErrorResult& aRv) {
+  // Step 1. Let readRequests be reader.[[readRequests]].
+  LinkedList<RefPtr<ReadRequest>> readRequests =
+      std::move(aReader->ReadRequests());
+
+  // Step 2. Set reader.[[readRequests]] to a new empty list.
+  // Note: The std::move already cleared this anyway.
+  aReader->ReadRequests().clear();
+
+  // Step 3. For each readRequest of readRequests,
+  while (RefPtr<ReadRequest> readRequest = readRequests.popFirst()) {
+    // Step 3.1. Perform readRequest’s error steps, given e.
+    readRequest->ErrorSteps(aCx, aError, aRv);
+    if (aRv.Failed()) {
+      return;
+    }
+  }
+}
+
+// https://streams.spec.whatwg.org/#abstract-opdef-readablestreamdefaultreaderrelease
+void ReadableStreamDefaultReaderRelease(JSContext* aCx,
+                                        ReadableStreamDefaultReader* aReader,
+                                        ErrorResult& aRv) {
+  // Step 1. Perform ! ReadableStreamReaderGenericRelease(reader).
+  ReadableStreamReaderGenericRelease(aReader, aRv);
+  if (aRv.Failed()) {
+    return;
+  }
+
+  // Step 2. Let e be a new TypeError exception.
+  ErrorResult rv;
+  rv.ThrowTypeError("Releasing lock");
+  JS::Rooted<JS::Value> error(aCx);
+  MOZ_ALWAYS_TRUE(ToJSValue(aCx, std::move(rv), &error));
+
+  // Step 3. Perform ! ReadableStreamDefaultReaderErrorReadRequests(reader, e).
+  ReadableStreamDefaultReaderErrorReadRequests(aCx, aReader, error, aRv);
+}
+
 // https://streams.spec.whatwg.org/#default-reader-release-lock
 void ReadableStreamDefaultReader::ReleaseLock(ErrorResult& aRv) {
-  // Step 1.
+  // Step 1. If this.[[stream]] is undefined, return.
   if (!mStream) {
     return;
   }
 
-  // Step 2.
-  if (!mReadRequests.isEmpty()) {
-    aRv.ThrowTypeError(
-        "Cannot release lock while read requests are still pending.");
-    return;
+  AutoJSAPI jsapi;
+  if (!jsapi.Init(mGlobal)) {
+    return aRv.ThrowUnknownError("Internal error");
   }
+  JSContext* cx = jsapi.cx();
 
-  // Step 3.
-  ReadableStreamReaderGenericRelease(this, aRv);
+  // Step 2. Perform ! ReadableStreamDefaultReaderRelease(this).
+  RefPtr<ReadableStreamDefaultReader> thisRefPtr = this;
+  ReadableStreamDefaultReaderRelease(cx, thisRefPtr, aRv);
 }
 
 // https://streams.spec.whatwg.org/#generic-reader-closed

dom/streams/ReadableStreamDefaultReader.h

@@ -95,6 +95,14 @@ extern void SetUpReadableStreamDefaultReader(
     JSContext* aCx, ReadableStreamDefaultReader* aReader,
     ReadableStream* aStream, ErrorResult& aRv);
 
+void ReadableStreamDefaultReaderErrorReadRequests(
+    JSContext* aCx, ReadableStreamDefaultReader* aReader,
+    JS::Handle<JS::Value> aError, ErrorResult& aRv);
+
+void ReadableStreamDefaultReaderRelease(JSContext* aCx,
+                                        ReadableStreamDefaultReader* aReader,
+                                        ErrorResult& aRv);
+
 }  // namespace dom
 }  // namespace mozilla