Make IsReadableStreamDefaultReader() take an ExceptionState&

Also remove the unsafe version of IsReadableStream().

Remove BooleanOperation() helper from readable_stream_operations.cc as
it is no longer used.

Modify the tests to pass an ExceptionState& argument to
IsReadableStream() and IsReadableStreamDefaultReader().

Bug: 853189
Change-Id: I26393861760e3bdab6991e522565067aabae5fb4
Reviewed-on: https://chromium-review.googlesource.com/1118089Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571086}

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

@@ -17,20 +17,6 @@ namespace blink {
 
 namespace {
 
-base::Optional<bool> BooleanOperation(ScriptState* script_state,
-                                      ScriptValue value,
-                                      const char* operation) {
-  v8::Local<v8::Value> args[] = {value.V8Value()};
-  v8::Local<v8::Value> result_value;
-  bool result_bool = false;
-  if (V8ScriptRunner::CallExtra(script_state, operation, args)
-          .ToLocal(&result_value) &&
-      result_value->BooleanValue(script_state->GetContext()).To(&result_bool)) {
-    return result_bool;
-  }
-  return base::nullopt;
-}
-
 base::Optional<bool> BooleanOperationWithRethrow(
     ScriptState* script_state,
     ScriptValue value,
@@ -66,15 +52,18 @@ bool BooleanOperationForDCheck(ScriptState* script_state,
                                ScriptValue value,
                                const char* operation,
                                bool fallback_value) {
+  v8::Local<v8::Value> args[] = {value.V8Value()};
+  v8::Local<v8::Value> result_value;
+  bool result_bool = false;
   v8::TryCatch block(script_state->GetIsolate());
-  base::Optional<bool> result =
-      BooleanOperation(script_state, value, operation);
-  if (block.HasCaught()) {
-    DCHECK(!result.has_value());
-    return fallback_value;
+  if (V8ScriptRunner::CallExtra(script_state, operation, args)
+          .ToLocal(&result_value) &&
+      result_value->BooleanValue(script_state->GetContext()).To(&result_bool)) {
+    DCHECK(!block.HasCaught());
+    return result_bool;
   }
-  DCHECK(result.has_value());
-  return result.value();
+  DCHECK(block.HasCaught());
+  return fallback_value;
 }
 
 // Performs IsReadableStreamDefaultReader(value), catching exceptions. Should
@@ -129,12 +118,6 @@ ScriptValue ReadableStreamOperations::GetReader(ScriptState* script_state,
   return result;
 }
 
-base::Optional<bool> ReadableStreamOperations::IsReadableStream(
-    ScriptState* script_state,
-    ScriptValue value) {
-  return BooleanOperation(script_state, value, "IsReadableStream");
-}
-
 base::Optional<bool> ReadableStreamOperations::IsReadableStream(
     ScriptState* script_state,
     ScriptValue value,
@@ -211,8 +194,10 @@ base::Optional<bool> ReadableStreamOperations::IsErrored(
 
 base::Optional<bool> ReadableStreamOperations::IsReadableStreamDefaultReader(
     ScriptState* script_state,
-    ScriptValue value) {
-  return BooleanOperation(script_state, value, "IsReadableStreamDefaultReader");
+    ScriptValue value,
+    ExceptionState& exception_state) {
+  return BooleanOperationWithRethrow(
+      script_state, value, "IsReadableStreamDefaultReader", exception_state);
 }
 
 ScriptPromise ReadableStreamOperations::DefaultReaderRead(

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

@@ -55,9 +55,6 @@ class CORE_EXPORT ReadableStreamOperations {
   // This function assumes |IsReadableStream(stream)|.
   static ScriptValue GetReader(ScriptState*, ScriptValue stream);
 
-  // IsReadableStream. Exceptions are not caught.
-  static base::Optional<bool> IsReadableStream(ScriptState*, ScriptValue);
-
   // IsReadableStream, exception-catching version. Exceptions will be passed to
   // |exception_state|.
   static base::Optional<bool> IsReadableStream(ScriptState*,
@@ -110,8 +107,10 @@ class CORE_EXPORT ReadableStreamOperations {
                                         ExceptionState& exception_state);
 
   // IsReadableStreamDefaultReader.
-  static base::Optional<bool> IsReadableStreamDefaultReader(ScriptState*,
-                                                            ScriptValue);
+  static base::Optional<bool> IsReadableStreamDefaultReader(
+      ScriptState*,
+      ScriptValue,
+      ExceptionState& exception_state);
 
   // ReadableStreamDefaultReaderRead
   // This function assumes |IsReadableStreamDefaultReader(reader)|.

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

@@ -122,22 +122,25 @@ TEST(ReadableStreamOperationsTest, IsReadableStream) {
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
                    scope.GetScriptState(),
                    ScriptValue(scope.GetScriptState(),
-                               v8::Undefined(scope.GetIsolate())))
+                               v8::Undefined(scope.GetIsolate())),
+                   ASSERT_NO_EXCEPTION)
                    .value_or(true));
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
                    scope.GetScriptState(),
-                   ScriptValue::CreateNull(scope.GetScriptState()))
+                   ScriptValue::CreateNull(scope.GetScriptState()),
+                   ASSERT_NO_EXCEPTION)
                    .value_or(true));
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
                    scope.GetScriptState(),
                    ScriptValue(scope.GetScriptState(),
-                               v8::Object::New(scope.GetIsolate())))
+                               v8::Object::New(scope.GetIsolate())),
+                   ASSERT_NO_EXCEPTION)
                    .value_or(true));
   ScriptValue stream = EvalWithPrintingError(&scope, "new ReadableStream()");
   EXPECT_FALSE(stream.IsEmpty());
