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 530 matching lines @@
 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}}"));

[Yuki, 2017/01/19 09:57:28]

You may want to change the class name to "{{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) {
+  ExceptionState exceptionState(info.GetIsolate(), ExceptionState::ConstructionContext, "{{interface_name}}");

[Yuki, 2017/01/19 09:57:28]

I think this should be ExceptionState::GetterContext because this is the getter
callback for named constructor.  Also you should specify
"{{named_constructor.name}}" as a propertyName argument.

And let's move this declaration into the following if-clause because you need to
throw an exception only inside the if-clause.

[Yuki, 2017/01/19 10:09:30]

And the interface name should be "Window" instead of "{{interface_name}}" though
it's unfortunate to have to hard-code here.

Named constructors are exposed as
    window.Image
where "Window" is an interface name of |window| and "Image" is a property name.

Hard-coding is not great, but we know that only Window exposes named
constructors, so the interface name must be "Window".

[sashab, 2017/01/20 04:37:05]

Hard-coded window, added propertyName argument and moved declaration into
if-statement. Also added comment:
// Only Window exposes named constructors, so that will always be the interface
name.

+  v8::Local<v8::Value> data = info.Data();
+  DCHECK(data->IsExternal());
+  V8PerContextData* perContextData =
+      V8PerContextData::from(info.Holder()->CreationContext());
+  if (!perContextData)
+    return;
+
+  v8::Local<v8::Function> namedConstructorInterface = perContextData->constructorForType(WrapperTypeInfo::unwrap(data));

[Yuki, 2017/01/19 09:57:28]

Exactly speaking, this is a "named constructor" and not a DOM interface (while
HTMLImageElement is a DOM interface).  So let's name this "namedConstructor".

[Yuki, 2017/01/19 09:57:28]

Now this function is specific to {{v8_class}} and you know that you want to use
{{v8_class}}Constructor::wrapperTypeInfo defined above, you no longer need to
use |data| here.

[sashab, 2017/01/20 04:37:05]

Done the name change, and removed use of data :)

+  v8::Local<v8::Function> interface = perContextData->constructorForType(&{{v8_class}}::wrapperTypeInfo);
+
+  // Set the prototype of named constructors to the regular constructor.
+  v8::Local<v8::Context> context = info.GetIsolate()->GetCurrentContext();
+  auto privateProperty = V8PrivateProperty::getNamedConstructorInitialized(info.GetIsolate());
+  v8::Local<v8::Value> privateValue = privateProperty.get(context, namedConstructorInterface);
+  if (privateValue.IsEmpty()) {
+    v8::MaybeLocal<v8::Value> interfacePrototype = interface->Get(context, v8AtomicString(info.GetIsolate(), "prototype"));
+    if (interfacePrototype.IsEmpty())
+      exceptionState.throwTypeError("Could not get prototype for {{v8_class}}.");

[Yuki, 2017/01/19 09:57:28]

Please return immediately right after throwing an exception.

[sashab, 2017/01/20 04:37:05]

Done

+
+    v8::Maybe<bool> setResult = namedConstructorInterface->Set(context, v8AtomicString(info.GetIsolate(), "prototype"), interfacePrototype.ToLocalChecked());
+    if (setResult.IsNothing() || setResult.ToChecked() != true) {
+      exceptionState.throwTypeError("Could not set prototype for {{interface_name}}.");

[Yuki, 2017/01/19 09:57:28]

Please return immediately after throwing an exception.
And the message should be "set prototype for {{named_constructor.name}}" because
you're setting the prototype object on the named constructor, I think?

[sashab, 2017/01/20 04:37:05]

Yes, done.

+    }
+
+    privateProperty.set(context, namedConstructorInterface, v8::True(info.GetIsolate()));
+  }
+
+  v8SetReturnValue(
+      info, perContextData->constructorForType(WrapperTypeInfo::unwrap(data)));

[Yuki, 2017/01/19 09:57:28]

|namedConstructorInterface| is what you'd like to return because this is the
getter callback for named constructor.
    v8SetReturnValue(info, namedConstructorInterface);

[sashab, 2017/01/20 04:37:05]

Ahh of course, thank you. Done.

+}
+
 {% 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 %}