Add C++ wrapper for TransformStream

Design doc
https://docs.google.com/document/d/17goe4jacAYjHHtprfVPSrqDvF_J58u2qirz0HQ58sQ4/edit

Implement the C++ classes TransformStream and
TransformStreamDefaultController. These provide thin wrappers for the
equivalent JavaScript classes.

Modify the JavaScript TransformStream implementation to pass the
|controller| argument to the algorithms. This is a departure from the
standard, but the difference is not observable to user code. It makes
the memory management considerably simpler as no C++ reference to the
TransformStreamDefaultController needs to be retained between calls.

Define the interface TransformStreamTransformer.

Also create unit tests for these new classes.

Bug: 845427
Change-Id: I067a8ff15daaa4912760fbdb9ca4697705f2e3f8
Reviewed-on: https://chromium-review.googlesource.com/1156324Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582512}

third_party/blink/renderer/bindings/core/v8/script_function.cc

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/script_function.h"
 
 #include "third_party/blink/renderer/platform/bindings/v8_binding.h"
+#include "third_party/blink/renderer/platform/wtf/assertions.h"
 
 namespace blink {
 
@@ -26,6 +27,16 @@ v8::Local<v8::Function> ScriptFunction::BindToV8Function() {
       .ToLocalChecked();
 }
 
+ScriptValue ScriptFunction::Call(ScriptValue) {
+  NOTREACHED();
+  return ScriptValue();
+}
+
+void ScriptFunction::CallRaw(const v8::FunctionCallbackInfo<v8::Value>& args) {
+  ScriptValue result = Call(ScriptValue(GetScriptState(), args[0]));
+  V8SetReturnValue(args, result.V8Value());
+}
+
 void ScriptFunction::CallCallback(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   DCHECK(args.Data()->IsExternal());
@@ -33,9 +44,7 @@ void ScriptFunction::CallCallback(
                                                "Blink_CallCallback");
   ScriptFunction* script_function = static_cast<ScriptFunction*>(
       v8::Local<v8::External>::Cast(args.Data())->Value());
-  ScriptValue result = script_function->Call(
-      ScriptValue(script_function->GetScriptState(), args[0]));
-  V8SetReturnValue(args, result.V8Value());
+  script_function->CallRaw(args);
 }
 
 }  // namespace blink

third_party/blink/renderer/bindings/core/v8/script_function.h

