Assert the connectedSubframeCount is consistent and fix over counting
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Jan 2013 10:48:14 +0000 (10:48 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Jan 2013 10:48:14 +0000 (10:48 +0000)
commitf08ed51a8c9dd1003e712321f7116af4ab082f38
tree7bf36e1a10dfbe1eb7c4000c72dd3a7dd9a90c67
parent4235075584c278fae2306b2e739c0b5ec06fef04
Assert the connectedSubframeCount is consistent and fix over counting
https://bugs.webkit.org/show_bug.cgi?id=107302

Patch by Elliott Sprehn <esprehn@gmail.com> on 2013-01-25
Reviewed by Alexey Proskuryakov.

Source/WebCore:

Add a debug assertion that walks the subtree during frame disconnection
and manually counts the number of connected subframes to assert that the
value from Node::connectedSubframeCount() is the same as if we traversed
through the tree.

In fixing the places where this assertion failed I made document destruction
faster by not walking the entire document looking for frames if the entire
frame tree has been destroyed by way of FrameLoader::detachChildren().
I had inadvertently introduced this improvement in r133933, but then I
regressed it in r140090 when we switched to counting because I didn't
realize we destroy the frame tree separate of frame disconnection on
document unload so all frames could have been destroyed but the counts
left on the ancestors.

I also fixed another overcounting case where the adoption agency algorithm
may call ContainerNode::takeAllChildrenFrom() which in turn calls
ContainerNode::removeAllChildren() and could have left a connected subframe
count on the node even though all the frames had been removed.

This assertion did not uncover any cases of undercounting the number of
frames.

This also fixes a rare edge case where removeChild of an iframe that
was already being unloaded would not unload the frame until the top level
unload was done, and a reparenting of the iframe would not cause it to load.

Test: fast/frames/reparent-in-unload-contentdocument.html

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::removeAllChildren):
(WebCore::ContainerNode::parserInsertBefore):
(WebCore::ContainerNode::parserRemoveChild):
(WebCore::ContainerNode::parserAppendChild):
* dom/ContainerNodeAlgorithms.cpp:
(WebCore):
(WebCore::assertConnectedSubframeCountIsConsistent):
* dom/ContainerNodeAlgorithms.h:
(WebCore):
(WebCore::ChildFrameDisconnector::disconnect):
* dom/Node.cpp:
(WebCore::Node::updateAncestorConnectedSubframeCountForRemoval):
(WebCore):
(WebCore::Node::updateAncestorConnectedSubframeCountForInsertion):
* dom/Node.h:
(Node):
* html/HTMLFrameOwnerElement.cpp:
(WebCore::HTMLFrameOwnerElement::clearContentFrame):
(WebCore):
(WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
* html/HTMLFrameOwnerElement.h:
(HTMLFrameOwnerElement):

LayoutTests:

Add a test that removing an iframe in the middle of unload causes the
contentDocument to become immediately inaccessible.

* fast/frames/reparent-in-unload-contentdocument-expected.txt: Added.
* fast/frames/reparent-in-unload-contentdocument.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@140807 268f45cc-cd09-0410-ab3c-d52691b4dbfc
LayoutTests/ChangeLog
LayoutTests/fast/frames/reparent-in-unload-contentdocument-expected.txt [new file with mode: 0644]
LayoutTests/fast/frames/reparent-in-unload-contentdocument.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/ContainerNode.cpp
Source/WebCore/dom/ContainerNodeAlgorithms.cpp
Source/WebCore/dom/ContainerNodeAlgorithms.h
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/Node.h
Source/WebCore/html/HTMLFrameOwnerElement.cpp
Source/WebCore/html/HTMLFrameOwnerElement.h