I don't think we need registerDynamicSubtreeNodeList and unregisterDynamicSubtreeNodeList. In particular, the following comment seems no longer valid because TreeScopeAdopter::moveTreeToNewScope clears node list caches whenever nodes are adopted into a new tree scope. // We haven't been receiving notifications while there were no registered lists, so the cache is invalid now. if (data->nodeLists() && (!treeScope() || !treeScope()->hasNodeListCaches())) data->nodeLists()->invalidateCaches();
Created attachment 131427 [details] cleanup
Comment on attachment 131427 [details] cleanup r=me.
Comment on attachment 131427 [details] cleanup Attachment 131427 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11941228 New failing tests: fast/forms/label/labels-remove-parent-label.html fast/forms/label/labels-set-htmlFor-attribute.html fast/forms/label/labels-remove-htmlFor-attribute.html fast/regions/get-regions-by-content-node-vert-rl.html fast/regions/get-regions-by-content-node-horiz-bt.html fast/forms/label/labels-add-htmlFor-label.html fast/regions/get-regions-by-content-node-vert-lr.html fast/regions/get-regions-by-content-node-horiz-tb.html fast/forms/label/labels-change-htmlFor-attribute.html fast/forms/label/labels-add-parent-label.html fast/forms/label/labels-remove-htmlFor-label.html fast/regions/get-regions-by-content-node2.html
Comment on attachment 131427 [details] cleanup Ugh... this isn't quite right because of LabelsNodeList and RegionNodeList. They need to be notified of changes at the document level instead of at the node it belongs :(
Changing the scope of the bug a little bit.
Created attachment 131501 [details] Patch
Comment on attachment 131501 [details] Patch Attachment 131501 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11942360 New failing tests: fast/regions/get-regions-by-content-node-vert-lr.html fast/regions/get-regions-by-content-node-horiz-bt.html fast/regions/get-regions-by-content-node-vert-rl.html fast/regions/get-regions-by-content-node-horiz-tb.html fast/regions/get-regions-by-content-node.html fast/regions/get-regions-by-content-node2.html
Sigh... the bug 80638 is haunting me :(
Created attachment 131560 [details] Also fix the bug 80638
Comment on attachment 131560 [details] Also fix the bug 80638 Attachment 131560 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11942407
Comment on attachment 131560 [details] Also fix the bug 80638 Attachment 131560 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11945371 New failing tests: fast/regions/get-regions-by-content-node-vert-lr.html fast/regions/get-regions-by-content-node-horiz-bt.html fast/regions/get-regions-by-content-node-vert-rl.html fast/regions/get-regions-by-content-node-horiz-tb.html fast/regions/get-regions-by-content-node.html fast/regions/get-regions-by-content-node2.html
ugh.... regions node lists were passing tests due to "redundant" cache invalidation magically updating otherwise stale node lists :-(
Created attachment 131777 [details] Updated for ToT
Comment on attachment 131777 [details] Updated for ToT Attachment 131777 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/11948868
Committed in http://trac.webkit.org/changeset/110797.
Apparently I mangled patches together when I was landing a patch for https://bugs.webkit.org/show_bug.cgi?id=80706.
Comment on attachment 131777 [details] Updated for ToT View in context: https://bugs.webkit.org/attachment.cgi?id=131777&action=review r=me but please remove references to region node lists from the code/changelog. Bonus points for removing a parseAttribute() overload. :) > Source/WebCore/dom/DynamicNodeList.h:90 > + DynamicNodeList::invalidateCache; Needs "using" to build on win, but you know that.
The patch was "preemptively" landed in http://trac.webkit.org/changeset/110797 by accident. Fixed the reviewer name in http://trac.webkit.org/changeset/110817.