[dom] support iterable<> NodeList

[dom] support iterable<> NodeList

Adds missing iterable<Node> support to NodeList. This results in 4
additional enumerable properties on the interface:

- values()
- entries()
- keys()
- forEach()

The WebIDL spec wants each of these methods to return an ArrayIterator,
but for the time being these are still returning IDL Iterator types.

BUG=229398
LOG=N
R=jsbell@chromium.org, philipj@opera.com

[caitp (gmail), 2015-07-01 17:07:30]

PTAL + /cc DOM-y people

[jsbell, 2015-07-01 18:00:30]

Code l*g*t*m but:

* Is HTMLCollection/HTMLAllCollection supposed to be iterable<> ? I'm not seeing
it in the specs!

* Needs tests (preferably testharness.js-based) that actually exercise the new
methods, rather than just asserting their existence.

https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h
File Source/core/dom/NodeList.h (right):

https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h#...
Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
Does this need to become a header include rather than just a forward declare?

https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
File Source/core/html/HTMLAllCollection.idl (right):

https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
Source/core/html/HTMLAllCollection.idl:46: iterable<Element>;
I don't actually see iterable<> on HTMLCollection or HTMLAllCollection in either
DOM or HTML specs. Is it under discussion? (if so, pointers?) Removed?

https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
File Source/core/html/HTMLCollection.idl (right):

https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
Source/core/html/HTMLCollection.idl:33: iterable<Element>;
Ditto.

[jsbell, 2015-07-01 18:01:08]

jsbell@chromium.org changed reviewers:
+ philipj@opera.com
- arv@chromium.org

[jsbell, 2015-07-01 18:01:08]

-arv, +philipj

[caitp (gmail), 2015-07-01 18:07:08]

On 2015/07/01 18:00:30, jsbell wrote:
> Code l*g*t*m but:
> 
> * Is HTMLCollection/HTMLAllCollection supposed to be iterable<> ? I'm not
seeing
> it in the specs!
> 
> * Needs tests (preferably testharness.js-based) that actually exercise the new
> methods, rather than just asserting their existence.
> 
> https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h
> File Source/core/dom/NodeList.h (right):
> 
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h#...
> Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
> Does this need to become a header include rather than just a forward declare?
> 
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> File Source/core/html/HTMLAllCollection.idl (right):
> 
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> Source/core/html/HTMLAllCollection.idl:46: iterable<Element>;
> I don't actually see iterable<> on HTMLCollection or HTMLAllCollection in
either
> DOM or HTML specs. Is it under discussion? (if so, pointers?) Removed?
> 
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> File Source/core/html/HTMLCollection.idl (right):
> 
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> Source/core/html/HTMLCollection.idl:33: iterable<Element>;
> Ditto.

You're right --- IIRC it was added previously, but maybe it was only for
NodeList

[arv (Not doing code reviews), 2015-07-01 18:10:15]

On 2015/07/01 18:07:08, caitp wrote:
> On 2015/07/01 18:00:30, jsbell wrote:
> > Code l*g*t*m but:
> > 
> > * Is HTMLCollection/HTMLAllCollection supposed to be iterable<> ? I'm not
> seeing
> > it in the specs!
> > 
> > * Needs tests (preferably testharness.js-based) that actually exercise the
new
> > methods, rather than just asserting their existence.
> > 
> > https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h
> > File Source/core/dom/NodeList.h (right):
> > 
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h#...
> > Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
> > Does this need to become a header include rather than just a forward
declare?
> > 
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> > File Source/core/html/HTMLAllCollection.idl (right):
> > 
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> > Source/core/html/HTMLAllCollection.idl:46: iterable<Element>;
> > I don't actually see iterable<> on HTMLCollection or HTMLAllCollection in
> either
> > DOM or HTML specs. Is it under discussion? (if so, pointers?) Removed?
> > 
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> > File Source/core/html/HTMLCollection.idl (right):
> > 
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> > Source/core/html/HTMLCollection.idl:33: iterable<Element>;
> > Ditto.
> 
> You're right --- IIRC it was added previously, but maybe it was only for
> NodeList

The problem is that iterable<T> adds "entries", "keys", "values" and @@iterator
but since HTMLCollection and HTMLAllCollection have named getters you run into
clashes of the name. I would hope that we could add @@iterator to these at
least. But that is up to the spec authors to discuss.

