Make ReadableStream extendable

IDL defined ReadableStream was not extendable because it didn't use
provided |info.Holder()| as its wrapper. Instead, it creates a new
wrapper in Create functions.

This CL solves the issue by making the constructor "custom". The custom
constructor creates a blink::ReadableStream first, then associates it
to the wrapper, and then calls the appropriate Init function.

Bug: 906476
Change-Id: I91a7fec1cf4ee5e08704a944b0af852584d1679e
Reviewed-on: https://chromium-review.googlesource.com/c/1347643
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610391}

third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/general-expected.txt

@@ -32,6 +32,7 @@ PASS ReadableStream: enqueue should throw when the stream is closed
 PASS ReadableStream: should call underlying source methods as methods
 PASS ReadableStream: desiredSize when closed
 PASS ReadableStream: desiredSize when errored
+PASS ReadableStream: ReadableStream is extendable
 PASS ReadableStream strategies: the default strategy should give desiredSize of 1 to start, decreasing by 1 per enqueue
 PASS ReadableStream strategies: the default strategy should continue giving desiredSize of 1 if the chunks are read immediately
 PASS ReadableStream integration test: adapting a random push source

third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/general.dedicatedworker-expected.txt

@@ -32,6 +32,7 @@ PASS ReadableStream: enqueue should throw when the stream is closed
 PASS ReadableStream: should call underlying source methods as methods
 PASS ReadableStream: desiredSize when closed
 PASS ReadableStream: desiredSize when errored
+PASS ReadableStream: ReadableStream is extendable
 PASS ReadableStream strategies: the default strategy should give desiredSize of 1 to start, decreasing by 1 per enqueue
 PASS ReadableStream strategies: the default strategy should continue giving desiredSize of 1 if the chunks are read immediately
 PASS ReadableStream integration test: adapting a random push source

third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/general.js

@@ -774,6 +774,13 @@ test(() => {
   });
 }, 'ReadableStream: desiredSize when errored');
 
