Make pair iterators inherit from %IteratorPrototype%.

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

Index: third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
diff --git a/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl b/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
index c4826362fc0bf1e58c88e5ebcfcf6dc89a1bc876..4ebad8415559b43ee94a55e6c31170b253075372 100644
--- a/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/interface_base.cpp.tmpl
@@ -532,6 +532,29 @@ static void install{{v8_class}}Template(v8::Isolate* isolate, const DOMWrapperWo
   {% 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