Fix V8 bindings for named constructors to set prototype object correctly

third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl

Index: third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
diff --git a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl b/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
index 1d1db395fd020fa5787c018aa58ca4b1322fc3e4..9ccca5435d88a8694b7df0964a9063e9f01fb433 100644
--- a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
@@ -522,6 +522,8 @@ static void {{cpp_class}}OriginSafeMethodSetter(v8::Local<v8::Name> name, v8::Lo
 {% block named_constructor %}
 {% from 'methods.cpp.tmpl' import generate_constructor with context %}
 {% if named_constructor %}
+{% set parent_wrapper_type_info = '&V8%s::wrapperTypeInfo' % parent_interface
+                                  if parent_interface else '0' %}
 {% set active_scriptwrappable_inheritance =
     'InheritFromActiveScriptWrappable'
     if active_scriptwrappable else
@@ -532,7 +534,7 @@ static void {{cpp_class}}OriginSafeMethodSetter(v8::Local<v8::Name> name, v8::Lo
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wglobal-constructors"
 #endif
-const WrapperTypeInfo {{v8_class}}Constructor::wrapperTypeInfo = { gin::kEmbedderBlink, {{v8_class}}Constructor::domTemplate, {{v8_class}}::trace, {{v8_class}}::traceWrappers, 0, {{prepare_prototype_and_interface_object_func}}, "{{interface_name}}", 0, WrapperTypeInfo::WrapperTypeObjectPrototype, WrapperTypeInfo::{{wrapper_class_id}}, WrapperTypeInfo::{{active_scriptwrappable_inheritance}}, WrapperTypeInfo::{{event_target_inheritance}}, WrapperTypeInfo::{{lifetime}} };
+const WrapperTypeInfo {{v8_class}}Constructor::wrapperTypeInfo = { gin::kEmbedderBlink, {{v8_class}}Constructor::domTemplate, {{v8_class}}::trace, {{v8_class}}::traceWrappers, 0, {{prepare_prototype_and_interface_object_func}}, "{{interface_name}}", {{parent_wrapper_type_info}}, WrapperTypeInfo::WrapperTypeObjectPrototype, WrapperTypeInfo::{{wrapper_class_id}}, WrapperTypeInfo::{{active_scriptwrappable_inheritance}}, WrapperTypeInfo::{{event_target_inheritance}}, WrapperTypeInfo::{{lifetime}} };
 #if defined(COMPONENT_BUILD) && defined(WIN32) && COMPILER(CLANG)
 #pragma clang diagnostic pop
 #endif
@@ -548,12 +550,50 @@ v8::Local<v8::FunctionTemplate> {{v8_class}}Constructor::domTemplate(v8::Isolate
   result = v8::FunctionTemplate::New(isolate, {{v8_class}}ConstructorCallback);
   v8::Local<v8::ObjectTemplate> instanceTemplate = result->InstanceTemplate();
   instanceTemplate->SetInternalFieldCount({{v8_class}}::internalFieldCount);
-  result->SetClassName(v8AtomicString(isolate, "{{cpp_class}}"));
+  result->SetClassName(v8AtomicString(isolate, "{{named_constructor.name}}"));
   result->Inherit({{v8_class}}::domTemplate(isolate, world));
   data->setInterfaceTemplate(world, &domTemplateKey, result);
   return result;
 }
 
+void {{v8_class}}Constructor::NamedConstructorAttributeGetter(
+    v8::Local<v8::Name> propertyName,
+    const v8::PropertyCallbackInfo<v8::Value>& info) {
+  v8::Local<v8::Value> data = info.Data();

[Yuki, 2017/01/20 05:54:42]

You no longer use |data|.

[sashab, 2017/01/25 02:34:51]

Done

+  DCHECK(data->IsExternal());
+  V8PerContextData* perContextData =
+      V8PerContextData::from(info.Holder()->CreationContext());
+  if (!perContextData)
+    return;
+
+  v8::Local<v8::Function> namedConstructor = perContextData->constructorForType(&{{v8_class}}Constructor::wrapperTypeInfo);
+  v8::Local<v8::Function> interface = perContextData->constructorForType(&{{v8_class}}::wrapperTypeInfo);

[haraken, 2017/01/20 08:07:04]

Move this code into the if clause at line 576.

[sashab, 2017/01/25 02:34:51]

Oh yup nice, thx done.

+
+  // Set the prototype of named constructors to the regular constructor.
+  v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();

[haraken, 2017/01/20 08:07:04]

You're using info.Holder()->CreationContext() at line 565 whereas you're using
GetCurrentContext() at line 573. Which one is correct?

[Yuki, 2017/01/20 15:20:30]

Both are correct.

The interface obejct and prototype object should be created in the relevant
realm (= relevant context) of the receiver object (= holder object).  So it's
correct.

APIs for private property and get/set operations require a v8::Context, but it
doesn't create anything new, so using the current context is appropriate.

Having said that, it's a good idea to rename this variable to |currentContext|
so that we don't get confused.

[sashab, 2017/01/25 02:34:51]

Renamed to |currentContext| and only used info.Holder()->CreationContext().

+  auto privateProperty = V8PrivateProperty::getNamedConstructorInitialized(info.GetIsolate());
+  v8::Local<v8::Value> privateValue = privateProperty.get(context, namedConstructor);
+  if (privateValue.IsEmpty()) {
+    // Only Window exposes named constructors, so that will always be the interface name.
+    ExceptionState exceptionState(info.GetIsolate(), ExceptionState::GetterContext, "Window", "{{named_constructor.name}}");
+    v8::MaybeLocal<v8::Value> interfacePrototype = interface->Get(context, v8AtomicString(info.GetIsolate(), "prototype"));

[haraken, 2017/01/20 08:07:04]

Can you avoid using a MaybeLocal handle?

  Local<v8::Value> interfacePrototype;
  if (!interface->Get(...)->ToLocal(interfacePrototype)) {
    return;
  }
  ...;

[sashab, 2017/01/25 02:34:51]

Nice! Done.

+    if (interfacePrototype.IsEmpty()) {
+      exceptionState.throwTypeError("Could not get prototype for {{v8_class}}.");
+      return;
+    }
+
+    v8::Maybe<bool> setResult = namedConstructor->Set(context, v8AtomicString(info.GetIsolate(), "prototype"), interfacePrototype.ToLocalChecked());

[haraken, 2017/01/20 08:07:04]

Ditto. Avoid using Maybe. You can use To.

[sashab, 2017/01/25 02:34:51]

Done. Changed to:

if (namedConstructor->Set(currentContext, v8AtomicString(info.GetIsolate(),
"prototype"), interfacePrototype).To() != true) {

+    if (setResult.IsNothing() || setResult.ToChecked() != true) {
+      exceptionState.throwTypeError("Could not set prototype for {{named_constructor.name}}.");
+      return;
+    }
+
+    privateProperty.set(context, namedConstructor, v8::True(info.GetIsolate()));
+  }
+
+  v8SetReturnValue(info, namedConstructor);
+}
+
 {% endif %}
 {% endblock %}