+test(() => {
+   class Extended extends ReadableStream {
+     newMethod() { return 'foo' };
+   };
+   assert_equals((new Extended()).newMethod(), 'foo');
+}, 'ReadableStream: ReadableStream is extendable');
+
 test(() => {
 
   let startCalled = false;

third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/general.serviceworker.https-expected.txt

@@ -33,6 +33,7 @@ PASS ReadableStream: enqueue should throw when the stream is closed
 PASS ReadableStream: should call underlying source methods as methods
 PASS ReadableStream: desiredSize when closed
 PASS ReadableStream: desiredSize when errored
+PASS ReadableStream: ReadableStream is extendable
 PASS ReadableStream strategies: the default strategy should give desiredSize of 1 to start, decreasing by 1 per enqueue
 PASS ReadableStream strategies: the default strategy should continue giving desiredSize of 1 if the chunks are read immediately
 PASS ReadableStream integration test: adapting a random push source

third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/general.sharedworker-expected.txt

@@ -32,6 +32,7 @@ PASS ReadableStream: enqueue should throw when the stream is closed
 PASS ReadableStream: should call underlying source methods as methods
 PASS ReadableStream: desiredSize when closed
 PASS ReadableStream: desiredSize when errored
+PASS ReadableStream: ReadableStream is extendable
 PASS ReadableStream strategies: the default strategy should give desiredSize of 1 to start, decreasing by 1 per enqueue
 PASS ReadableStream strategies: the default strategy should continue giving desiredSize of 1 if the chunks are read immediately
 PASS ReadableStream integration test: adapting a random push source

third_party/blink/renderer/bindings/bindings.gni

@@ -21,6 +21,7 @@ bindings_core_v8_files =
                     "core/v8/custom/v8_message_event_custom.cc",
                     "core/v8/custom/v8_pop_state_event_custom.cc",
                     "core/v8/custom/v8_promise_rejection_event_custom.cc",
+                    "core/v8/custom/v8_readable_stream_custom.cc",
                     "core/v8/custom/v8_shadow_root_custom.cc",
                     "core/v8/custom/v8_window_custom.cc",
                     "core/v8/custom/v8_xml_http_request_custom.cc",

third_party/blink/renderer/bindings/core/v8/custom/v8_readable_stream_custom.cc

+// Copyright 2018 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.
+
+#include "third_party/blink/renderer/bindings/core/v8/v8_readable_stream.h"
+
+#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
+#include "third_party/blink/renderer/core/streams/readable_stream.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
+#include "third_party/blink/renderer/platform/bindings/script_state.h"
+#include "third_party/blink/renderer/platform/bindings/v8_binding.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+void V8ReadableStream::constructorCustom(
+    const v8::FunctionCallbackInfo<v8::Value>& info) {
+  RUNTIME_CALL_TIMER_SCOPE_DISABLED_BY_DEFAULT(
+      info.GetIsolate(), "Blink_ReadableStream_ConstructorCallback");
+
+  ExceptionState exception_state(info.GetIsolate(),
+                                 ExceptionState::kConstructionContext,
+                                 "ReadableStream");
+  ScriptState* script_state =
+      ScriptState::From(info.NewTarget().As<v8::Object>()->CreationContext());
+
+  ScriptValue underlying_source =
+      ScriptValue(ScriptState::Current(info.GetIsolate()),
+                  v8::Undefined(info.GetIsolate()));
+  ScriptValue strategy = ScriptValue(ScriptState::Current(info.GetIsolate()),
+                                     v8::Undefined(info.GetIsolate()));
+  int num_args = info.Length();
+  auto* impl = MakeGarbageCollected<ReadableStream>();
+  v8::Local<v8::Object> wrapper = info.Holder();
+  wrapper = impl->AssociateWithWrapper(
+      info.GetIsolate(), &V8ReadableStream::wrapperTypeInfo, wrapper);
+
+  if (num_args >= 1) {
+    underlying_source =
+        ScriptValue(ScriptState::Current(info.GetIsolate()), info[0]);
+  }
+  if (num_args >= 2)
+    strategy = ScriptValue(ScriptState::Current(info.GetIsolate()), info[1]);
+
+  impl->Init(script_state, underlying_source, strategy, exception_state);
+  if (exception_state.HadException()) {
+    return;
+  }
+  V8SetReturnValue(info, wrapper);
+}
+
+}  // namespace blink

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

@@ -23,11 +23,58 @@ class ReadableStream::NoopFunction : public ScriptFunction {
   ScriptValue Call(ScriptValue value) override { return value; }
 };
 
