Fix V8 bindings for named constructors to set prototype object correctly

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

 {% extends 'interface_base.cpp.tmpl' %}
 
 {% set has_prepare_prototype_and_interface_object =
     unscopables or has_conditional_attributes_on_prototype or
     methods | conditionally_exposed(is_partial) %}
 {% set prepare_prototype_and_interface_object_func =
     '%s::preparePrototypeAndInterfaceObject' % v8_class
     if has_prepare_prototype_and_interface_object
     else 'nullptr' %}
 
@@ skipping 504 matching lines @@
 }
 
 {% endif %}
 {% endblock %}
 
 
 {##############################################################################}
 {% 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
     'NotInheritFromActiveScriptWrappable' %}
 // Suppress warning: global constructors, because struct WrapperTypeInfo is trivial
 // and does not depend on another global objects.
 #if defined(COMPONENT_BUILD) && defined(WIN32) && COMPILER(CLANG)
 #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
 
 {{generate_constructor(named_constructor)}}
 v8::Local<v8::FunctionTemplate> {{v8_class}}Constructor::domTemplate(v8::Isolate* isolate, const DOMWrapperWorld& world) {
   static int domTemplateKey; // This address is used for a key to look up the dom template.
   V8PerIsolateData* data = V8PerIsolateData::from(isolate);
   v8::Local<v8::FunctionTemplate> result = data->findInterfaceTemplate(world, &domTemplateKey);
   if (!result.IsEmpty())
     return result;
 
   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 %}
 
 {##############################################################################}
 {% block overloaded_constructor %}
 {% if constructor_overloads %}
 static void constructor(const v8::FunctionCallbackInfo<v8::Value>& info) {
   ExceptionState exceptionState(info.GetIsolate(), ExceptionState::ConstructionContext, "{{interface_name}}");
   {# 2. Initialize argcount to be min(maxarg, n). #}
   switch (std::min({{constructor_overloads.maxarg}}, info.Length())) {
@@ skipping 389 matching lines @@
 }
 
 {% for method in methods if method.overloads and method.overloads.has_partial_overloads %}
 void {{v8_class}}::register{{method.name | blink_capitalize}}MethodForPartialInterface(void (*method)(const v8::FunctionCallbackInfo<v8::Value>&)) {
   {{cpp_class}}V8Internal::{{method.name}}MethodForPartialInterface = method;
 }
 
 {% endfor %}
 {% endif %}
 {% endblock %}