v8binding: Make PaintWorkletGlobalScope use IDL callback function

Converts PaintWorkletGlobalScope.registerPaint's second argument
|paintCtor| into IDL callback function type.  Also, makes
Paint Worklet use |PaintCallback| of callback function type.

Change-Id: I6706499705a116c85f5c092a46309854db88708c
Reviewed-on: https://chromium-review.googlesource.com/c/1478850Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634582}

third_party/blink/renderer/bindings/modules/v8/generated.gni

@@ -103,6 +103,8 @@ generated_modules_callback_function_files = [
   "$bindings_modules_v8_output_dir/v8_navigator_user_media_success_callback.h",
   "$bindings_modules_v8_output_dir/v8_notification_permission_callback.cc",
   "$bindings_modules_v8_output_dir/v8_notification_permission_callback.h",
+  "$bindings_modules_v8_output_dir/v8_paint_callback.cc",
+  "$bindings_modules_v8_output_dir/v8_paint_callback.h",
   "$bindings_modules_v8_output_dir/v8_position_callback.cc",
   "$bindings_modules_v8_output_dir/v8_position_callback.h",
   "$bindings_modules_v8_output_dir/v8_position_error_callback.cc",

third_party/blink/renderer/modules/csspaint/css_paint_definition.cc

@@ -5,7 +5,10 @@
 #include "third_party/blink/renderer/modules/csspaint/css_paint_definition.h"
 
 #include <memory>
+
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_no_argument_constructor.h"
+#include "third_party/blink/renderer/bindings/modules/v8/v8_paint_callback.h"
 #include "third_party/blink/renderer/core/css/css_computed_style_declaration.h"
 #include "third_party/blink/renderer/core/css/cssom/prepopulated_computed_style_property_map.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
@@ -14,7 +17,6 @@
 #include "third_party/blink/renderer/modules/csspaint/paint_size.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/bindings/v8_binding_macros.h"
-#include "third_party/blink/renderer/platform/bindings/v8_object_constructor.h"
 #include "third_party/blink/renderer/platform/graphics/paint_generated_image.h"
 
 namespace blink {
@@ -33,8 +35,8 @@ FloatSize GetSpecifiedSize(const FloatSize& size, float zoom) {
 
 CSSPaintDefinition* CSSPaintDefinition::Create(
     ScriptState* script_state,
-    v8::Local<v8::Function> constructor,
-    v8::Local<v8::Function> paint,
+    V8NoArgumentConstructor* constructor,
+    V8PaintCallback* paint,
     const Vector<CSSPropertyID>& native_invalidation_properties,
     const Vector<AtomicString>& custom_invalidation_properties,
     const Vector<CSSSyntaxDescriptor>& input_argument_types,
@@ -46,15 +48,15 @@ CSSPaintDefinition* CSSPaintDefinition::Create(
 
 CSSPaintDefinition::CSSPaintDefinition(
     ScriptState* script_state,
-    v8::Local<v8::Function> constructor,
-    v8::Local<v8::Function> paint,
+    V8NoArgumentConstructor* constructor,
+    V8PaintCallback* paint,
     const Vector<CSSPropertyID>& native_invalidation_properties,
     const Vector<AtomicString>& custom_invalidation_properties,
     const Vector<CSSSyntaxDescriptor>& input_argument_types,
     const PaintRenderingContext2DSettings* context_settings)
     : script_state_(script_state),
-      constructor_(script_state->GetIsolate(), constructor),
-      paint_(script_state->GetIsolate(), paint),
+      constructor_(constructor),
+      paint_(paint),
       did_call_constructor_(false),
       context_settings_(context_settings) {
   native_invalidation_properties_ = native_invalidation_properties;
@@ -77,15 +79,13 @@ scoped_refptr<Image> CSSPaintDefinition::Paint(
   ScriptState::Scope scope(script_state_);
 
   MaybeCreatePaintInstance();
-
-  v8::Isolate* isolate = script_state_->GetIsolate();
-  v8::Local<v8::Value> instance = instance_.NewLocal(isolate);
-
-  // We may have failed to create an instance class, in which case produce an
+  // We may have failed to create an instance, in which case produce an
   // invalid image.
-  if (IsUndefinedOrNull(instance))
+  if (instance_.IsEmpty())
     return nullptr;
 
+  v8::Isolate* isolate = script_state_->GetIsolate();
+
   DCHECK(layout_object.GetNode());
   CanvasColorParams color_params;
   if (!context_settings_->alpha()) {
@@ -102,31 +102,19 @@ scoped_refptr<Image> CSSPaintDefinition::Paint(
           layout_object.GetNode(), native_invalidation_properties_,
           custom_invalidation_properties_);
 
-  Vector<v8::Local<v8::Value>, 4> argv;
-  if (paint_arguments) {
-    argv = {
-        ToV8(rendering_context, script_state_->GetContext()->Global(), isolate),
-        ToV8(paint_size, script_state_->GetContext()->Global(), isolate),
-        ToV8(style_map, script_state_->GetContext()->Global(), isolate),
-        ToV8(*paint_arguments, script_state_->GetContext()->Global(), isolate)};
-  } else {
-    argv = {
-        ToV8(rendering_context, script_state_->GetContext()->Global(), isolate),
-        ToV8(paint_size, script_state_->GetContext()->Global(), isolate),
-        ToV8(style_map, script_state_->GetContext()->Global(), isolate)};
-  }
-
-  v8::Local<v8::Function> paint = paint_.NewLocal(isolate);
-
-  v8::TryCatch block(isolate);
-  block.SetVerbose(true);
+  CSSStyleValueVector empty_paint_arguments;
+  if (!paint_arguments)
+    paint_arguments = &empty_paint_arguments;
 
-  V8ScriptRunner::CallFunction(paint, ExecutionContext::From(script_state_),
-                               instance, argv.size(), argv.data(), isolate);
+  v8::TryCatch try_catch(isolate);
+  try_catch.SetVerbose(true);
 
   // The paint function may have produced an error, in which case produce an
   // invalid image.
-  if (block.HasCaught()) {
+  if (paint_
+          ->Invoke(instance_.NewLocal(isolate), rendering_context, paint_size,
+                   style_map, *paint_arguments)
+          .IsNothing()) {
     return nullptr;
   }
 
@@ -137,20 +125,15 @@ scoped_refptr<Image> CSSPaintDefinition::Paint(
 void CSSPaintDefinition::MaybeCreatePaintInstance() {
   if (did_call_constructor_)
     return;
+  did_call_constructor_ = true;
 
   DCHECK(instance_.IsEmpty());
 
-  v8::Isolate* isolate = script_state_->GetIsolate();
-  v8::Local<v8::Function> constructor = constructor_.NewLocal(isolate);
-  DCHECK(!IsUndefinedOrNull(constructor));
-
-  v8::Local<v8::Value> paint_instance;
-  if (V8ScriptRunner::CallAsConstructor(
-          isolate, constructor, ExecutionContext::From(script_state_), 0, {})
-          .ToLocal(&paint_instance))
-    instance_.Set(isolate, paint_instance);
+  ScriptValue paint_instance;
+  if (!constructor_->Construct().To(&paint_instance))
+    return;
 
-  did_call_constructor_ = true;
+  instance_.Set(constructor_->GetIsolate(), paint_instance.V8Value());
 }
 
 void CSSPaintDefinition::Trace(Visitor* visitor) {

third_party/blink/renderer/modules/csspaint/css_paint_definition.h

@@ -12,6 +12,7 @@
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/platform/bindings/name_client.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
+#include "third_party/blink/renderer/platform/bindings/trace_wrapper_member.h"
 #include "third_party/blink/renderer/platform/bindings/trace_wrapper_v8_reference.h"
 #include "third_party/blink/renderer/platform/geometry/float_size.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
@@ -20,8 +21,10 @@
 namespace blink {
 
 class Image;
-class ScriptState;
 class ImageResourceObserver;
+class ScriptState;
+class V8NoArgumentConstructor;
+class V8PaintCallback;
 
 // Represents a javascript class registered on the PaintWorkletGlobalScope by
 // the author. It will store the properties for invalidation and input argument
@@ -32,8 +35,8 @@ class MODULES_EXPORT CSSPaintDefinition final
  public:
   static CSSPaintDefinition* Create(
       ScriptState*,
-      v8::Local<v8::Function> constructor,
-      v8::Local<v8::Function> paint,
+      V8NoArgumentConstructor* constructor,
+      V8PaintCallback* paint,
       const Vector<CSSPropertyID>&,
       const Vector<AtomicString>& custom_invalidation_properties,
       const Vector<CSSSyntaxDescriptor>& input_argument_types,
@@ -41,8 +44,8 @@ class MODULES_EXPORT CSSPaintDefinition final
 
   CSSPaintDefinition(
       ScriptState*,
-      v8::Local<v8::Function> constructor,
-      v8::Local<v8::Function> paint,
+      V8NoArgumentConstructor* constructor,
+      V8PaintCallback* paint,
       const Vector<CSSPropertyID>& native_invalidation_properties,
       const Vector<AtomicString>& custom_invalidation_properties,
       const Vector<CSSSyntaxDescriptor>& input_argument_types,
@@ -76,13 +79,6 @@ class MODULES_EXPORT CSSPaintDefinition final
 
   ScriptState* GetScriptState() const { return script_state_; }
 
-  v8::Local<v8::Function> ConstructorForTesting(v8::Isolate* isolate) {
-    return constructor_.NewLocal(isolate);
-  }
-  v8::Local<v8::Function> PaintFunctionForTesting(v8::Isolate* isolate) {
-    return paint_.NewLocal(isolate);
-  }
-
   virtual void Trace(blink::Visitor* visitor);
   const char* NameInHeapSnapshot() const override {
     return "CSSPaintDefinition";
@@ -96,8 +92,8 @@ class MODULES_EXPORT CSSPaintDefinition final
   // This object keeps the class instance object, constructor function and
   // paint function alive. It participates in wrapper tracing as it holds onto
   // V8 wrappers.
-  TraceWrapperV8Reference<v8::Function> constructor_;
-  TraceWrapperV8Reference<v8::Function> paint_;
+  TraceWrapperMember<V8NoArgumentConstructor> constructor_;
+  TraceWrapperMember<V8PaintCallback> paint_;
 
   // At the moment there is only ever one instance of a paint class per type.
   TraceWrapperV8Reference<v8::Value> instance_;

third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc

@@ -6,8 +6,10 @@
 
 #include "third_party/blink/renderer/bindings/core/v8/idl_types.h"
 #include "third_party/blink/renderer/bindings/core/v8/native_value_traits_impl.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_no_argument_constructor.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_object_parser.h"
 #include "third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h"
+#include "third_party/blink/renderer/bindings/modules/v8/v8_paint_callback.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_paint_rendering_context_2d_settings.h"
 #include "third_party/blink/renderer/core/css/css_property_names.h"
 #include "third_party/blink/renderer/core/css/css_syntax_descriptor.h"
@@ -23,6 +25,7 @@
 #include "third_party/blink/renderer/modules/csspaint/css_paint_worklet.h"
 #include "third_party/blink/renderer/modules/csspaint/paint_worklet.h"
 #include "third_party/blink/renderer/modules/csspaint/paint_worklet_proxy_client.h"
+#include "third_party/blink/renderer/platform/bindings/callback_method_retriever.h"
 #include "third_party/blink/renderer/platform/bindings/v8_binding_macros.h"
 
 namespace blink {
@@ -30,7 +33,7 @@ namespace blink {
 namespace {
 
 bool ParseInputArguments(v8::Local<v8::Context> context,
-                         v8::Local<v8::Function> constructor,
+                         v8::Local<v8::Object> constructor,
                          Vector<CSSSyntaxDescriptor>* input_argument_types,
                          ExceptionState* exception_state) {
   v8::Isolate* isolate = context->GetIsolate();
@@ -67,7 +70,7 @@ bool ParseInputArguments(v8::Local<v8::Context> context,
 
 PaintRenderingContext2DSettings* ParsePaintRenderingContext2DSettings(
     v8::Local<v8::Context> context,
-    v8::Local<v8::Function> constructor,
+    v8::Local<v8::Object> constructor,
     ExceptionState* exception_state) {
   v8::Isolate* isolate = context->GetIsolate();
   v8::TryCatch block(isolate);
@@ -149,10 +152,11 @@ void PaintWorkletGlobalScope::Dispose() {
   WorkletGlobalScope::Dispose();
 }
 
-void PaintWorkletGlobalScope::registerPaint(
-    const String& name,
-    const ScriptValue& constructor_value,
-    ExceptionState& exception_state) {
+void PaintWorkletGlobalScope::registerPaint(const String& name,
+                                            V8NoArgumentConstructor* paint_ctor,
+                                            ExceptionState& exception_state) {
+  // https://drafts.css-houdini.org/css-paint-api/#dom-paintworkletglobalscope-registerpaint
+
   RegisterWithProxyClientIfNeeded();
 
   if (name.IsEmpty()) {
@@ -162,22 +166,20 @@ void PaintWorkletGlobalScope::registerPaint(
 
   if (paint_definitions_.Contains(name)) {
     exception_state.ThrowDOMException(
-        DOMExceptionCode::kNotSupportedError,
+        DOMExceptionCode::kInvalidModificationError,
         "A class with name:'" + name + "' is already registered.");
     return;
   }
 
   v8::Local<v8::Context> context = ScriptController()->GetContext();
 
-  DCHECK(constructor_value.V8Value()->IsFunction());
-  v8::Local<v8::Function> constructor =
-      v8::Local<v8::Function>::Cast(constructor_value.V8Value());
+  v8::Local<v8::Object> v8_paint_ctor = paint_ctor->CallbackObject();
 
   Vector<CSSPropertyID> native_invalidation_properties;
   Vector<AtomicString> custom_invalidation_properties;
 
   if (!V8ObjectParser::ParseCSSPropertyList(
-          context, constructor, "inputProperties",
+          context, v8_paint_ctor, "inputProperties",
           &native_invalidation_properties, &custom_invalidation_properties,
           &exception_state))
     return;
@@ -185,33 +187,35 @@ void PaintWorkletGlobalScope::registerPaint(
   // Get input argument types. Parse the argument type values only when
   // cssPaintAPIArguments is enabled.
   Vector<CSSSyntaxDescriptor> input_argument_types;
-  if (!ParseInputArguments(context, constructor, &input_argument_types,
+  if (!ParseInputArguments(context, v8_paint_ctor, &input_argument_types,
                            &exception_state))
     return;
 
   PaintRenderingContext2DSettings* context_settings =
-      ParsePaintRenderingContext2DSettings(context, constructor,
+      ParsePaintRenderingContext2DSettings(context, v8_paint_ctor,
                                            &exception_state);
   if (!context_settings)
     return;
 
-  v8::Local<v8::Object> prototype;
-  if (!V8ObjectParser::ParsePrototype(context, constructor, &prototype,
-                                      &exception_state))
+  CallbackMethodRetriever retriever(paint_ctor);
+
+  retriever.GetPrototypeObject(exception_state);
+  if (exception_state.HadException())
     return;
 
-  v8::Local<v8::Function> paint;
-  if (!V8ObjectParser::ParseFunction(context, prototype, "paint", &paint,
-                                     &exception_state))
+  v8::Local<v8::Function> v8_paint =
+      retriever.GetMethodOrThrow("paint", exception_state);
+  if (exception_state.HadException())
     return;
+  V8PaintCallback* paint = V8PaintCallback::Create(v8_paint);
 
   CSSPaintDefinition* definition = CSSPaintDefinition::Create(
-      ScriptController()->GetScriptState(), constructor, paint,
+      ScriptController()->GetScriptState(), paint_ctor, paint,
       native_invalidation_properties, custom_invalidation_properties,
       input_argument_types, context_settings);
   paint_definitions_.Set(name, definition);
 
-  // TODO(xidachen): the following steps should be done with a postTask when
+  // TODO(xidachen): the following steps should be done with a PostTask when
   // we move PaintWorklet off main thread.
   if (!RuntimeEnabledFeatures::OffMainThreadCSSPaintEnabled()) {
     PaintWorklet* paint_worklet =

third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.h

@@ -17,6 +17,7 @@ namespace blink {
 
 class CSSPaintDefinition;
 class ExceptionState;
+class V8NoArgumentConstructor;
 class WorkerReportingProxy;
 
 class MODULES_EXPORT PaintWorkletGlobalScope final : public WorkletGlobalScope {
@@ -48,7 +49,7 @@ class MODULES_EXPORT PaintWorkletGlobalScope final : public WorkletGlobalScope {
 
   bool IsPaintWorkletGlobalScope() const final { return true; }
   void registerPaint(const String& name,
-                     const ScriptValue& constructor_value,
+                     V8NoArgumentConstructor* paint_ctor,
                      ExceptionState&);
 
   CSSPaintDefinition* FindDefinition(const String& name);

third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.idl

@@ -9,7 +9,9 @@
     Global=(Worklet,PaintWorklet)
 ] interface PaintWorkletGlobalScope : WorkletGlobalScope {
     [Measure] readonly attribute unrestricted double devicePixelRatio;
-    // TODO(yukishiino): |paintCtor| should be of callback function type
-    // (should be: callback T = any ()).
-    [Measure, RaisesException] void registerPaint(DOMString name, CallbackFunctionTreatedAsScriptValue paintCtor);
+    [Measure, RaisesException] void registerPaint(DOMString name, NoArgumentConstructor paintCtor);
 };
+
+// Blink-specific type for paint function
+// https://drafts.css-houdini.org/css-paint-api/#paint-definition-paint-function
+callback PaintCallback = void (PaintRenderingContext2D renderingContext, PaintSize paintSize, StylePropertyMapReadOnly styleMap, sequence<CSSStyleValue> inputArguments);

third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_test.cc

@@ -138,9 +138,6 @@ class PaintWorkletGlobalScopeTest : public PageTestBase {
 
       CSSPaintDefinition* definition = global_scope->FindDefinition("test");
       ASSERT_TRUE(definition);
-
-      EXPECT_TRUE(definition->ConstructorForTesting(isolate)->IsFunction());
-      EXPECT_TRUE(definition->PaintFunctionForTesting(isolate)->IsFunction());
     }
 
     {

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-002.https.html

@@ -25,7 +25,8 @@ try {
         }
     });
 } catch(ex) {
-    if (ex.name == "TypeError" && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The provided value cannot be converted to a sequence.")
+    // 'inputArguments' property is a string, not a sequence.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-003.https.html

@@ -25,7 +25,8 @@ try {
         }
     });
 } catch(ex) {
-    if (ex.name == "TypeError" && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': Invalid argument types.")
+    // 'non-sense-type' is not a proper name of CSS properties.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-005.https.html

@@ -22,7 +22,8 @@ try {
     registerPaint('foo', class { paint() { } });
     registerPaint('foo', class { paint() { } });
 } catch(ex) {
-    if (ex.name == "NotSupportedError" && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': A class with name:'foo' is already registered.")
+    // 'foo' gets registered twice.
+    if (ex.name == 'InvalidModificationError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-006.https.html

@@ -21,7 +21,8 @@ var testsPassed = false;
 try {
     registerPaint('', class { });
 } catch(ex) {
-    if (ex.name == 'TypeError' && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The empty string is not a valid name.")
+    // The empty string is not a valid name.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-008.https.html

@@ -25,7 +25,8 @@ try {
         }
     });
 } catch(ex) {
-    if (ex.name == 'TypeError' && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The provided value cannot be converted to a sequence.")
+    // 'inputProperties' property is 42, not a sequence<DOMString>.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-009.https.html

@@ -23,7 +23,8 @@ try {
     a.prototype = undefined;
     registerPaint('foo5', a);
 } catch(ex) {
-    if (ex.name == 'TypeError' && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The 'prototype' object on the class does not exist.")
+    // The prototype object is undefined, not an object.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-010.https.html

@@ -23,7 +23,8 @@ try {
     b.prototype = 42;
     registerPaint('foo6', b);
 } catch(ex) {
-    if (ex.name == 'TypeError' && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The 'prototype' property on the class is not an object.")
+    // The prototype object is 42, not an object.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-011.https.html

@@ -21,7 +21,8 @@ var testsPassed = false;
 try {
     registerPaint('foo7', class { });
 } catch(ex) {
-    if (ex.name == 'TypeError' && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The 'paint' property on the prototype does not exist.")
+    // 'paint' property doesn't exist on the prototype chain.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-012.https.html

@@ -25,7 +25,8 @@ try {
         }
     });
 } catch(ex) {
-    if (ex.name == 'TypeError' && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': The 'paint' property on the prototype is not a function.")
+    // 'paint' property's value is 42, not a function.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }
 

third_party/blink/web_tests/external/wpt/css/css-paint-api/parse-input-arguments-016.https.html

@@ -25,7 +25,8 @@ try {
         }
     });
 } catch(ex) {
-    if (ex.name == "TypeError" && ex.message == "Failed to execute 'registerPaint' on 'PaintWorkletGlobalScope': cannot convert to dictionary.")
+    // 'contextOptions' property is 42, not a dictionary.
+    if (ex.name == 'TypeError')
         testsPassed = true;
 }