-ReadableStream::ReadableStream(ScriptState* script_state,
-                               v8::Local<v8::Object> object)
-    : object_(script_state->GetIsolate(), object) {
+void ReadableStream::Init(ScriptState* script_state,
+                          ScriptValue underlying_source,
+                          ScriptValue strategy,
+                          ExceptionState& exception_state) {
+  ScriptValue value = ReadableStreamOperations::CreateReadableStream(
+      script_state, underlying_source, strategy, exception_state);
+  if (exception_state.HadException())
+    return;
+
+  DCHECK(value.IsObject());
+  InitWithInternalStream(script_state, value.V8Value().As<v8::Object>(),
+                         exception_state);
+}
+
+void ReadableStream::InitWithInternalStream(ScriptState* script_state,
+                                            v8::Local<v8::Object> object,
+                                            ExceptionState& exception_state) {
   DCHECK(ReadableStreamOperations::IsReadableStreamForDCheck(
       script_state, ScriptValue(script_state, object)));
+  object_.Set(script_state->GetIsolate(), object);
+
+  v8::Isolate* isolate = script_state->GetIsolate();
+  v8::TryCatch block(isolate);
+  v8::Local<v8::Value> wrapper = ToV8(this, script_state);
+  if (wrapper.IsEmpty()) {
+    exception_state.RethrowV8Exception(block.Exception());
+    return;
+  }
+
+  v8::Local<v8::Context> context = script_state->GetContext();
+  v8::Local<v8::Object> bindings =
+      context->GetExtrasBindingObject().As<v8::Object>();
+  v8::Local<v8::Value> symbol_value;
+  if (!bindings->Get(context, V8String(isolate, "internalReadableStreamSymbol"))
+           .ToLocal(&symbol_value)) {
+    exception_state.RethrowV8Exception(block.Exception());
+    return;
+  }
+
+  if (wrapper.As<v8::Object>()
+          ->Set(context, symbol_value.As<v8::Symbol>(), object)
+          .IsNothing()) {
+    exception_state.RethrowV8Exception(block.Exception());
+    return;
+  }
+
+  // This is needed because sometimes a ReadableStream can be detached from
+  // the owner object such as Response.
+  if (!RetainWrapperDuringConstruction(this, script_state)) {
+    exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
+                                      "Cannot queue task to retain wrapper");
+  }
 }
 
 ReadableStream* ReadableStream::Create(ScriptState* script_state,
@@ -52,27 +99,10 @@ ReadableStream* ReadableStream::Create(ScriptState* script_state,
                                        ScriptValue underlying_source,
                                        ScriptValue strategy,
                                        ExceptionState& exception_state) {
-  ScriptValue value = ReadableStreamOperations::CreateReadableStream(
-      script_state, underlying_source, strategy, exception_state);
-
-  if (value.IsEmpty())
-    return nullptr;
-
-  DCHECK(value.V8Value()->IsObject());
-  return CreateFromInternalStream(script_state, value, exception_state);
-}
-
-ReadableStream* ReadableStream::CreateFromInternalStream(
-    ScriptState* script_state,
-    v8::Local<v8::Object> object,
-    ExceptionState& exception_state) {
-  auto* stream = MakeGarbageCollected<ReadableStream>(script_state, object);
-
-  v8::TryCatch block(script_state->GetIsolate());
-  if (!stream->Init(script_state)) {
-    exception_state.RethrowV8Exception(block.Exception());
+  auto* stream = MakeGarbageCollected<ReadableStream>();
+  stream->Init(script_state, underlying_source, strategy, exception_state);
+  if (exception_state.HadException())
     return nullptr;
-  }
   return stream;
 }
 
@@ -85,6 +115,17 @@ ReadableStream* ReadableStream::CreateFromInternalStream(
       script_state, object.V8Value().As<v8::Object>(), exception_state);
 }
 