@@ -41,14 +41,13 @@ namespace blink {
 // A common way of using ScriptFunction is as follows:
 //
 // class DerivedFunction : public ScriptFunction {
-//     // This returns a V8 function which the DerivedFunction is bound to.
-//     // The DerivedFunction is destructed when the V8 function is
-//     // garbage-collected.
-//     static v8::Local<v8::Function> createFunction(ScriptState* scriptState)
-//     {
-//         DerivedFunction* self = new DerivedFunction(scriptState);
-//         return self->bindToV8Function();
-//     }
+//   // This returns a V8 function which the DerivedFunction is bound to.
+//   // The DerivedFunction is destroyed when the V8 function is
+//   // garbage-collected.
+//   static v8::Local<v8::Function> CreateFunction(ScriptState* script_state) {
+//     DerivedFunction* self = new DerivedFunction(script_state);
+//     return self->BindToV8Function();
+//   }
 // };
 class CORE_EXPORT ScriptFunction
     : public GarbageCollectedFinalized<ScriptFunction> {
@@ -65,7 +64,14 @@ class CORE_EXPORT ScriptFunction
   v8::Local<v8::Function> BindToV8Function();
 
  private:
-  virtual ScriptValue Call(ScriptValue) = 0;
+  // Subclasses should implement one of Call() or CallRaw(). Most will implement
+  // Call().
+  virtual ScriptValue Call(ScriptValue);
+
+  // To support more than one argument, or for low-level access to the V8 API,
+  // implement CallRaw(). The default implementation delegates to Call().
+  virtual void CallRaw(const v8::FunctionCallbackInfo<v8::Value>&);
+
   static void CallCallback(const v8::FunctionCallbackInfo<v8::Value>&);
 
   Member<ScriptState> script_state_;

third_party/blink/renderer/core/BUILD.gn

@@ -2085,6 +2085,7 @@ jumbo_source_set("unit_tests") {
     "scroll/scrollable_area_test.cc",
     "scroll/scrollbar_theme_overlay_test.cc",
     "streams/readable_stream_operations_test.cc",
+    "streams/transform_stream_test.cc",
     "style/border_value_test.cc",
     "style/computed_style_test.cc",
     "style/filter_operations_test.cc",

third_party/blink/renderer/core/streams/BUILD.gn

@@ -9,6 +9,11 @@ blink_core_sources("streams") {
     "readable_stream_default_controller_wrapper.h",
     "readable_stream_operations.cc",
     "readable_stream_operations.h",
+    "transform_stream.cc",
+    "transform_stream.h",
+    "transform_stream_default_controller.cc",
+    "transform_stream_default_controller.h",
+    "transform_stream_transformer.h",
     "underlying_source_base.cc",
     "underlying_source_base.h",
   ]

third_party/blink/renderer/core/streams/TransformStream.js

@@ -23,7 +23,12 @@
   const _writable = v8.createPrivateSymbol('[[writable]]');
   const _controlledTransformStream =
       v8.createPrivateSymbol('[[controlledTransformStream]]');
+
+  // Unlike the version in the standard, the controller is passed to this.
   const _flushAlgorithm = v8.createPrivateSymbol('[[flushAlgorithm]]');
+
+  // Unlike the version in the standard, the controller is passed in as the
+  // second argument.
   const _transformAlgorithm = v8.createPrivateSymbol('[[transformAlgorithm]]');
 
   // Javascript functions. It is important to use these copies, as the ones on
@@ -45,7 +50,7 @@
   const {
     hasOwnPropertyNoThrow,
     resolvePromise,
-    CreateAlgorithmFromUnderlyingMethodPassingController,
+    CreateAlgorithmFromUnderlyingMethod,
     CallOrNoop1,
     MakeSizeAlgorithmFromSizeFunction,
     PromiseCall2,
@@ -129,6 +134,8 @@
 
   const TransformStream_prototype = TransformStream.prototype;
 
+  // The controller is passed to |transformAlgorithm| and |flushAlgorithm|,
+  // unlike in the standard.
   function CreateTransformStream(
       startAlgorithm, transformAlgorithm, flushAlgorithm, writableHighWaterMark,
       writableSizeAlgorithm, readableHighWaterMark, readableSizeAlgorithm) {
@@ -322,8 +329,8 @@
         }
       };
     }
-    const flushAlgorithm = CreateAlgorithmFromUnderlyingMethodPassingController(
-        transformer, 'flush', 0, controller, 'transformer.flush');
+    const flushAlgorithm = CreateAlgorithmFromUnderlyingMethod(
+        transformer, 'flush', 1, 'transformer.flush');
     SetUpTransformStreamDefaultController(
         stream, controller, transformAlgorithm, flushAlgorithm);
   }
@@ -397,11 +404,11 @@
         // assert(binding.isWritableStreamWritable(writable),
         //        `state is "writable"`);
 
-        return controller[_transformAlgorithm](chunk);
+        return controller[_transformAlgorithm](chunk, controller);
       });
     }
 
-    return controller[_transformAlgorithm](chunk);
+    return controller[_transformAlgorithm](chunk, controller);
   }
 
   function TransformStreamDefaultSinkAbortAlgorithm(stream, reason) {
@@ -412,7 +419,7 @@
   function TransformStreamDefaultSinkCloseAlgorithm(stream) {
     const readable = stream[_readable];
     const controller = stream[_transformStreamController];
-    const flushPromise = controller[_flushAlgorithm]();
+    const flushPromise = controller[_flushAlgorithm](controller);
     TransformStreamDefaultControllerClearAlgorithms(controller);
 
     return thenPromise(
@@ -445,6 +452,22 @@
     return stream[_backpressureChangePromise];
   }
 
+  // A wrapper for CreateTransformStream() with only the arguments that
+  // blink::TransformStream needs. |transformAlgorithm| and |flushAlgorithm| are
+  // passed the controller, unlike in the standard.
+  function createTransformStreamSimple(transformAlgorithm, flushAlgorithm) {
+    return CreateTransformStream(() => Promise_resolve(),
+                                 transformAlgorithm, flushAlgorithm);
+  }
+
+  function getTransformStreamReadable(stream) {
+    return stream[_readable];
+  }
+
+  function getTransformStreamWritable(stream) {
+    return stream[_writable];
+  }
+
   //
   // Additions to the global object
   //
@@ -459,5 +482,10 @@
   //
   // Exports to Blink
   //
-  binding.CreateTransformStream = CreateTransformStream;
+  Object.assign(binding, {
+    createTransformStreamSimple,
+    TransformStreamDefaultControllerEnqueue,
+    getTransformStreamReadable,
+    getTransformStreamWritable
+  });
 });

