Make pair iterators inherit from %IteratorPrototype%.

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

 {% filter format_blink_cpp_source_code %}
 
 {% include 'copyright_block.txt' %}
 #include "{{v8_class_or_partial}}.h"
 
 {% for filename in cpp_includes if filename != '%s.h' % cpp_class_or_partial %}
 #include "{{filename}}"
 {% endfor %}
 
 namespace blink {
@@ skipping 514 matching lines @@
   {% if iterator_method %}
   {% filter exposed(iterator_method.exposed_test) %}
   {% filter runtime_enabled(iterator_method.runtime_enabled_feature_name) %}
   // Iterator (@@iterator)
   static const V8DOMConfiguration::SymbolKeyedMethodConfiguration symbolKeyedIteratorConfiguration = { v8::Symbol::GetIterator, {{v8_class_or_partial}}::iteratorMethodCallback, 0, v8::DontEnum, V8DOMConfiguration::OnPrototype, V8DOMConfiguration::CheckHolder, V8DOMConfiguration::DoNotCheckAccess };
   V8DOMConfiguration::installMethod(isolate, world, prototypeTemplate, signature, symbolKeyedIteratorConfiguration);
   {% endfilter %}
   {% endfilter %}
   {% endif %}
 
+  {% if interface_name == 'Iterator' %}
+  // The WebIDL spec says when an interface has pair iterators the iterators it returns must be

[Yuki, 2017/04/07 09:25:03]

nit: Could you wrap lines within 80 columns?

[Raphael Kubo da Costa (rakuco), 2017/04/07 09:45:45]

Done.

+  // instances of the "default iterator object" whose [[Prototype]] points to an "iterator prototype
+  // object" whose [[Prototype]], on its turn, points to %IteratorPrototype%. next() must be
+  // implemented in the "iterator prototype object", while %IteratorPrototype% provides @@iterator.
+  // References: https://heycam.github.io/webidl/#es-default-iterator-object
+  //             https://heycam.github.io/webidl/#es-iterator-prototype-object
+  //
+  // The iterators we return from interfaces that have pair interators adhere to the above by:
+  // - Adding the "next()" property to its prototype object.
+  // - Making the prototype object inherit from %IteratorPrototype% with the hack below, which
+  //   creates another function template with no prototype and sets the "prototype" property of its
+  //   function object.
+  //   When |interfaceTemplate|'s function object is created, its prototype.__proto__ will point to
+  //   the value of the "prototype" property of |iteratorPrototypeTemplate|, which is exactly what
+  //   we want.
+  v8::Local<v8::FunctionTemplate> iteratorPrototypeTemplate = v8::FunctionTemplate::New(isolate);

[Yuki, 2017/04/07 09:25:03]

nit: The name is a bit confusing.

Let me suppose
  F = iteratorPrototypeTemplate
then, I'd say that F.PrototypeTemplate() is supposed to be the intrinsic
iterator prototype *template* (but actually not, so we remove it) and
F.Get("prototype") is the intrinsic iterator prototype.  F itself is a (dummy)
interface template, and not a prototype object nor template.

What about intrinsicIteratorInterfaceTemplate for example?
(intrinsicIteratorPrototypesDummyInterfaceTemplate is too long, I think).  I'm
open to other options.

[Yuki, 2017/04/07 09:25:03]

nit:

This is okay, but let me elaborate something.

AFAIK, V8 GC does not collect FunctionTemplates, so they can cause a leak.  It's
Blink's responsibility not to create FunctionTemplates unnecessarily too much.

At a glance, this code looks like creating a new FunctionTemplate on every call
(so this looks no good), however, actually this is okay because:
1) |interfaceTemplate| is cached in V8PerIsolateData, so we create only one
FunctionTemplate per interface, and
2) there is only one interface "Iterator" that creates this FunctionTemplate, so
no need to reuse this FunctionTemplate.

If we had two or more interfaces that use this FunctionTemplate, we should
register this FunctionTemplate in V8PerIsolateData (c.f.
V8PerIsolateData::{find,set}InterfaceTemplate), and reuse it.

So, probably it's better to comment that we don't need to cache this
FunctionTemplate in V8PerIsolateData because only "Iterator" needs it.

[Raphael Kubo da Costa (rakuco), 2017/04/07 09:45:45]

How about intrinsicIteratorPrototypeInterfaceTemplate? It's quite long but still
fits 80 columns :-)

[Raphael Kubo da Costa (rakuco), 2017/04/07 09:45:45]

Thanks for the explanation! I'm still wrapping my head around all the
isolate+context combinations in the bindings (and the multiple Script* classes),
so an explanation like that is very useful :-)

I'm adding a comment here that should hopefully clarify this to other readers.

+  iteratorPrototypeTemplate->RemovePrototype();
+  iteratorPrototypeTemplate->SetIntrinsicDataProperty(v8AtomicString(isolate, "prototype"),
+                                                      v8::kIteratorPrototype);
+  interfaceTemplate->Inherit(iteratorPrototypeTemplate);
+  {% endif %}
+
   {% if interface_name == 'Location' %}
   // Symbol.toPrimitive
   // Prevent author scripts to inject Symbol.toPrimitive property into location
   // objects, also prevent the look-up of Symbol.toPrimitive through the
   // prototype chain.
   instanceTemplate->Set(v8::Symbol::GetToPrimitive(isolate),
                         v8::Undefined(isolate),
                         static_cast<v8::PropertyAttribute>(
                             v8::ReadOnly | v8::DontEnum | v8::DontDelete));
   {% endif %}
@@ skipping 210 matching lines @@
 {% endif %}
 }
 {% endif %}
 
 {% endblock %}
 {##############################################################################}
 {% block partial_interface %}{% endblock %}
 }  // namespace blink
 
 {% endfilter %}{# format_blink_cpp_source_code #}