+ReadableStream* ReadableStream::CreateFromInternalStream(
+    ScriptState* script_state,
+    v8::Local<v8::Object> object,
+    ExceptionState& exception_state) {
+  auto* stream = MakeGarbageCollected<ReadableStream>();
+  stream->InitWithInternalStream(script_state, object, exception_state);
+  if (exception_state.HadException())
+    return nullptr;
+  return stream;
+}
+
 ReadableStream* ReadableStream::CreateWithCountQueueingStrategy(
     ScriptState* script_state,
     UnderlyingSourceBase* underlying_source,
@@ -97,16 +138,16 @@ ReadableStream* ReadableStream::CreateWithCountQueueingStrategy(
 
   ScriptValue value = ReadableStreamOperations::CreateReadableStream(
       script_state, underlying_source, strategy);
-
   if (value.IsEmpty())
     return nullptr;
 
+  ExceptionState exception_state(script_state->GetIsolate(),
+                                 ExceptionState::kConstructionContext,
+                                 "ReadableStream");
   DCHECK(value.V8Value()->IsObject());
-  v8::Local<v8::Object> object = value.V8Value().As<v8::Object>();
-  auto* stream = MakeGarbageCollected<ReadableStream>(script_state, object);
-
-  if (!stream->Init(script_state))
-    return nullptr;
+  auto* stream = CreateFromInternalStream(script_state, value, exception_state);
+  if (exception_state.HadException())
+    exception_state.ClearException();
   return stream;
 }
 
@@ -383,12 +424,15 @@ void ReadableStream::Tee(ScriptState* script_state,
   DCHECK(v8_branch1->IsObject());
   DCHECK(v8_branch2->IsObject());
 
-  ReadableStream* temp_branch1 = CreateFromInternalStream(
+  ReadableStream* temp_branch1 = MakeGarbageCollected<ReadableStream>();
+  ReadableStream* temp_branch2 = MakeGarbageCollected<ReadableStream>();
+
+  temp_branch1->InitWithInternalStream(
       script_state, v8_branch1.As<v8::Object>(), exception_state);
   if (exception_state.HadException())
     return;
 
-  ReadableStream* temp_branch2 = CreateFromInternalStream(
+  temp_branch2->InitWithInternalStream(
       script_state, v8_branch2.As<v8::Object>(), exception_state);
   if (exception_state.HadException())
     return;
@@ -476,31 +520,4 @@ ScriptValue ReadableStream::GetInternalStream(ScriptState* script_state) const {
                      object_.NewLocal(script_state->GetIsolate()));
 }
 
-bool ReadableStream::Init(ScriptState* script_state) {
-  v8::Isolate* isolate = script_state->GetIsolate();
-  v8::Local<v8::Object> internal_stream = object_.NewLocal(isolate);
-  v8::Local<v8::Value> wrapper = ToV8(this, script_state);
-  if (wrapper.IsEmpty())
-    return false;
-
-  v8::Local<v8::Context> context = script_state->GetContext();
-  v8::Local<v8::Object> bindings =
-      context->GetExtrasBindingObject().As<v8::Object>();
-  v8::Local<v8::Value> symbol_value;
-  if (!bindings->Get(context, V8String(isolate, "internalReadableStreamSymbol"))
-           .ToLocal(&symbol_value)) {
-    return false;
-  }
-
-  if (wrapper.As<v8::Object>()
-          ->Set(context, symbol_value.As<v8::Symbol>(), internal_stream)
-          .IsNothing()) {
-    return false;
-  }
-
-  // This is needed because sometimes a ReadableStream can be detached from
-  // the owner object such as Response.
-  return RetainWrapperDuringConstruction(this, script_state);
-}
-
 }  // namespace blink

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

@@ -25,9 +25,22 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  // Use one of Create functions. This is public only for MakeGarbageCollected.
-  ReadableStream(ScriptState*, v8::Local<v8::Object> object);
+  // Call one of Init functions before using the instance.
+  ReadableStream() = default;
+  ~ReadableStream() override = default;
 
+  // If an error happens, |exception_state.HadException()| will be true, and
+  // |this| will not be usable after that.
+  void Init(ScriptState*,
+            ScriptValue underlying_source,
+            ScriptValue strategy,
+            ExceptionState& exception_state);
+  void InitWithInternalStream(ScriptState*,
+                              v8::Local<v8::Object> object,
+                              ExceptionState& exception_state);
+
+  // Create* functions call Init* internally and returns null when an error
+  // happens.
   static ReadableStream* Create(ScriptState*, ExceptionState&);
   static ReadableStream* Create(ScriptState*,
                                 ScriptValue underlying_source,
@@ -49,8 +62,6 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
       UnderlyingSourceBase* underlying_source,
       size_t high_water_mark);
 
-  ~ReadableStream() override = default;
-
   void Trace(Visitor* visitor) override;
 
   // IDL defined functions
@@ -109,8 +120,6 @@ class CORE_EXPORT ReadableStream : public ScriptWrappable {
  private:
   class NoopFunction;
 
-  bool Init(ScriptState*);
-
   TraceWrapperV8Reference<v8::Object> object_;
 };
 

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

@@ -5,7 +5,8 @@
 // https://streams.spec.whatwg.org/#rs-class
 [
     Exposed=(Window,Worker,Worklet),
-    Constructor(optional any underlyingSource, optional any strategy),
+    // TODO(yhirano): Remove CustomConstructor. See https://crbug.com/906476.
+    CustomConstructor(optional any underlyingSource, optional any strategy),
     RaisesException=Constructor,
     ConstructorCallWith=ScriptState
 ] interface ReadableStream {