third_party/blink/renderer/core/streams/transform_stream.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/core/streams/transform_stream.h"
+
+#include "third_party/blink/renderer/bindings/core/v8/generated_code_helper.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_function.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_script_runner.h"
+#include "third_party/blink/renderer/core/streams/transform_stream_default_controller.h"
+#include "third_party/blink/renderer/core/streams/transform_stream_transformer.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 "third_party/blink/renderer/platform/heap/visitor.h"
+
+namespace blink {
+
+// Base class for FlushAlgorithm and TransformAlgorithm. Contains common
+// construction code and members.
+class TransformStream::Algorithm : public ScriptFunction {
+ public:
+  // This is templated just to avoid having two identical copies of the
+  // function.
+  template <typename T>
+  static v8::Local<v8::Function> Create(TransformStreamTransformer* transformer,
+                                        ScriptState* script_state,
+                                        ExceptionState& exception_state) {
+    auto* algorithm = new T(transformer, script_state, exception_state);
+    return algorithm->BindToV8Function();
+  }
+
+  void Trace(Visitor* visitor) override {
+    visitor->Trace(transformer_);
+    ScriptFunction::Trace(visitor);
+  }
+
+ protected:
+  Algorithm(TransformStreamTransformer* transformer,
+            ScriptState* script_state,
+            ExceptionState& exception_state)
+      : ScriptFunction(script_state),
+        transformer_(transformer),
+        context_(exception_state.Context()),
+        interface_name_(exception_state.InterfaceName()),
+        property_name_(exception_state.PropertyName()) {}
+
+  // AlgorithmScope holds the stack-allocated objects used by the CallRaw()
+  // methods for FlushAlgorithm and TransformAlgorithm.
+  class AlgorithmScope {
+    STACK_ALLOCATED();
+
+   public:
+    AlgorithmScope(Algorithm* owner,
+                   const v8::FunctionCallbackInfo<v8::Value>& info,
+                   v8::Local<v8::Value> controller)
+        : controller_(owner->GetScriptState(), controller),
+          exception_state_(owner->GetScriptState()->GetIsolate(),
+                           owner->context_,
+                           owner->interface_name_,
+                           owner->property_name_),
+          reject_promise_scope_(info, exception_state_) {}
+
+    TransformStreamDefaultController* GetController() { return &controller_; }
+
+    ExceptionState* GetExceptionState() { return &exception_state_; }
+
+   private:
+    TransformStreamDefaultController controller_;
+    ExceptionState exception_state_;
+    ExceptionToRejectPromiseScope reject_promise_scope_;
+  };
+
+  Member<TransformStreamTransformer> transformer_;
+  const ExceptionState::ContextType context_;
+  const char* const interface_name_;
+  const char* const property_name_;
+};
+
+class TransformStream::FlushAlgorithm : public TransformStream::Algorithm {
+ protected:
+  using Algorithm::Algorithm;
+
+ private:
+  void CallRaw(const v8::FunctionCallbackInfo<v8::Value>& info) override {
+    DCHECK_EQ(info.Length(), 1);
+    AlgorithmScope algorithm_scope(this, info, info[0]);
+    ExceptionState& exception_state = *algorithm_scope.GetExceptionState();
+
+    transformer_->Flush(algorithm_scope.GetController(), exception_state);
+    if (exception_state.HadException())
+      return;
+    V8SetReturnValue(info,
+                     ScriptPromise::CastUndefined(GetScriptState()).V8Value());
+  }
+};
+
+class TransformStream::TransformAlgorithm : public TransformStream::Algorithm {
+ protected:
+  using Algorithm::Algorithm;
+
+ private:
+  void CallRaw(const v8::FunctionCallbackInfo<v8::Value>& info) override {
+    DCHECK_EQ(info.Length(), 2);
+    AlgorithmScope algorithm_scope(this, info, info[1]);
+    ExceptionState& exception_state = *algorithm_scope.GetExceptionState();
+
+    transformer_->Transform(info[0], algorithm_scope.GetController(),
+                            exception_state);
+    if (exception_state.HadException())
+      return;
+    V8SetReturnValue(info,
+                     ScriptPromise::CastUndefined(GetScriptState()).V8Value());
+  }
+};
+
+TransformStream::TransformStream() = default;
+
+TransformStream::~TransformStream() = default;
+
+void TransformStream::Init(TransformStreamTransformer* transformer,
+                           ScriptState* script_state,
+                           ExceptionState& exception_state) {
+  auto transform_algorithm = Algorithm::Create<TransformAlgorithm>(
+      transformer, script_state, exception_state);
+  auto flush_algorithm = Algorithm::Create<FlushAlgorithm>(
+      transformer, script_state, exception_state);
+  v8::Local<v8::Value> args[] = {transform_algorithm, flush_algorithm};
+  v8::TryCatch block(script_state->GetIsolate());
+  v8::Local<v8::Value> stream;
+  if (!V8ScriptRunner::CallExtra(script_state, "createTransformStreamSimple",
+                                 args)
+           .ToLocal(&stream)) {
+    DCHECK(block.HasCaught());
+    exception_state.RethrowV8Exception(block.Exception());
+    return;
+  }
+  DCHECK(!block.HasCaught());
+  DCHECK(stream->IsObject());
+  stream_.Set(script_state->GetIsolate(), stream);
+}
+
+ScriptValue TransformStream::Readable(ScriptState* script_state,
+                                      ExceptionState& exception_state) const {
+  return Accessor("getTransformStreamReadable", script_state, exception_state);
+}
+
+ScriptValue TransformStream::Writable(ScriptState* script_state,
+                                      ExceptionState& exception_state) const {
+  return Accessor("getTransformStreamWritable", script_state, exception_state);
+}
+
+void TransformStream::Trace(Visitor* visitor) {
+  visitor->Trace(stream_);
+}
+
+ScriptValue TransformStream::Accessor(const char* accessor_function_name,
+                                      ScriptState* script_state,
+                                      ExceptionState& exception_state) const {
+  v8::Local<v8::Value> result;
+  v8::Local<v8::Value> args[] = {stream_.NewLocal(script_state->GetIsolate())};
+  DCHECK(args[0]->IsObject());
+  v8::TryCatch block(script_state->GetIsolate());
+  if (!V8ScriptRunner::CallExtra(script_state, accessor_function_name, args)
+           .ToLocal(&result)) {
+    DCHECK(block.HasCaught());
+    exception_state.RethrowV8Exception(block.Exception());
+    return ScriptValue();
+  }
+  DCHECK(!block.HasCaught());
+  return ScriptValue(script_state, result);
+}
+
+}  // namespace blink

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

