Add StreamPipeOptions to Readable Streams

In an effort to change the blink implementation of readable
streams to better match the Web IDL descriptions in the Streams
API Standard, this CL adds the StreamPipeOptions dictionary [1]
to the ReadableStream class.

[1] https://streams.spec.whatwg.org/#dictdef-streampipeoptions

Bug: 1093862
Change-Id: Ic03fff0d645786dd48c4d8b7cd8682e0442bff0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2488329
Commit-Queue: Nidhi Jaju <nidhijaju@google.com>
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821113}

third_party/blink/renderer/bindings/generated_in_core.gni

@@ -295,6 +295,8 @@ generated_dictionary_sources_in_core = [
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_security_policy_violation_event_init.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_shadow_root_init.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_shadow_root_init.h",
+  "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_stream_pipe_options.h",
+  "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_stream_pipe_options.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_submit_event_init.cc",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_submit_event_init.h",
   "$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_text_format_update_event_init.cc",

third_party/blink/renderer/bindings/idl_in_core.gni

@@ -486,6 +486,7 @@ static_idl_files_in_core = get_path_info(
           "//third_party/blink/renderer/core/streams/readable_stream.idl",
           "//third_party/blink/renderer/core/streams/readable_stream_default_controller.idl",
           "//third_party/blink/renderer/core/streams/readable_stream_default_reader.idl",
+          "//third_party/blink/renderer/core/streams/stream_pipe_options.idl",
           "//third_party/blink/renderer/core/streams/transform_stream.idl",
           "//third_party/blink/renderer/core/streams/transform_stream_default_controller.idl",
           "//third_party/blink/renderer/core/streams/underlying_sink_base.idl",

third_party/blink/renderer/core/core_idl_files.gni

@@ -736,6 +736,7 @@ core_dictionary_idl_files =
                     "mojo/test/mojo_interface_request_event_init.idl",
                     "page/scrolling/scroll_state_init.idl",
                     "resize_observer/resize_observer_options.idl",
+                    "streams/stream_pipe_options.idl",
                     "streams/queuing_strategy_init.idl",
                     "timing/measure_memory/measure_memory.idl",
                     "timing/measure_memory/measure_memory_breakdown.idl",

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

@@ -19,6 +19,7 @@
 #include "third_party/blink/renderer/core/streams/readable_stream_default_controller.h"
 #include "third_party/blink/renderer/core/streams/readable_stream_reader.h"
 #include "third_party/blink/renderer/core/streams/stream_algorithms.h"
+#include "third_party/blink/renderer/core/streams/stream_pipe_options.h"
 #include "third_party/blink/renderer/core/streams/stream_promise_resolver.h"
 #include "third_party/blink/renderer/core/streams/transferable_streams.h"
 #include "third_party/blink/renderer/core/streams/underlying_source_base.h"
@@ -42,62 +43,14 @@ namespace blink {
 ReadableStream::PipeOptions::PipeOptions()
     : prevent_close_(false), prevent_abort_(false), prevent_cancel_(false) {}
 
-ReadableStream::PipeOptions::PipeOptions(ScriptState* script_state,
-                                         ScriptValue options,
-                                         ExceptionState& exception_state) {
-  auto* isolate = script_state->GetIsolate();
-  v8::TryCatch block(isolate);
-  v8::Local<v8::Value> options_value = options.V8Value();
-  v8::Local<v8::Object> options_object;
-  if (options_value->IsUndefined()) {
-    options_object = v8::Object::New(isolate);
-  } else if (!options_value->ToObject(script_state->GetContext())
-                  .ToLocal(&options_object)) {
-    exception_state.RethrowV8Exception(block.Exception());
-    return;
-  }
-
-  // 4. Set preventClose to ! ToBoolean(preventClose), set preventAbort to !
-  // ToBoolean(preventAbort), and set preventCancel to !
-  // ToBoolean(preventCancel).
-  prevent_close_ =
-      GetBoolean(script_state, options_object, "preventClose", exception_state);
-  if (exception_state.HadException()) {
-    return;
-  }
-
-  prevent_abort_ =
-      GetBoolean(script_state, options_object, "preventAbort", exception_state);
-  if (exception_state.HadException()) {
-    return;
-  }
-
-  prevent_cancel_ = GetBoolean(script_state, options_object, "preventCancel",
-                               exception_state);
-  if (exception_state.HadException()) {
-    return;
-  }
-
-  v8::Local<v8::Value> signal_value;
-  if (!options_object
-           ->Get(script_state->GetContext(), V8AtomicString(isolate, "signal"))
-           .ToLocal(&signal_value)) {
-    exception_state.RethrowV8Exception(block.Exception());
-    return;
-  }
-
-  // 5. If signal is not undefined, and signal is not an instance of the
-  // AbortSignal interface, throw a TypeError exception.
-  if (signal_value->IsUndefined())
-    return;
-
-  signal_ = V8AbortSignal::ToImplWithTypeCheck(isolate, signal_value);
-  if (!signal_) {
-    exception_state.ThrowTypeError(
-        "'signal' must be an AbortSignal object or undefined");
-    return;
-  }
-}
+ReadableStream::PipeOptions::PipeOptions(const StreamPipeOptions* options)
+    : prevent_close_(options->hasPreventClose() ? options->preventClose()
+                                                : false),
+      prevent_abort_(options->hasPreventAbort() ? options->preventAbort()
+                                                : false),
+      prevent_cancel_(options->hasPreventCancel() ? options->preventCancel()
+                                                  : false),
+      signal_(options->hasSignal() ? options->signal() : nullptr) {}
 
 void ReadableStream::PipeOptions::Trace(Visitor* visitor) const {
   visitor->Trace(signal_);
@@ -1280,15 +1233,13 @@ ScriptValue ReadableStream::pipeThrough(ScriptState* script_state,
                                         ScriptValue transform_stream,
                                         ExceptionState& exception_state) {
   return pipeThrough(script_state, transform_stream,
-                     ScriptValue(script_state->GetIsolate(),
-                                 v8::Undefined(script_state->GetIsolate())),
-                     exception_state);
+                     StreamPipeOptions::Create(), exception_state);
 }
 
 // https://streams.spec.whatwg.org/#rs-pipe-through
 ScriptValue ReadableStream::pipeThrough(ScriptState* script_state,
                                         ScriptValue transform_stream,
-                                        ScriptValue options,
+                                        const StreamPipeOptions* options,
                                         ExceptionState& exception_state) {
   // https://streams.spec.whatwg.org/#rs-pipe-through
   // The first part of this function implements the unpacking of the {readable,
@@ -1341,58 +1292,48 @@ ScriptValue ReadableStream::pipeThrough(ScriptState* script_state,
     return ScriptValue();
   }
 
-  // 4. Set preventClose to ! ToBoolean(preventClose), set preventAbort to !
-  //    ToBoolean(preventAbort), and set preventCancel to !
-  //    ToBoolean(preventCancel).
-
-  // 5. If signal is not undefined, and signal is not an instance of the
+  // 4. If signal is not undefined, and signal is not an instance of the
   //    AbortSignal interface, throw a TypeError exception.
-  auto* pipe_options =
-      MakeGarbageCollected<PipeOptions>(script_state, options, exception_state);
-  if (exception_state.HadException()) {
-    return ScriptValue();
-  }
+  auto* pipe_options = MakeGarbageCollected<PipeOptions>(options);
 
-  // 6. If ! IsReadableStreamLocked(*this*) is *true*, throw a *TypeError*
+  // 5. If ! IsReadableStreamLocked(*this*) is *true*, throw a *TypeError*
   //    exception.
   if (IsLocked(this)) {
     exception_state.ThrowTypeError("Cannot pipe a locked stream");
     return ScriptValue();
   }
 
-  // 7. If ! IsWritableStreamLocked(_writable_) is *true*, throw a *TypeError*
+  // 6. If ! IsWritableStreamLocked(_writable_) is *true*, throw a *TypeError*
   //    exception.
   if (WritableStream::IsLocked(writable_stream)) {
     exception_state.ThrowTypeError(kWritableIsLocked);
     return ScriptValue();
   }
 
-  // 8. Let _promise_ be ! ReadableStreamPipeTo(*this*, _writable_,
+  // 7. Let _promise_ be ! ReadableStreamPipeTo(*this*, _writable_,
   //    _preventClose_, _preventAbort_, _preventCancel_,
   //   _signal_).
 
   ScriptPromise promise =
       PipeTo(script_state, this, writable_stream, pipe_options);
 
-  // 9. Set _promise_.[[PromiseIsHandled]] to *true*.
+  // 8. Set _promise_.[[PromiseIsHandled]] to *true*.
   promise.MarkAsHandled();
 
-  // 10. Return _readable_.
+  // 9. Return _readable_.
   return ScriptValue(script_state->GetIsolate(), readable);
 }
 
 ScriptPromise ReadableStream::pipeTo(ScriptState* script_state,
                                      ScriptValue destination,
                                      ExceptionState& exception_state) {
-  return pipeTo(script_state, destination,
-                ScriptValue(script_state->GetIsolate(),
-                            v8::Undefined(script_state->GetIsolate())),
+  return pipeTo(script_state, destination, StreamPipeOptions::Create(),
                 exception_state);
 }
 
 ScriptPromise ReadableStream::pipeTo(ScriptState* script_state,
                                      ScriptValue destination_value,
-                                     ScriptValue options,
+                                     const StreamPipeOptions* options,
                                      ExceptionState& exception_state) {
   // https://streams.spec.whatwg.org/#rs-pipe-to
   // 2. If ! IsWritableStream(dest) is false, return a promise rejected with a
@@ -1406,26 +1347,19 @@ ScriptPromise ReadableStream::pipeTo(ScriptState* script_state,
     return ScriptPromise();
   }
 
-  // 3. Set preventClose to ! ToBoolean(preventClose), set preventAbort to !
-  //    ToBoolean(preventAbort), and set preventCancel to !
-  //    ToBoolean(preventCancel).
-  // 4. If signal is not undefined, and signal is not an instance of the
+  // 3. If signal is not undefined, and signal is not an instance of the
   //    AbortSignal interface, return a promise rejected with a TypeError
   //    exception.
-  auto* pipe_options =
-      MakeGarbageCollected<PipeOptions>(script_state, options, exception_state);
-  if (exception_state.HadException()) {
-    return ScriptPromise();
-  }
+  auto* pipe_options = MakeGarbageCollected<PipeOptions>(options);
 
-  // 5. If ! IsReadableStreamLocked(this) is true, return a promise rejected
+  // 4. If ! IsReadableStreamLocked(this) is true, return a promise rejected
   // with a TypeError exception.
   if (IsLocked(this)) {
     exception_state.ThrowTypeError("Cannot pipe a locked stream");
     return ScriptPromise();
   }
 
-  // 6. If ! IsWritableStreamLocked(dest) is true, return a promise rejected
+  // 5. If ! IsWritableStreamLocked(dest) is true, return a promise rejected
   // with a TypeError exception.
   if (WritableStream::IsLocked(destination)) {
     exception_state.ThrowTypeError("Cannot pipe to a locked stream");

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

@@ -25,6 +25,7 @@ class ScriptPromise;
 class ScriptState;
 class StrategySizeAlgorithm;
 class StreamAlgorithm;
+class StreamPipeOptions;
 class StreamPromiseResolver;
 class StreamStartAlgorithm;
 class UnderlyingSourceBase;
@@ -40,9 +41,7 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
   class PipeOptions : public GarbageCollected<PipeOptions> {
    public:
     PipeOptions();
-    PipeOptions(ScriptState* script_state,
-                ScriptValue options,
-                ExceptionState& exception_state);
+    explicit PipeOptions(const StreamPipeOptions* options);
 
     bool PreventClose() const { return prevent_close_; }
     bool PreventAbort() const { return prevent_abort_; }
@@ -123,7 +122,7 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
   // https://streams.spec.whatwg.org/#rs-pipe-through
   ScriptValue pipeThrough(ScriptState*,
                           ScriptValue transform_stream,
-                          ScriptValue options,
+                          const StreamPipeOptions* options,
                           ExceptionState&);
 
   ScriptPromise pipeTo(ScriptState*, ScriptValue destination, ExceptionState&);
@@ -131,7 +130,7 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
   // https://streams.spec.whatwg.org/#rs-pipe-to
   ScriptPromise pipeTo(ScriptState*,
                        ScriptValue destination_value,
-                       ScriptValue options,
+                       const StreamPipeOptions* options,
                        ExceptionState&);
 
   // https://streams.spec.whatwg.org/#rs-tee

third_party/blink/renderer/core/streams/readable_stream.idl

@@ -13,8 +13,8 @@
     [RaisesException, CallWith=ScriptState] Promise<any> cancel(optional any reason);
     [RaisesException, CallWith=ScriptState, MeasureAs=ReadableStreamGetReader] ReadableStreamDefaultReader getReader(optional any mode);
     [RaisesException, CallWith=ScriptState, MeasureAs=ReadableStreamPipeThrough] any pipeThrough(
-        any transformStream, optional any options);
+        any transformStream, optional StreamPipeOptions options);
     [RaisesException, CallWith=ScriptState, MeasureAs=ReadableStreamPipeTo] Promise<any> pipeTo(
-        any destination, optional any option);
+        any destination, optional StreamPipeOptions options);
     [RaisesException, CallWith=ScriptState] any tee();
 };

third_party/blink/renderer/core/streams/stream_pipe_options.idl

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// https://streams.spec.whatwg.org/#dictdef-streampipeoptions
+
+dictionary StreamPipeOptions {
+  boolean preventClose = false;
+  boolean preventAbort = false;
+  boolean preventCancel = false;
+  AbortSignal signal;
+};

third_party/blink/web_tests/external/wpt/streams/piping/general.any.js

@@ -198,3 +198,14 @@ promise_test(t => {
     }
   }), 'pipeTo should reject');
 }, 'pipeTo() should reject if an option getter grabs a writer');
