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 ffa4afa9179079dbec49bd01ee3957dc7a2e9b0f..d9bcc4b8917b73b1a8958f58bf585d90a008f5e6 100644
--- a/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/interface.cpp.tmpl
@@ -554,6 +554,41 @@ v8::Local<v8::FunctionTemplate> {{v8_class}}Constructor::domTemplate(v8::Isolate
   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 %}