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}} };

[Yuki, 2017/01/26 07:42:26]

I'm very sorry that I was wrong about this.

We should specify |nullptr| (i.e. 0) here as the parent wrapper type info.  I
was misunderstanding that Image.__proto__ should be HTMLElement, but it
shouldn't because Image is not considered as an interface object.

I'm very sorry, but could you revert this point (and the above definition of
parent_wrapper_type_info, and the corresponding layout test)?

[sashab, 2017/02/21 06:37:29]

No prob :) Done.

 #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::Context> creationContext = info.Holder()->CreationContext();
+  V8PerContextData* perContextData = V8PerContextData::from(creationContext);

[Yuki, 2017/01/26 07:08:53]

This is okay that you're correctly using the *creation context*.

+  if (!perContextData) {
+    // TODO(yukishiino): Return a valid named constructor even after the context is detached
+    return;
+  }
+
+  v8::Local<v8::Function> namedConstructor = perContextData->constructorForType(&{{v8_class}}Constructor::wrapperTypeInfo);
+
+  // Set the prototype of named constructors to the regular constructor.
+  auto privateProperty = V8PrivateProperty::getNamedConstructorInitialized(info.GetIsolate());
+  v8::Local<v8::Value> privateValue = privateProperty.get(creationContext, namedConstructor);

[Yuki, 2017/01/26 07:08:53]

It's quite confusing, but *current context* can be different from *creation
context*.  So, would you use *current context* here and below?

v8::Local<v8::Context> currentContext = info.GetIsolate()->GetCurrentContext();

and you should use the |currentContext| here and below.

[haraken, 2017/01/26 09:43:55]

How can |creationContext| be different from |currentContext| here?

If they're different, we'll end up with caching a named constructor of context X
on a private property of a context Y. Which seems wrong to me.
 

[Yuki, 2017/01/26 10:07:11]

Access to |anotherWindow.Image|, then you're in the current context (by
definition), and accessing to |aotherWindow|, which is created in a different
context (this is called |creationContext| in this code because |Image| should be
created in the same context of |anotherWindow|).
The passed context here is NOT used to create a private key, value to be stored
in a private key, or anything else.

Players here are:
- object (or holder) of the private property
- the key (symbol) of the private property
- the value of the private property
i.e. we're performing
    obj[privateSymbol] = value
and there are three things.

|obj| is the named constructor, which is already created in |creationContext|.

|privateSymbol| does NOT depend on any v8::Context because it's common among
contexts, it only depends on v8::Isolate.

|value| is the prototype object of the interface (|interfacePrototype|), which
will be retrieved below (line 575 to 576).  Note that it's extracted from
|perContextData|, it's created in |creationContext|.

Nothing is created in |currentContext|.  |currentContext| is just used to make
V8 do "Get" and "Set" properties.  These operations should be done in
currentContext, and creationContext shouldn't be abused.

I think that haraken@ remembers a discussion that V8 may not need to take a
context in these APIs (except for throwing an exception, but an exception must
always be thrown in the current context).  Anyway, V8 decided to take a context
(even if it's not needed), then we should pass the current context.

[sashab, 2017/02/21 06:37:29]

I don't understand the conversation, but changed to use currentContext. :)

+  if (privateValue.IsEmpty()) {
+    v8::Local<v8::Function> interface = perContextData->constructorForType(&{{v8_class}}::wrapperTypeInfo);
+    v8::Local<v8::Value> interfacePrototype = interface->Get(creationContext, v8AtomicString(info.GetIsolate(), "prototype")).ToLocalChecked();

[Yuki, 2017/01/26 07:08:53]

These interface-Get and namedConstructor-Set should be done on *currentContext*
instead of creationContext.

[sashab, 2017/02/21 06:37:29]

Done.

+    bool result = namedConstructor->Set(creationContext, v8AtomicString(info.GetIsolate(), "prototype"), interfacePrototype).ToChecked();
+    if (!result)
+      return;
+    privateProperty.set(creationContext, namedConstructor, v8::True(info.GetIsolate()));

[Yuki, 2017/01/26 07:08:53]

This also should be currentContext instead of creationContext.

[sashab, 2017/02/21 06:37:29]

Done.

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