-  EXPECT_TRUE(
-      ReadableStreamOperations::IsReadableStream(scope.GetScriptState(), stream)
-          .value_or(false));
+  EXPECT_TRUE(ReadableStreamOperations::IsReadableStream(
+                  scope.GetScriptState(), stream, ASSERT_NO_EXCEPTION)
+                  .value_or(false));
 }
 
 TEST(ReadableStreamOperationsTest, IsReadableStreamDefaultReaderInvalid) {
@@ -146,22 +149,25 @@ TEST(ReadableStreamOperationsTest, IsReadableStreamDefaultReaderInvalid) {
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
                    scope.GetScriptState(),
                    ScriptValue(scope.GetScriptState(),
-                               v8::Undefined(scope.GetIsolate())))
+                               v8::Undefined(scope.GetIsolate())),
+                   ASSERT_NO_EXCEPTION)
                    .value_or(true));
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
                    scope.GetScriptState(),
-                   ScriptValue::CreateNull(scope.GetScriptState()))
+                   ScriptValue::CreateNull(scope.GetScriptState()),
+                   ASSERT_NO_EXCEPTION)
                    .value_or(true));
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
                    scope.GetScriptState(),
                    ScriptValue(scope.GetScriptState(),
-                               v8::Object::New(scope.GetIsolate())))
+                               v8::Object::New(scope.GetIsolate())),
+                   ASSERT_NO_EXCEPTION)
                    .value_or(true));
   ScriptValue stream = EvalWithPrintingError(&scope, "new ReadableStream()");
   ASSERT_FALSE(stream.IsEmpty());
 
   EXPECT_FALSE(ReadableStreamOperations::IsReadableStreamDefaultReader(
-                   scope.GetScriptState(), stream)
+                   scope.GetScriptState(), stream, ASSERT_NO_EXCEPTION)
                    .value_or(true));
 }
 
@@ -181,11 +187,11 @@ TEST(ReadableStreamOperationsTest, GetReader) {
                   .value_or(false));
   ASSERT_FALSE(reader.IsEmpty());
 
-  EXPECT_FALSE(
-      ReadableStreamOperations::IsReadableStream(scope.GetScriptState(), reader)
-          .value_or(true));
+  EXPECT_FALSE(ReadableStreamOperations::IsReadableStream(
+                   scope.GetScriptState(), reader, ASSERT_NO_EXCEPTION)
+                   .value_or(true));
   EXPECT_TRUE(ReadableStreamOperations::IsReadableStreamDefaultReader(
-                  scope.GetScriptState(), reader)
+                  scope.GetScriptState(), reader, ASSERT_NO_EXCEPTION)
                   .value_or(false));
 
   // Already locked!
@@ -221,7 +227,7 @@ TEST(ReadableStreamOperationsTest, Read) {
                             "new ReadableStream({start}).getReader()");
   EXPECT_FALSE(reader.IsEmpty());
   ASSERT_TRUE(ReadableStreamOperations::IsReadableStreamDefaultReader(
-                  scope.GetScriptState(), reader)
+                  scope.GetScriptState(), reader, ASSERT_NO_EXCEPTION)
                   .value_or(false));
 
   Iteration* it1 = new Iteration();