+
+promise_test(t => {
+  const rs = new ReadableStream({
+    start(controller) {
+      controller.close();
+    }
+  });
+  const ws = new WritableStream();
+
+  return rs.pipeTo(ws, null);
+}, 'pipeTo() promise should resolve if null is passed');

third_party/blink/web_tests/external/wpt/streams/piping/throwing-options.any-expected.txt

-This is a testharness.js-based test.
-FAIL pipeTo should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeThrough should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeTo should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeThrough should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeTo should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeThrough should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeTo should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-FAIL pipeThrough should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/streams/piping/throwing-options.any.serviceworker-expected.txt

-This is a testharness.js-based test.
-FAIL pipeTo should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeThrough should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeTo should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeThrough should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeTo should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeThrough should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeTo should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-FAIL pipeThrough should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/streams/piping/throwing-options.any.sharedworker-expected.txt

-This is a testharness.js-based test.
-FAIL pipeTo should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeThrough should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeTo should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeThrough should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeTo should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeThrough should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeTo should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-FAIL pipeThrough should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/streams/piping/throwing-options.any.worker-expected.txt

-This is a testharness.js-based test.
-FAIL pipeTo should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeThrough should stop after getting preventAbort throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort"] length 1, got ["preventClose", "preventAbort"] length 2
-FAIL pipeTo should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeThrough should stop after getting preventCancel throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel"] length 2, got ["preventClose", "preventAbort", "preventCancel"] length 3
-FAIL pipeTo should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeThrough should stop after getting preventClose throws assert_array_equals: options should be touched in the right order lengths differ, expected array ["preventAbort", "preventCancel", "preventClose"] length 3, got ["preventClose"] length 1
-FAIL pipeTo should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-FAIL pipeThrough should stop after getting signal throws assert_array_equals: options should be touched in the right order expected property 0 to be "preventAbort" but got "preventClose" (expected array ["preventAbort", "preventCancel", "preventClose", "signal"] got ["preventClose", "preventAbort", "preventCancel", "signal"])
-Harness: the test ran to completion.
-