Bug 115584 – Unify ways to cache named item in HTMLCollections

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Ryosuke Niwa]

Right now, HTMLCollection uses TreeScope's DocumentOrderedMap for id-attribute names and manual tree traversal for name-attribute names. In addition HTMLNameCollection has a special hash counted set on HTMLDocument as a fast path. We need to unify all these cases.

[Ryosuke Niwa]

Created attachment 200518 [details]
Work in progress

[Ryosuke Niwa]

Created attachment 200538 [details]
Work in progress 2

[Radar WebKit Bug Importer]

<rdar://problem/13818072>

[Ryosuke Niwa]

Created attachment 200766 [details]
Work in progress 3

[Ryosuke Niwa]

Created attachment 200771 [details]
Cleanup

[Build Bot]

Comment on attachment 200771 [details]
Cleanup

Attachment 200771 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/377274

New failing tests:
css3/filters/custom/filter-fallback-to-software.html

[Build Bot]

Created attachment 200815 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2

[Antti Koivisto]

Comment on attachment 200771 [details]
Cleanup

r=me

[Ryosuke Niwa]

Committed r149652: <http://trac.webkit.org/changeset/149652>

[Darin Adler]

Comment on attachment 200771 [details]
Cleanup

View in context: https://bugs.webkit.org/attachment.cgi?id=200771&action=review

> Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:110
> +    return toJS(exec, thisObj->globalObject(), WTF::getPtr(collection));

It’s fine to use getPtr in automatically generated code where we need to be generic, but in handwritten code we should just call collection.get().

[Antoine Quint]

This changed caused http://webkit.org/b/117836.

[Adam Barth]

When I merged this patch to Blink, I found that it broke the behavior below.  I haven't checked whether WebKit had the same issue.

$ cat LayoutTests/fast/dom/window-polluter-tricky.html 
<script src="../js/resources/js-test-pre.js"></script>
<form name="Taco" id="Taco"></form>
<applet name="Bravo" id="Bravo"></applet>
<script>
shouldBe("window.Taco", "document.getElementById('Taco')");
shouldBe("window.Bravo", "document.getElementById('Bravo')");

shouldBe("document.Taco", "document.getElementById('Taco')");
shouldBe("document.Bravo", "document.getElementById('Bravo')");
</script>
<script src="../js/resources/js-test-post.js"></script>

[Ryosuke Niwa]

(In reply to comment #12)
> When I merged this patch to Blink, I found that it broke the behavior below.  I haven't checked whether WebKit had the same issue.

This patch has a lot of dependencies to my other refactoring patches so maybe you're missing some of that. FWIW, I can't reproduce the bug on ToT WebKit.

[Adam Barth]

> This patch has a lot of dependencies to my other refactoring patches so maybe you're missing some of that.

Yes, merging it wasn't trivial.  :)

> FWIW, I can't reproduce the bug on ToT WebKit.

That's good to hear.  Looks like it was probably fixed in http://trac.webkit.org/changeset/150187.  You might want to land the test in any case so that you don't regress the behavior in the future.