Streams: Update queuing strategies to match standard

Update the IDL for ByteLengthQueuingStrategy and CountQueuingStrategy
to match the standard.

The most noticeable semantic difference is that incorrect calls like
`new CountQueuingStrategy(10)`, which previously silently gave
you the default highWaterMark, will now throw an exception.

Also cache the "size" function on the global object. Add a test that
iframes have a different "size" function, since they have a different
global object.

Change-Id: I9b6763944c0abc278451c28a4fcf615f624e1fdb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2104815
Reviewed-by: Victor Costan <[email protected]>
Reviewed-by: Yutaka Hirano <[email protected]>
Commit-Queue: Adam Rice <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#788474}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 90cddd79b11a9f7ce4dd3dbb2caaeca5041efd54

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

@@ -14,7 +14,7 @@ void ScriptFunction::Trace(Visitor* visitor) const {
   CustomWrappableAdapter::Trace(visitor);
 }
 
-v8::Local<v8::Function> ScriptFunction::BindToV8Function() {
+v8::Local<v8::Function> ScriptFunction::BindToV8Function(int length) {
 #if DCHECK_IS_ON()
   DCHECK(!bind_to_v8_function_already_called_);
   bind_to_v8_function_already_called_ = true;
@@ -24,7 +24,7 @@ v8::Local<v8::Function> ScriptFunction::BindToV8Function() {
   // The wrapper is held alive by the CallHandlerInfo internally in V8 as long
   // as the function is alive.
   return v8::Function::New(script_state_->GetContext(), CallCallback, wrapper,
-                           0, v8::ConstructorBehavior::kThrow)
+                           length, v8::ConstructorBehavior::kThrow)
       .ToLocalChecked();
 }
 

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

@@ -64,7 +64,9 @@ class CORE_EXPORT ScriptFunction : public CustomWrappableAdapter {
 
   ScriptState* GetScriptState() const { return script_state_; }
 
-  v8::Local<v8::Function> BindToV8Function();
+  // It is not usually necessary to set |length| unless the function is exposed
+  // to JavaScript.
+  v8::Local<v8::Function> BindToV8Function(int length = 0);
 
  private:
   // Subclasses should implement one of Call() or CallRaw(). Most will implement

blink/renderer/core/streams/byte_length_queuing_strategy.cc

@@ -16,13 +16,27 @@ namespace blink {
 
 namespace {
 
+static const V8PrivateProperty::SymbolKey
+    kByteLengthQueuingStrategySizeFunction;
+
 class ByteLengthQueuingStrategySizeFunction final : public ScriptFunction {
  public:
   static v8::Local<v8::Function> CreateFunction(ScriptState* script_state) {
     ByteLengthQueuingStrategySizeFunction* self =
         MakeGarbageCollected<ByteLengthQueuingStrategySizeFunction>(
             script_state);
-    return self->BindToV8Function();
+
+    // https://streams.spec.whatwg.org/#byte-length-queuing-strategy-size-function
+
+    // 2. Let F be ! CreateBuiltinFunction(steps, « », globalObject’s relevant
+    //    Realm).
+    // 4. Perform ! SetFunctionLength(F, 1).
+    v8::Local<v8::Function> function = self->BindToV8Function(/*length=*/1);
+
+    // 3. Perform ! SetFunctionName(F, "size").
+    function->SetName(V8String(script_state->GetIsolate(), "size"));
+
+    return function;
   }
 
   explicit ByteLengthQueuingStrategySizeFunction(ScriptState* script_state)
@@ -40,8 +54,10 @@ class ByteLengthQueuingStrategySizeFunction final : public ScriptFunction {
       chunk = args[0];
     }
 
-    // https://streams.spec.whatwg.org/#blqs-size
-    // 1. Return ? GetV(chunk, "byteLength").
+    // https://streams.spec.whatwg.org/#byte-length-queuing-strategy-size-function
+
+    // 1. Let steps be the following steps, given chunk:
+    //   1. Return ? GetV(chunk, "byteLength").
 
     // https://tc39.es/ecma262/#sec-getv
     // 1. Assert: IsPropertyKey(P) is true.
@@ -74,28 +90,18 @@ ByteLengthQueuingStrategy* ByteLengthQueuingStrategy::Create(
 ByteLengthQueuingStrategy::ByteLengthQueuingStrategy(
     ScriptState* script_state,
     const QueuingStrategyInit* init)
-    : high_water_mark_(script_state->GetIsolate(),
-                       HighWaterMarkOrUndefined(script_state, init)) {}
+    : high_water_mark_(init->highWaterMark()) {}
 
 ByteLengthQueuingStrategy::~ByteLengthQueuingStrategy() = default;
 
-ScriptValue ByteLengthQueuingStrategy::highWaterMark(
-    ScriptState* script_state) const {
-  return ScriptValue(script_state->GetIsolate(),
-                     high_water_mark_.NewLocal(script_state->GetIsolate()));
-}
-
 ScriptValue ByteLengthQueuingStrategy::size(ScriptState* script_state) const {
-  // We don't cache the result because normally this method will only be called
-  // once anyway.
-  return ScriptValue(
-      script_state->GetIsolate(),
-      ByteLengthQueuingStrategySizeFunction::CreateFunction(script_state));
-}
-
-void ByteLengthQueuingStrategy::Trace(Visitor* visitor) const {
-  visitor->Trace(high_water_mark_);
-  ScriptWrappable::Trace(visitor);
+  // https://streams.spec.whatwg.org/#byte-length-queuing-strategy-size-function
+  // 5. Set globalObject’s byte length queuing strategy size function to a
+  //    Function that represents a reference to F, with callback context equal
+  //    to globalObject’s relevant settings object.
+  return GetCachedSizeFunction(
+      script_state, kByteLengthQueuingStrategySizeFunction,
+      &ByteLengthQueuingStrategySizeFunction::CreateFunction);
 }
 
 }  // namespace blink

blink/renderer/core/streams/byte_length_queuing_strategy.h

@@ -14,7 +14,6 @@ namespace blink {
 class QueuingStrategyInit;
 class ScriptState;
 class ScriptValue;
-class Visitor;
 
 // https://streams.spec.whatwg.org/#blqs-class
 class ByteLengthQueuingStrategy final : public ScriptWrappable {
@@ -27,13 +26,11 @@ class ByteLengthQueuingStrategy final : public ScriptWrappable {
   ByteLengthQueuingStrategy(ScriptState*, const QueuingStrategyInit*);
   ~ByteLengthQueuingStrategy() override;
 
-  ScriptValue highWaterMark(ScriptState*) const;
+  double highWaterMark() const { return high_water_mark_; }
   ScriptValue size(ScriptState*) const;
 
-  void Trace(Visitor*) const override;
-
  private:
-  const TraceWrapperV8Reference<v8::Value> high_water_mark_;
+  const double high_water_mark_;
 };
 
 }  // namespace blink

blink/renderer/core/streams/byte_length_queuing_strategy.idl

@@ -7,9 +7,9 @@
 [
     Exposed=(Window,Worker,Worklet)
 ] interface ByteLengthQueuingStrategy {
-    [CallWith=ScriptState, MeasureAs=ByteLengthQueuingStrategyConstructor] constructor([PermissiveDictionaryConversion] QueuingStrategyInit init);
-    [CallWith=ScriptState] readonly attribute any highWaterMark;
+    [CallWith=ScriptState, MeasureAs=ByteLengthQueuingStrategyConstructor] constructor(QueuingStrategyInit init);
+    readonly attribute unrestricted double highWaterMark;
 
-    // size is an accessor that returns a function.
+    // The standard specifies "Function" but the IDL compiler doesn't like it.
     [CallWith=ScriptState] readonly attribute any size;
 };

blink/renderer/core/streams/count_queuing_strategy.cc

@@ -15,21 +15,34 @@ namespace blink {
 
 namespace {
 
+static const V8PrivateProperty::SymbolKey kCountQueuingStrategySizeFunction;
+
 class CountQueuingStrategySizeFunction final : public ScriptFunction {
  public:
   static v8::Local<v8::Function> CreateFunction(ScriptState* script_state) {
     CountQueuingStrategySizeFunction* self =
         MakeGarbageCollected<CountQueuingStrategySizeFunction>(script_state);
-    return self->BindToV8Function();
+
+    // https://streams.spec.whatwg.org/#count-queuing-strategy-size-function
+    // 2. Let F be ! CreateBuiltinFunction(steps, « », globalObject’s relevant
+    //    Realm).
+    // 4. Perform ! SetFunctionLength(F, 0).
+    v8::Local<v8::Function> function = self->BindToV8Function(/*length=*/0);
+
+    // 3. Perform ! SetFunctionName(F, "size").
+    function->SetName(V8String(script_state->GetIsolate(), "size"));
+
+    return function;
   }
 
   explicit CountQueuingStrategySizeFunction(ScriptState* script_state)
       : ScriptFunction(script_state) {}
 
  private:
   void CallRaw(const v8::FunctionCallbackInfo<v8::Value>& args) override {
-    // https://streams.spec.whatwg.org/#cqs-size
-    // 1. Return 1.
+    // https://streams.spec.whatwg.org/#count-queuing-strategy-size-function
+    // 1. Let steps be the following steps:
+    //   1. Return 1.
     args.GetReturnValue().Set(
         v8::Integer::New(GetScriptState()->GetIsolate(), 1));
   }
@@ -45,28 +58,18 @@ CountQueuingStrategy* CountQueuingStrategy::Create(
 
 CountQueuingStrategy::CountQueuingStrategy(ScriptState* script_state,
                                            const QueuingStrategyInit* init)
-    : high_water_mark_(script_state->GetIsolate(),
-                       HighWaterMarkOrUndefined(script_state, init)) {}
+    : high_water_mark_(init->highWaterMark()) {}
 
 CountQueuingStrategy::~CountQueuingStrategy() = default;
 
-ScriptValue CountQueuingStrategy::highWaterMark(
-    ScriptState* script_state) const {
-  return ScriptValue(script_state->GetIsolate(),
-                     high_water_mark_.NewLocal(script_state->GetIsolate()));
-}
-
 ScriptValue CountQueuingStrategy::size(ScriptState* script_state) const {
-  // We don't cache the result because normally this method will only be called
-  // once anyway.
-  return ScriptValue(
-      script_state->GetIsolate(),
-      CountQueuingStrategySizeFunction::CreateFunction(script_state));
-}
-
-void CountQueuingStrategy::Trace(Visitor* visitor) const {
-  visitor->Trace(high_water_mark_);
-  ScriptWrappable::Trace(visitor);
+  // https://streams.spec.whatwg.org/#count-queuing-strategy-size-function
+  // 5. Set globalObject’s count queuing strategy size function to a Function
+  //    that represents a reference to F, with callback context equal to
+  //    globalObject’s relevant settings object.
+  return GetCachedSizeFunction(
+      script_state, kCountQueuingStrategySizeFunction,
+      &CountQueuingStrategySizeFunction::CreateFunction);
 }
 
 }  // namespace blink

blink/renderer/core/streams/count_queuing_strategy.h

@@ -15,7 +15,6 @@ namespace blink {
 class QueuingStrategyInit;
 class ScriptState;
 class ScriptValue;
-class Visitor;
 
 // https://streams.spec.whatwg.org/#blqs-class
 class CORE_EXPORT CountQueuingStrategy final : public ScriptWrappable {
@@ -27,13 +26,11 @@ class CORE_EXPORT CountQueuingStrategy final : public ScriptWrappable {
   CountQueuingStrategy(ScriptState*, const QueuingStrategyInit*);
   ~CountQueuingStrategy() override;
 
-  ScriptValue highWaterMark(ScriptState*) const;
+  double highWaterMark() const { return high_water_mark_; }
   ScriptValue size(ScriptState*) const;
 
-  void Trace(Visitor*) const override;
-
  private:
-  const TraceWrapperV8Reference<v8::Value> high_water_mark_;
+  const double high_water_mark_;
 };
 
 }  // namespace blink

blink/renderer/core/streams/count_queuing_strategy.idl

@@ -7,9 +7,9 @@
 [
     Exposed=(Window,Worker,Worklet)
 ] interface CountQueuingStrategy {
-    [CallWith=ScriptState, MeasureAs=CountQueuingStrategyConstructor] constructor([PermissiveDictionaryConversion] QueuingStrategyInit init);
-    [CallWith=ScriptState] readonly attribute any highWaterMark;
+    [CallWith=ScriptState, MeasureAs=CountQueuingStrategyConstructor] constructor(QueuingStrategyInit init);
+    readonly attribute unrestricted double highWaterMark;
 
-    // size is an accessor that returns a function.
+    // The standard specifies "Function" but the IDL compiler doesn't like it.
     [CallWith=ScriptState] readonly attribute any size;
 };

blink/renderer/core/streams/queuing_strategy_common.cc

@@ -4,20 +4,26 @@
 
 #include "third_party/blink/renderer/core/streams/queuing_strategy_common.h"
 
+#include "third_party/blink/renderer/bindings/core/v8/script_value.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_queuing_strategy_init.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 
 namespace blink {
 
-v8::Local<v8::Value> HighWaterMarkOrUndefined(ScriptState* script_state,
-                                              const QueuingStrategyInit* init) {
-  v8::Local<v8::Value> high_water_mark;
-  if (init->hasHighWaterMark()) {
-    high_water_mark = init->highWaterMark().V8Value();
-  } else {
-    high_water_mark = v8::Undefined(script_state->GetIsolate());
+ScriptValue GetCachedSizeFunction(ScriptState* script_state,
+                                  const V8PrivateProperty::SymbolKey& key,
+                                  SizeFunctionFactory factory) {
+  auto* isolate = script_state->GetIsolate();
+  auto function_cache = V8PrivateProperty::GetSymbol(isolate, key);
+  v8::Local<v8::Object> global_proxy = script_state->GetContext()->Global();
+  v8::Local<v8::Value> function;
+  if (!function_cache.GetOrUndefined(global_proxy).ToLocal(&function) ||
+      function->IsUndefined()) {
+    function = factory(script_state);
+    bool is_set = function_cache.Set(global_proxy, function);
+    DCHECK(is_set || isolate->IsExecutionTerminating());
   }
-  return high_water_mark;
+  return ScriptValue(isolate, function);
 }
 
 }  // namespace blink

blink/renderer/core/streams/queuing_strategy_common.h

@@ -5,15 +5,21 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_QUEUING_STRATEGY_COMMON_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_STREAMS_QUEUING_STRATEGY_COMMON_H_
 
+#include "third_party/blink/renderer/platform/bindings/v8_private_property.h"
 #include "v8/include/v8.h"
 
 namespace blink {
 
 class ScriptState;
-class QueuingStrategyInit;
+class ScriptValue;
 
-v8::Local<v8::Value> HighWaterMarkOrUndefined(ScriptState* script_state,
-                                              const QueuingStrategyInit* init);
+using SizeFunctionFactory = v8::Local<v8::Function> (*)(ScriptState*);
+
+// Returns the value cached on the global proxy object under |key|, or, if that
+// is not set, caches and returns the result of calling |factory|.
+ScriptValue GetCachedSizeFunction(ScriptState*,
+                                  const V8PrivateProperty::SymbolKey& key,
+                                  SizeFunctionFactory factory);
 
 }  // namespace blink
 

blink/renderer/core/streams/queuing_strategy_init.idl

@@ -2,9 +2,8 @@
 // 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/#cqs-class
+// https://streams.spec.whatwg.org/#dictdef-queuingstrategyinit
 
-[PermissiveDictionaryConversion]
 dictionary QueuingStrategyInit {
-  any highWaterMark;
+  required unrestricted double highWaterMark;
 };

blink/renderer/core/streams/writable_stream.cc

@@ -203,8 +203,7 @@ WritableStream* WritableStream::CreateWithCountQueueingStrategy(
   // introduces unnecessary trips through V8. Implement algorithms based on an
   // UnderlyingSinkBase.
   auto* init = QueuingStrategyInit::Create();
-  init->setHighWaterMark(
-      ScriptValue::From(script_state, static_cast<double>(high_water_mark)));
+  init->setHighWaterMark(static_cast<double>(high_water_mark));
   auto* strategy = CountQueuingStrategy::Create(script_state, init);
   ScriptValue strategy_value = ScriptValue::From(script_state, strategy);
   if (strategy_value.IsEmpty())

blink/renderer/modules/native_file_system/native_file_system_writable_file_stream.cc

@@ -36,8 +36,7 @@ NativeFileSystemWritableFileStream* NativeFileSystemWritableFileStream::Create(
   auto* init = QueuingStrategyInit::Create();
   // HighWaterMark set to 1 here. This allows the stream to appear available
   // without adding additional buffering.
-  init->setHighWaterMark(
-      ScriptValue::From(script_state, static_cast<double>(1)));
+  init->setHighWaterMark(1);
   auto* strategy = CountQueuingStrategy::Create(script_state, init);
   ScriptValue strategy_value = ScriptValue::From(script_state, strategy);