Make IsClosed() and IsErrored() take an ExceptionState& argument

Modify IsClosed() and IsErrored() in blink::ReadableStreamOperations to
take an ExceptionState& parameters. Modify the wrappers in
BodyStreamBuffer to also take ExceptionState&, and make the callers
handle exceptions from them correctly.

Bug: 853189
Change-Id: I64a806571bd2c080de3ee2b5966cad6142ba0e15
Reviewed-on: https://chromium-review.googlesource.com/1116634
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571038}

third_party/blink/renderer/core/fetch/body_stream_buffer.cc

@@ -177,7 +177,11 @@ scoped_refptr<BlobDataHandle> BodyStreamBuffer::DrainAsBlobDataHandle(
     ExceptionState& exception_state) {
   DCHECK(!IsStreamLockedForDCheck());
   DCHECK(!IsStreamDisturbedForDCheck());
-  if (IsStreamClosed() || IsStreamErrored())
+  const base::Optional<bool> is_closed = IsStreamClosed(exception_state);
+  if (exception_state.HadException() || is_closed.value())
+    return nullptr;
+  const base::Optional<bool> is_errored = IsStreamErrored(exception_state);
+  if (exception_state.HadException() || is_errored.value())
     return nullptr;
 
   if (made_from_readable_stream_)
@@ -198,7 +202,11 @@ scoped_refptr<EncodedFormData> BodyStreamBuffer::DrainAsFormData(
     ExceptionState& exception_state) {
   DCHECK(!IsStreamLockedForDCheck());
   DCHECK(!IsStreamDisturbedForDCheck());
-  if (IsStreamClosed() || IsStreamErrored())
+  const base::Optional<bool> is_closed = IsStreamClosed(exception_state);
+  if (exception_state.HadException() || is_closed.value())
+    return nullptr;
+  const base::Optional<bool> is_errored = IsStreamErrored(exception_state);
+  if (exception_state.HadException() || is_errored.value())
     return nullptr;
 
   if (made_from_readable_stream_)
@@ -349,14 +357,16 @@ base::Optional<bool> BodyStreamBuffer::IsStreamReadable(
                                 exception_state);
 }
 
-bool BodyStreamBuffer::IsStreamClosed() {
-  return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsClosed,
-                                          true);
+base::Optional<bool> BodyStreamBuffer::IsStreamClosed(
+    ExceptionState& exception_state) {
+  return BooleanStreamOperation(ReadableStreamOperations::IsClosed,
+                                exception_state);
 }
 
-bool BodyStreamBuffer::IsStreamErrored() {
-  return BooleanStreamOperationOrFallback(ReadableStreamOperations::IsErrored,
-                                          true);
+base::Optional<bool> BodyStreamBuffer::IsStreamErrored(
+    ExceptionState& exception_state) {
+  return BooleanStreamOperation(ReadableStreamOperations::IsErrored,
+                                exception_state);
 }
 
 base::Optional<bool> BodyStreamBuffer::IsStreamLocked(
@@ -490,20 +500,6 @@ void BodyStreamBuffer::StopLoading() {
   loader_ = nullptr;
 }
 
-bool BodyStreamBuffer::BooleanStreamOperationOrFallback(
-    base::Optional<bool> (*predicate)(ScriptState*, ScriptValue),
-    bool fallback_value) {
-  if (stream_broken_)
-    return fallback_value;
-  ScriptState::Scope scope(script_state_.get());
-  const base::Optional<bool> result = predicate(script_state_.get(), Stream());
-  if (!result.has_value()) {
-    stream_broken_ = true;
-    return fallback_value;
-  }
-  return result.value();
-}
-
 base::Optional<bool> BodyStreamBuffer::BooleanStreamOperation(
     base::Optional<bool> (*predicate)(ScriptState*,
                                       ScriptValue,
@@ -556,21 +552,26 @@ BytesConsumer* BodyStreamBuffer::ReleaseHandle(
     }
     return new ReadableStreamBytesConsumer(script_state_.get(), reader);
   }
-  // We need to call these before calling closeAndLockAndDisturb.
-  const bool is_closed = IsStreamClosed();
-  const bool is_errored = IsStreamErrored();
+  // We need to call these before calling CloseAndLockAndDisturb.
+  const base::Optional<bool> is_closed = IsStreamClosed(exception_state);
+  if (exception_state.HadException())
+    return nullptr;
+  const base::Optional<bool> is_errored = IsStreamErrored(exception_state);
+  if (exception_state.HadException())
+    return nullptr;
+
   BytesConsumer* consumer = consumer_.Release();
 
   CloseAndLockAndDisturb(exception_state);
   if (exception_state.HadException())
     return nullptr;
 
-  if (is_closed) {
+  if (is_closed.value()) {
     // Note that the stream cannot be "draining", because it doesn't have
     // the internal buffer.
     return BytesConsumer::CreateClosed();
   }
-  if (is_errored)
+  if (is_errored.value())
     return BytesConsumer::CreateErrored(BytesConsumer::Error("error"));
 
   DCHECK(consumer);

third_party/blink/renderer/core/fetch/body_stream_buffer.h

@@ -63,8 +63,8 @@ class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
   String DebugName() const override { return "BodyStreamBuffer"; }
 
   base::Optional<bool> IsStreamReadable(ExceptionState&);
-  bool IsStreamClosed();
-  bool IsStreamErrored();
+  base::Optional<bool> IsStreamClosed(ExceptionState&);
+  base::Optional<bool> IsStreamErrored(ExceptionState&);
   base::Optional<bool> IsStreamLocked(ExceptionState&);
   bool IsStreamLockedForDCheck();
   base::Optional<bool> IsStreamDisturbed(ExceptionState&);
@@ -93,19 +93,10 @@ class CORE_EXPORT BodyStreamBuffer final : public UnderlyingSourceBase,
   void EndLoading();
   void StopLoading();
 
-  // Implementation of IsStream*() methods that don't take an ExceptionState&
-  // parameter. Delegates to |predicate|, one of the methods defined in
-  // ReadableStreamOperations. Sets |stream_broken_| if |predicate| returns
-  // empty. Returns |fallback_value| if |stream_broken_| is or becomes true.
-  bool BooleanStreamOperationOrFallback(
-      base::Optional<bool> (*predicate)(ScriptState*, ScriptValue),
-      bool fallback_value);
-
-  // Implementation of IsStream*() methods that take an ExceptionState&
-  // parameter. Delegates to |predicate|, one of the methods defined in
-  // ReadableStreamOperations. Sets |stream_broken_| and throws if |predicate|
-  // throws. Throws an exception if called when |stream_broken_| is already
-  // true.
+  // Implementation of IsStream*() methods. Delegates to |predicate|, one of the
+  // methods defined in ReadableStreamOperations. Sets |stream_broken_| and
+  // throws if |predicate| throws. Throws an exception if called when
+  // |stream_broken_| is already true.
   base::Optional<bool> BooleanStreamOperation(
       base::Optional<bool> (*predicate)(ScriptState*,
                                         ScriptValue,

third_party/blink/renderer/core/fetch/body_stream_buffer_test.cc

@@ -443,7 +443,7 @@ TEST_F(BodyStreamBufferTest, LoadClosedHandle) {
   BodyStreamBuffer* buffer = new BodyStreamBuffer(
       scope.GetScriptState(), BytesConsumer::CreateClosed(), nullptr);
 
-  EXPECT_TRUE(buffer->IsStreamClosed());
+  EXPECT_TRUE(buffer->IsStreamClosed(ASSERT_NO_EXCEPTION).value_or(false));
 
   EXPECT_FALSE(buffer->IsStreamLocked(ASSERT_NO_EXCEPTION).value_or(true));
   EXPECT_FALSE(buffer->IsStreamDisturbed(ASSERT_NO_EXCEPTION).value_or(true));
@@ -473,7 +473,7 @@ TEST_F(BodyStreamBufferTest, LoadErroredHandle) {
       scope.GetScriptState(),
       BytesConsumer::CreateErrored(BytesConsumer::Error()), nullptr);
 
-  EXPECT_TRUE(buffer->IsStreamErrored());
+  EXPECT_TRUE(buffer->IsStreamErrored(ASSERT_NO_EXCEPTION).value_or(false));
 
   EXPECT_FALSE(buffer->IsStreamLocked(ASSERT_NO_EXCEPTION).value_or(true));
   EXPECT_FALSE(buffer->IsStreamDisturbed(ASSERT_NO_EXCEPTION).value_or(true));

third_party/blink/renderer/core/streams/readable_stream_operations.cc

@@ -193,16 +193,20 @@ base::Optional<bool> ReadableStreamOperations::IsReadable(
 
 base::Optional<bool> ReadableStreamOperations::IsClosed(
     ScriptState* script_state,
-    ScriptValue stream) {
+    ScriptValue stream,
+    ExceptionState& exception_state) {
   DCHECK(IsReadableStreamForDCheck(script_state, stream));
-  return BooleanOperation(script_state, stream, "IsReadableStreamClosed");
+  return BooleanOperationWithRethrow(script_state, stream,
+                                     "IsReadableStreamClosed", exception_state);
 }
 
 base::Optional<bool> ReadableStreamOperations::IsErrored(
     ScriptState* script_state,
-    ScriptValue stream) {
+    ScriptValue stream,
+    ExceptionState& exception_state) {
   DCHECK(IsReadableStreamForDCheck(script_state, stream));
-  return BooleanOperation(script_state, stream, "IsReadableStreamErrored");
+  return BooleanOperationWithRethrow(
+      script_state, stream, "IsReadableStreamErrored", exception_state);
 }
 
 base::Optional<bool> ReadableStreamOperations::IsReadableStreamDefaultReader(

third_party/blink/renderer/core/streams/readable_stream_operations.h

@@ -99,11 +99,15 @@ class CORE_EXPORT ReadableStreamOperations {
 
   // IsReadableStreamClosed.
   // This function assumes |IsReadableStream(stream)|.
-  static base::Optional<bool> IsClosed(ScriptState*, ScriptValue stream);
+  static base::Optional<bool> IsClosed(ScriptState*,
+                                       ScriptValue stream,
+                                       ExceptionState& exception_state);
 
   // IsReadableStreamErrored.
   // This function assumes |IsReadableStream(stream)|.
-  static base::Optional<bool> IsErrored(ScriptState*, ScriptValue stream);
+  static base::Optional<bool> IsErrored(ScriptState*,
+                                        ScriptValue stream,
+                                        ExceptionState& exception_state);
 
   // IsReadableStreamDefaultReader.
   static base::Optional<bool> IsReadableStreamDefaultReader(ScriptState*,

third_party/blink/renderer/core/streams/readable_stream_operations_test.cc

@@ -394,14 +394,15 @@ TEST(ReadableStreamOperationsTest, IsClosed) {
   ASSERT_FALSE(closed.IsEmpty());
   ASSERT_FALSE(errored.IsEmpty());
 
-  EXPECT_FALSE(
-      ReadableStreamOperations::IsClosed(scope.GetScriptState(), readable)
-          .value_or(true));
-  EXPECT_TRUE(ReadableStreamOperations::IsClosed(scope.GetScriptState(), closed)
+  EXPECT_FALSE(ReadableStreamOperations::IsClosed(scope.GetScriptState(),
+                                                  readable, ASSERT_NO_EXCEPTION)
+                   .value_or(true));
+  EXPECT_TRUE(ReadableStreamOperations::IsClosed(scope.GetScriptState(), closed,
+                                                 ASSERT_NO_EXCEPTION)
                   .value_or(false));
-  EXPECT_FALSE(
-      ReadableStreamOperations::IsClosed(scope.GetScriptState(), errored)
-          .value_or(true));
+  EXPECT_FALSE(ReadableStreamOperations::IsClosed(scope.GetScriptState(),
+                                                  errored, ASSERT_NO_EXCEPTION)
+                   .value_or(true));
 }
 
 TEST(ReadableStreamOperationsTest, IsErrored) {
@@ -416,15 +417,15 @@ TEST(ReadableStreamOperationsTest, IsErrored) {
   ASSERT_FALSE(closed.IsEmpty());
   ASSERT_FALSE(errored.IsEmpty());
 
-  EXPECT_FALSE(
-      ReadableStreamOperations::IsErrored(scope.GetScriptState(), readable)
-          .value_or(true));
-  EXPECT_FALSE(
-      ReadableStreamOperations::IsErrored(scope.GetScriptState(), closed)
-          .value_or(true));
-  EXPECT_TRUE(
-      ReadableStreamOperations::IsErrored(scope.GetScriptState(), errored)
-          .value_or(false));
+  EXPECT_FALSE(ReadableStreamOperations::IsErrored(
+                   scope.GetScriptState(), readable, ASSERT_NO_EXCEPTION)
+                   .value_or(true));
+  EXPECT_FALSE(ReadableStreamOperations::IsErrored(scope.GetScriptState(),
+                                                   closed, ASSERT_NO_EXCEPTION)
+                   .value_or(true));
+  EXPECT_TRUE(ReadableStreamOperations::IsErrored(scope.GetScriptState(),
+                                                  errored, ASSERT_NO_EXCEPTION)
+                  .value_or(false));
 }
 
 TEST(ReadableStreamOperationsTest, Tee) {