[philipj_slow, 2015-07-01 19:06:13]

(s/phillipj/philipj/ in the description, that's why I didn't see this initially.
Cursed be names with multiple spellings!)

[philipj_slow, 2015-07-01 19:13:45]

On 2015/07/01 18:10:15, arv wrote:
> On 2015/07/01 18:07:08, caitp wrote:
> > On 2015/07/01 18:00:30, jsbell wrote:
> > > Code l*g*t*m but:
> > > 
> > > * Is HTMLCollection/HTMLAllCollection supposed to be iterable<> ? I'm not
> > seeing
> > > it in the specs!
> > > 
> > > * Needs tests (preferably testharness.js-based) that actually exercise the
> new
> > > methods, rather than just asserting their existence.
> > > 
> > >
https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h
> > > File Source/core/dom/NodeList.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h#...
> > > Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
> > > Does this need to become a header include rather than just a forward
> declare?
> > > 
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> > > File Source/core/html/HTMLAllCollection.idl (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> > > Source/core/html/HTMLAllCollection.idl:46: iterable<Element>;
> > > I don't actually see iterable<> on HTMLCollection or HTMLAllCollection in
> > either
> > > DOM or HTML specs. Is it under discussion? (if so, pointers?) Removed?
> > > 
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> > > File Source/core/html/HTMLCollection.idl (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> > > Source/core/html/HTMLCollection.idl:33: iterable<Element>;
> > > Ditto.
> > 
> > You're right --- IIRC it was added previously, but maybe it was only for
> > NodeList
> 
> The problem is that iterable<T> adds "entries", "keys", "values" and
@@iterator
> but since HTMLCollection and HTMLAllCollection have named getters you run into
> clashes of the name. I would hope that we could add @@iterator to these at
> least. But that is up to the spec authors to discuss.

If this is the only reason that HTMLCollection cannot be iterable, we could add
a use counter to see if the named getter is called with any of the new names in
more than ~0% of cases. But perhaps file a bug at
https://github.com/whatwg/dom/issues/new if you want to pursue this to make sure
there's nothing else blocking it.

For now, I say do this for NodeList only. Have you checked what the support in
other engines looks like? If we are the first, it might be worth making a little
bit of noise about, since there must have been a gazillion suggestions to make
NodeList more array-like, and this might close the door on some of them. (I'm
thinking of [ArrayClass] but am not sure it actually applies.)

(I haven't looked at the code in this CL yet.)

[caitp (gmail), 2015-07-01 19:19:51]

On 2015/07/01 18:00:30, jsbell wrote:
> Code l*g*t*m but:
> 
> * Is HTMLCollection/HTMLAllCollection supposed to be iterable<> ? I'm not
seeing
> it in the specs!
> 
> * Needs tests (preferably testharness.js-based) that actually exercise the new
> methods, rather than just asserting their existence.

Added some behavioural tests --- but there's a catch 22, run-webkit-tests
reports a failure if the -expected.txt file is missing, and presubmit fails if
it's not missing. That is very confusing, and looking at other CLs which add
testharness.js tests, I'm not seeing how to work around that

[jsbell, 2015-07-01 19:22:03]

On 2015/07/01 19:19:51, caitp wrote:
> Added some behavioural tests --- but there's a catch 22, run-webkit-tests
> reports a failure if the -expected.txt file is missing, and presubmit fails if
> it's not missing. That is very confusing, and looking at other CLs which add
> testharness.js tests, I'm not seeing how to work around that

I'll give this a try locally.

[caitp (gmail), 2015-07-01 19:32:04]

On 2015/07/01 19:13:45, philipj wrote:
> On 2015/07/01 18:10:15, arv wrote:
> > On 2015/07/01 18:07:08, caitp wrote:
> > > On 2015/07/01 18:00:30, jsbell wrote:
> > > > Code l*g*t*m but:
> > > > 
> > > > * Is HTMLCollection/HTMLAllCollection supposed to be iterable<> ? I'm
not
> > > seeing
> > > > it in the specs!
> > > > 
> > > > * Needs tests (preferably testharness.js-based) that actually exercise
the
> > new
> > > > methods, rather than just asserting their existence.
> > > > 
> > > >
> https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h
> > > > File Source/core/dom/NodeList.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/dom/NodeList.h#...
> > > > Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
> > > > Does this need to become a header include rather than just a forward
> > declare?
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> > > > File Source/core/html/HTMLAllCollection.idl (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLAllCol...
> > > > Source/core/html/HTMLAllCollection.idl:46: iterable<Element>;
> > > > I don't actually see iterable<> on HTMLCollection or HTMLAllCollection
in
> > > either
> > > > DOM or HTML specs. Is it under discussion? (if so, pointers?) Removed?
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> > > > File Source/core/html/HTMLCollection.idl (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1220883007/diff/1/Source/core/html/HTMLCollec...
> > > > Source/core/html/HTMLCollection.idl:33: iterable<Element>;
> > > > Ditto.
> > > 
> > > You're right --- IIRC it was added previously, but maybe it was only for
> > > NodeList
> > 
> > The problem is that iterable<T> adds "entries", "keys", "values" and
> @@iterator
> > but since HTMLCollection and HTMLAllCollection have named getters you run
into
> > clashes of the name. I would hope that we could add @@iterator to these at
> > least. But that is up to the spec authors to discuss.
> 
> If this is the only reason that HTMLCollection cannot be iterable, we could
add
> a use counter to see if the named getter is called with any of the new names
in
> more than ~0% of cases. But perhaps file a bug at
> https://github.com/whatwg/dom/issues/new if you want to pursue this to make
sure
> there's nothing else blocking it.
> 
> For now, I say do this for NodeList only. Have you checked what the support in
> other engines looks like? If we are the first, it might be worth making a
little
> bit of noise about, since there must have been a gazillion suggestions to make
> NodeList more array-like, and this might close the door on some of them. (I'm
> thinking of [ArrayClass] but am not sure it actually applies.)
> 
> (I haven't looked at the code in this CL yet.)

Re: browser support, Gecko partially supports the iterable<Node> declaration
(@@iterator is there, but not forEach/keys/values/entries)

[jsbell, 2015-07-01 19:34:40]

On 2015/07/01 19:22:03, jsbell wrote:
> On 2015/07/01 19:19:51, caitp wrote:
> > Added some behavioural tests --- but there's a catch 22, run-webkit-tests
> > reports a failure if the -expected.txt file is missing, and presubmit fails
if
> > it's not missing. That is very confusing, and looking at other CLs which add
> > testharness.js tests, I'm not seeing how to work around that
> 
> I'll give this a try locally.

Works fine for me locally without -expected.txt (as expected). Just a branch,
apply this patch, run: run-webkit-tests --debug
fast/dom/NodeList/nodelist-iterable.html

[jsbell, 2015-07-01 19:41:19]

Since this is a live NodeList, a test case or two where the list is mutated
during iteration would be nice, e.g. one where the list is mutated "behind" the
current position and one where the list is mutated "ahead" of the current
position.

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
File LayoutTests/fast/dom/NodeList/nodelist-iterable.html (right):

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:3: <script
src="../../../resources/testharness.js"></script>
Nit: Add a <title> explaining the purpose of the test.

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:6: <div id='div1'></div>
nit: Be consistent in quoting (i.e. use "" here if used above)

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:19: assert_true(node
instanceof HTMLDivElement, "elements should be expected types");
Ditto (i.e. use ' if used above, etc).

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:64: if (window.testRunner)
Is this necessary?

[caitp (gmail), 2015-07-01 20:22:35]

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
File LayoutTests/fast/dom/NodeList/nodelist-iterable.html (right):

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:3: <script
src="../../../resources/testharness.js"></script>
On 2015/07/01 19:41:19, jsbell wrote:
> Nit: Add a <title> explaining the purpose of the test.

Done.

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:6: <div id='div1'></div>
On 2015/07/01 19:41:19, jsbell wrote:
> nit: Be consistent in quoting (i.e. use "" here if used above)

Done.

https://codereview.chromium.org/1220883007/diff/20001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:64: if (window.testRunner)
On 2015/07/01 19:41:19, jsbell wrote:
> Is this necessary?

Without it, the DOM elements in the test get added to the output, which breaks
when diffing against -expected.txt (which admittedly shouldn't be present at all
according to LayoutTests/PRESUBMIT.py)

[arv (Not doing code reviews), 2015-07-01 20:53:08]

I like

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/No...
File LayoutTests/fast/dom/NodeList/nodelist-iterable.html (right):

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/No...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:107:
container.style.display = "none";
You could put your html into a <template> element instead. You would have to
adjust your code slightly to take that into account.

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
File LayoutTests/fast/dom/script-tests/domListEnumeration.js (right):

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
LayoutTests/fast/dom/script-tests/domListEnumeration.js:100:
shouldBe("resultArray.length", "9");
Why did this change?

[caitp (gmail), 2015-07-01 20:57:36]

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
File LayoutTests/fast/dom/script-tests/domListEnumeration.js (right):

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
LayoutTests/fast/dom/script-tests/domListEnumeration.js:100:
shouldBe("resultArray.length", "9");
On 2015/07/01 20:53:08, arv wrote:
> Why did this change?

4 new enumerable properties (forEach, keys, values, entries)

[arv (Not doing code reviews), 2015-07-01 21:01:29]

So sad that WebIDL methods are enumerable.

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
File LayoutTests/fast/dom/script-tests/domListEnumeration.js (right):

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
LayoutTests/fast/dom/script-tests/domListEnumeration.js:100:
shouldBe("resultArray.length", "9");
On 2015/07/01 20:57:36, caitp wrote:
> On 2015/07/01 20:53:08, arv wrote:
> > Why did this change?
> 
> 4 new enumerable properties (forEach, keys, values, entries)

Wow. This test sucks. Why does iterateList use a for in loop?

[caitp (gmail), 2015-07-01 21:09:36]

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
File LayoutTests/fast/dom/script-tests/domListEnumeration.js (right):

https://codereview.chromium.org/1220883007/diff/60001/LayoutTests/fast/dom/sc...
LayoutTests/fast/dom/script-tests/domListEnumeration.js:100:
shouldBe("resultArray.length", "9");
On 2015/07/01 21:01:29, arv wrote:
> On 2015/07/01 20:57:36, caitp wrote:
> > On 2015/07/01 20:53:08, arv wrote:
> > > Why did this change?
> > 
> > 4 new enumerable properties (forEach, keys, values, entries)
> 
> Wow. This test sucks. Why does iterateList use a for in loop?
> 
> 
> 

Enumerability + not just own properties is my guess

[philipj_slow, 2015-07-01 21:25:45]

On 2015/07/01 21:01:29, arv wrote:
> So sad that WebIDL methods are enumerable.

Do you mean in general or just for the attributes added by iterable<>?

[arv (Not doing code reviews), 2015-07-01 21:26:54]

On 2015/07/01 21:25:45, philipj wrote:
> On 2015/07/01 21:01:29, arv wrote:
> > So sad that WebIDL methods are enumerable.
> 
> Do you mean in general or just for the attributes added by iterable<>?

In general. It is sad that the DOM does not follow ES on this.

[philipj_slow, 2015-07-01 21:46:22]

On 2015/07/01 21:26:54, arv wrote:
> On 2015/07/01 21:25:45, philipj wrote:
> > On 2015/07/01 21:01:29, arv wrote:
> > > So sad that WebIDL methods are enumerable.
> > 
> > Do you mean in general or just for the attributes added by iterable<>?
> 
> In general. It is sad that the DOM does not follow ES on this.

Now off-topic for this CL, but have you looked into what kind of havoc would
result from flipping the defaults so that everything is non-enumerable by
default, and there's an [Enumerable] instead of [NotEnumerable] attribute? Seems
rather radical, but I don't know to what extent content depends on enumeration.

[caitp (gmail), 2015-07-02 21:37:19]

Patchset #6 (id:100001) has been deleted

[jsbell, 2015-09-09 15:56:48]

What happened to this CL? Did we all just get distracted by other shiny things
or was there discussion elsewhere?

[caitp (gmail), 2015-09-09 15:58:20]

On 2015/09/09 15:56:48, jsbell wrote:
> What happened to this CL? Did we all just get distracted by other shiny things
> or was there discussion elsewhere?

I guess reviewers have been busy. I can always rebase and ask for another look,
but it takes a long time to do that over here =)

[caitp (gmail), 2015-09-18 20:18:55]

Patchset #7 (id:140001) has been deleted

[caitp (gmail), 2015-09-18 20:20:29]

On 2015/09/09 15:58:20, caitp wrote:
> On 2015/09/09 15:56:48, jsbell wrote:
> > What happened to this CL? Did we all just get distracted by other shiny
things
> > or was there discussion elsewhere?
> 
> I guess reviewers have been busy. I can always rebase and ask for another
look,
> but it takes a long time to do that over here =)

I've just made sure the patch still merges cleanly. If someone is willing to
dcommit it that's cool, or else I suppose it can wait until after the merge.

[jsbell, 2015-09-21 17:55:18]

non-owner lgtm (note previous question from comment #2 repeated, though)

https://codereview.chromium.org/1220883007/diff/160001/LayoutTests/fast/dom/N...
File LayoutTests/fast/dom/NodeList/nodelist-iterable.html (right):

https://codereview.chromium.org/1220883007/diff/160001/LayoutTests/fast/dom/N...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:35: let id = entry[0] + 1;
I'd expect an assertion about entry[0] here.

https://codereview.chromium.org/1220883007/diff/160001/LayoutTests/fast/dom/N...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:46: let node =
nodeList[id++];
I'd expect an assert about id here.

https://codereview.chromium.org/1220883007/diff/160001/Source/core/dom/NodeLi...
File Source/core/dom/NodeList.h (right):

https://codereview.chromium.org/1220883007/diff/160001/Source/core/dom/NodeLi...
Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
Is this actually needed, or can we keep the forward declaration?

[caitp (gmail), 2015-09-22 01:57:33]

https://codereview.chromium.org/1220883007/diff/160001/LayoutTests/fast/dom/N...
File LayoutTests/fast/dom/NodeList/nodelist-iterable.html (right):

https://codereview.chromium.org/1220883007/diff/160001/LayoutTests/fast/dom/N...
LayoutTests/fast/dom/NodeList/nodelist-iterable.html:35: let id = entry[0] + 1;
On 2015/09/21 17:55:18, jsbell wrote:
> I'd expect an assertion about entry[0] here.

`typeof id === 'number'`? It's messier to assert the actual value of id, and its
value is sort of tested below with the "div" + id assertion. I'll just add a
test for the number type?

https://codereview.chromium.org/1220883007/diff/160001/Source/core/dom/NodeLi...
File Source/core/dom/NodeList.h (right):

https://codereview.chromium.org/1220883007/diff/160001/Source/core/dom/NodeLi...
Source/core/dom/NodeList.h:30: #include "core/dom/Node.h"
On 2015/09/21 17:55:18, jsbell wrote:
> Is this actually needed, or can we keep the forward declaration?

My compiler didn't like the uses of the template parameter for reference-type
parameters without including the header, so I believe it is needed :(

I'm overseas at the moment and won't be able to build the tree very quickly to
test it, so maybe someone else would be willing to do that.

[philipj_slow, 2015-09-22 09:03:44]

I started a bot to see if this compiles and if some tests need fixing.

This is a pretty small feature, but I think an Intent to Implement and Ship on
blink-dev would still be a good idea, as people have wanted NodeList to be less
bad for a very long time and there have been other attemps like what is now
[LegacyArrayClass], so a few more eyes on this would be good.

[caitp (gmail), 2015-09-22 10:15:50]

On 2015/09/22 09:03:44, philipj wrote:
> I started a bot to see if this compiles and if some tests need fixing.
> 
> This is a pretty small feature, but I think an Intent to Implement and Ship on
> blink-dev would still be a good idea, as people have wanted NodeList to be
less
> bad for a very long time and there have been other attemps like what is now
> [LegacyArrayClass], so a few more eyes on this would be good.

I'll send an intent thread this week, but it looks like there are some rest
expectations that need updates as well.

[caitp (gmail), 2015-09-22 18:05:31]

Fixed expectations that caused failure in the try run.

Since there are separate expectations for android, I expect those to fail still,
but I'm not sure about that one.

I'm also not sure if the most recent patch will build, because the recent
dcommited pushes to the blink repo are preventing me from building on top of
lkgr. It probably won't be tested properly until after the 23rd

[caitp (gmail), 2015-09-23 11:04:38]

On 2015/09/22 18:05:31, caitp wrote:
> Fixed expectations that caused failure in the try run.
> 
> Since there are separate expectations for android, I expect those to fail
still,
> but I'm not sure about that one.
> 
> I'm also not sure if the most recent patch will build, because the recent
> dcommited pushes to the blink repo are preventing me from building on top of
> lkgr. It probably won't be tested properly until after the 23rd

It builds just fine, so that's good.