+// 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.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_H_
+
+#include "base/macros.h"
+#include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/platform/bindings/trace_wrapper_v8_reference.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/heap/heap.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+class ExceptionState;
+class ScriptState;
+class ScriptValue;
+class TransformStreamTransformer;
+class Visitor;
+
+// Creates and wraps a JavaScript TransformStream object with a transformation
+// defined in C++. Provides access to the readable and writable streams.
+//
+// On-heap references to this class must always be via a TraceWrapperMember, and
+// must always have an ancestor in the V8 heap, or |stream_| will be lost.
+//
+// To ensure that the JS TransformStream is always referenced, this class uses
+// two-stage construction. After calling the constructor, store the reference
+// in a TraceWrapperMember before calling Init(). Init() must always be called
+// before using the instance.
+class CORE_EXPORT TransformStream final
+    : public GarbageCollectedFinalized<TransformStream> {
+ public:
+  TransformStream();
+  ~TransformStream();
+
+  // If HadException is true on return, the object is invalid and should be
+  // destroyed.
+  void Init(TransformStreamTransformer*, ScriptState*, ExceptionState&);
+
+  ScriptValue Readable(ScriptState*, ExceptionState&) const;
+  ScriptValue Writable(ScriptState*, ExceptionState&) const;
+
+  void Trace(Visitor*);
+
+ private:
+  // These are class-scoped to avoid name clashes in jumbo builds.
+  class Algorithm;
+  class FlushAlgorithm;
+  class TransformAlgorithm;
+
+  // Common implementation for Readable() and Writable() accessors.
+  ScriptValue Accessor(const char* accessor_function_name,
+                       ScriptState*,
+                       ExceptionState&) const;
+
+  TraceWrapperV8Reference<v8::Value> stream_;
+
+  DISALLOW_COPY_AND_ASSIGN(TransformStream);
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_H_

third_party/blink/renderer/core/streams/transform_stream_default_controller.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/core/streams/transform_stream_default_controller.h"
+
+#include "third_party/blink/renderer/bindings/core/v8/v8_script_runner.h"
+
+namespace blink {
+
+TransformStreamDefaultController::TransformStreamDefaultController(
+    ScriptState* script_state,
+    v8::Local<v8::Value> controller)
+    : script_state_(script_state), controller_(controller) {
+  DCHECK(controller->IsObject());
+}
+
+void TransformStreamDefaultController::Enqueue(
+    v8::Local<v8::Value> chunk,
+    ExceptionState& exception_state) {
+  DCHECK(controller_->IsObject());
+  v8::Local<v8::Value> args[] = {controller_, chunk};
+  v8::TryCatch block(script_state_->GetIsolate());
+  V8ScriptRunner::CallExtra(script_state_,
+                            "TransformStreamDefaultControllerEnqueue", args);
+  if (block.HasCaught()) {
+    exception_state.RethrowV8Exception(block.Exception());
+    return;
+  }
+}
+
+}  // namespace blink

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

+// 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.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_DEFAULT_CONTROLLER_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_DEFAULT_CONTROLLER_H_
+
+#include "base/macros.h"
+#include "third_party/blink/renderer/core/core_export.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/heap/member.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+// Thin wrapper for the JavaScript TransformStreamDefaultController object. The
+// API mimics the JavaScript API
+// https://streams.spec.whatwg.org/#ts-default-controller-class but unneeded
+// parts have not been implemented.
+class CORE_EXPORT TransformStreamDefaultController final {
+  STACK_ALLOCATED();
+
+ public:
+  TransformStreamDefaultController(ScriptState*,
+                                   v8::Local<v8::Value> controller);
+
+  void Enqueue(v8::Local<v8::Value> chunk, ExceptionState&);
+
+ private:
+  Member<ScriptState> script_state_;
+  v8::Local<v8::Value> controller_;
+
+  DISALLOW_COPY_AND_ASSIGN(TransformStreamDefaultController);
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_DEFAULT_CONTROLLER_H_

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

+// 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.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_TRANSFORMER_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_TRANSFORMER_H_
+
+#include "base/macros.h"
+#include "third_party/blink/renderer/core/core_export.h"
+#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
+#include "third_party/blink/renderer/platform/heap/heap.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+class ExceptionState;
+class TransformStreamDefaultController;
+class Visitor;
+
+// Interface to be implemented by C++ code that needs to create a
+// TransformStream. Very similar to the JavaScript [Transformer
+// API](https://streams.spec.whatwg.org/#transformer-api), but asynchronous
+// transforms are not currently supported. Errors should be signalled by
+// exceptions.
+//
+// An instance is stored in a JS object as a Persistent reference, so to avoid
+// uncollectable cycles implementations must not directly or indirectly strongly
+// reference any JS object.
+class CORE_EXPORT TransformStreamTransformer
+    : public GarbageCollectedFinalized<TransformStreamTransformer> {
+ public:
+  TransformStreamTransformer() = default;
+  virtual ~TransformStreamTransformer() = default;
+
+  virtual void Transform(v8::Local<v8::Value> chunk,
+                         TransformStreamDefaultController*,
+                         ExceptionState&) = 0;
+  virtual void Flush(TransformStreamDefaultController*, ExceptionState&) = 0;
+
+  virtual void Trace(Visitor*) {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TransformStreamTransformer);
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_TRANSFORM_STREAM_TRANSFORMER_H_