Bug 129727 – appendChild shouldn't invalidate LiveNodeLists and HTMLCollections if they don't have valid caches

• Component: DOM
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Right now, invalidateNodeListAndCollectionCachesInAncestors invalidates node lists and HTML collections
on all ancestors whenever we're inserting or removing a node.

We don't have to do this when there are no node lists and HTML collections with valid caches.

[Ryosuke Niwa]

Created attachment 225856 [details]
Fixes the bug

[Ryosuke Niwa]

A/B test result: http://dromaeo.com/?id=218031,218032
Looks like a 1% progression.

[Build Bot]

Comment on attachment 225856 [details]
Fixes the bug

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

New failing tests:
dom/html/level2/html/HTMLIsIndexElement02.html

[Build Bot]

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

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

[Build Bot]

Comment on attachment 225856 [details]
Fixes the bug

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

New failing tests:
dom/html/level2/html/HTMLIsIndexElement02.html

[Build Bot]

Created attachment 225864 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

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

[Build Bot]

Comment on attachment 225856 [details]
Fixes the bug

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

New failing tests:
dom/html/level2/html/HTMLIsIndexElement02.html

[Build Bot]

Created attachment 225867 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

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

[Ryosuke Niwa]

Comment on attachment 225856 [details]
Fixes the bug

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

> Source/WebCore/dom/CollectionIndexCache.h:199
> -    if (index && m_currentNode) {
> +    if (index) {

Oops, this erroneous change caused a crash in dom/html/level2/html/HTMLIsIndexElement02.html

[Ryosuke Niwa]

Created attachment 225868 [details]
Reverted the erronous change

[Andreas Kling]

Comment on attachment 225868 [details]
Reverted the erronous change

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

Good idea. r=me

> Source/WebCore/dom/Node.cpp:728
> +    HashSet<LiveNodeList*> liveNodeLists;
> +    liveNodeLists.swap(m_listsInvalidatedAtDocument);

I'd write this with move semantics:
HashSet<LiveNodeList*> liveNodeLists = std::move(m_listsInvalidatedAtDocument);

> Source/WebCore/dom/Node.cpp:733
> +    HashSet<HTMLCollection*> collectionLists;
> +    collectionLists.swap(m_collectionsInvalidatedAtDocument);

Same here.

[Ryosuke Niwa]

Created attachment 225871 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 225871 [details]
Patch for landing

Clearing flags on attachment: 225871

Committed r165103: <http://trac.webkit.org/changeset/165103>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.