- Revision
- 134817
- Author
- [email protected]
- Date
- 2012-11-15 12:38:34 -0800 (Thu, 15 Nov 2012)
Log Message
Track subframe count to avoid traversing the tree when there's no subframes
https://bugs.webkit.org/show_bug.cgi?id=101821
Patch by Elliott Sprehn <[email protected]> on 2012-11-15
Reviewed by Ojan Vafai.
Bug 101619 showed a 9-14% improvement from not walking the children during
removeChild looking for frames when there's known to be no frames. The fix
in that bug only avoids this walk when the whole document has no frames, this
patch extends it to skip traversing subtrees that have no iframes by hooking
the frame assignment to walk up the tree and keep track of the count of frames
in the subtree on contentFrame assignment and then decrement it on disconnect.
No new tests, this is just a perf refactor.
* dom/ContainerNode.cpp:
(WebCore::willRemoveChildren):
* dom/ContainerNodeAlgorithms.cpp:
(WebCore::ChildFrameDisconnector::collectFrameOwners):
* dom/ContainerNodeAlgorithms.h:
(WebCore::ChildFrameDisconnector::ChildFrameDisconnector):
(ChildFrameDisconnector):
(WebCore::ChildFrameDisconnector::collectFrameOwners):
Renamed from collectDescendant() to better reflect what it really does.
(WebCore::ChildFrameDisconnector::disconnectCollectedFrameOwners):
Renamed from disconnect() to better reflect what it really does.
(WebCore::ChildFrameDisconnector::disconnect):
New method that does the collection of frame owners on either the root
or only it's descendants.
* dom/Node.cpp:
(WebCore::Node::connectedSubframeCount):
(WebCore::Node::incrementConnectedSubframeCount):
(WebCore::Node::decrementConnectedSubframeCount):
* dom/Node.h:
* dom/NodeRareData.h:
(WebCore::NodeRareData::NodeRareData):
(WebCore::NodeRareData::connectedSubframeCount):
(WebCore::NodeRareData::incrementConnectedSubframeCount):
(WebCore::NodeRareData::decrementConnectedSubframeCount):
* html/HTMLFrameOwnerElement.cpp:
(WebCore::HTMLFrameOwnerElement::setContentFrame):
(WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (134816 => 134817)
--- trunk/Source/WebCore/ChangeLog 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/ChangeLog 2012-11-15 20:38:34 UTC (rev 134817)
@@ -1,3 +1,47 @@
+2012-11-15 Elliott Sprehn <[email protected]>
+
+ Track subframe count to avoid traversing the tree when there's no subframes
+ https://bugs.webkit.org/show_bug.cgi?id=101821
+
+ Reviewed by Ojan Vafai.
+
+ Bug 101619 showed a 9-14% improvement from not walking the children during
+ removeChild looking for frames when there's known to be no frames. The fix
+ in that bug only avoids this walk when the whole document has no frames, this
+ patch extends it to skip traversing subtrees that have no iframes by hooking
+ the frame assignment to walk up the tree and keep track of the count of frames
+ in the subtree on contentFrame assignment and then decrement it on disconnect.
+
+ No new tests, this is just a perf refactor.
+
+ * dom/ContainerNode.cpp:
+ (WebCore::willRemoveChildren):
+ * dom/ContainerNodeAlgorithms.cpp:
+ (WebCore::ChildFrameDisconnector::collectFrameOwners):
+ * dom/ContainerNodeAlgorithms.h:
+ (WebCore::ChildFrameDisconnector::ChildFrameDisconnector):
+ (ChildFrameDisconnector):
+ (WebCore::ChildFrameDisconnector::collectFrameOwners):
+ Renamed from collectDescendant() to better reflect what it really does.
+ (WebCore::ChildFrameDisconnector::disconnectCollectedFrameOwners):
+ Renamed from disconnect() to better reflect what it really does.
+ (WebCore::ChildFrameDisconnector::disconnect):
+ New method that does the collection of frame owners on either the root
+ or only it's descendants.
+ * dom/Node.cpp:
+ (WebCore::Node::connectedSubframeCount):
+ (WebCore::Node::incrementConnectedSubframeCount):
+ (WebCore::Node::decrementConnectedSubframeCount):
+ * dom/Node.h:
+ * dom/NodeRareData.h:
+ (WebCore::NodeRareData::NodeRareData):
+ (WebCore::NodeRareData::connectedSubframeCount):
+ (WebCore::NodeRareData::incrementConnectedSubframeCount):
+ (WebCore::NodeRareData::decrementConnectedSubframeCount):
+ * html/HTMLFrameOwnerElement.cpp:
+ (WebCore::HTMLFrameOwnerElement::setContentFrame):
+ (WebCore::HTMLFrameOwnerElement::disconnectContentFrame):
+
2012-11-15 Alpha Lam <[email protected]>
[chromium] WebGL texImage2D fails with deferred image decoding
Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (134816 => 134817)
--- trunk/Source/WebCore/dom/ContainerNode.cpp 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2012-11-15 20:38:34 UTC (rev 134817)
@@ -373,7 +373,7 @@
dispatchChildRemovalEvents(child);
}
- ChildFrameDisconnector(container, ChildFrameDisconnector::DoNotIncludeRoot).disconnect();
+ ChildFrameDisconnector(container).disconnect(ChildFrameDisconnector::DescendantsOnly);
}
void ContainerNode::disconnectDescendantFrames()
Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (134816 => 134817)
--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2012-11-15 20:38:34 UTC (rev 134817)
@@ -109,10 +109,10 @@
}
}
-void ChildFrameDisconnector::collectDescendant(ElementShadow* shadow)
+void ChildFrameDisconnector::collectFrameOwners(ElementShadow* shadow)
{
for (ShadowRoot* root = shadow->youngestShadowRoot(); root; root = root->olderShadowRoot())
- collectDescendant(root, IncludeRoot);
+ collectFrameOwners(root);
}
void ChildFrameDisconnector::Target::disconnect()
Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h (134816 => 134817)
--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h 2012-11-15 20:38:34 UTC (rev 134817)
@@ -265,33 +265,22 @@
class ChildFrameDisconnector {
public:
- enum ShouldIncludeRoot {
- DoNotIncludeRoot,
- IncludeRoot
+ enum DisconnectPolicy {
+ RootAndDescendants,
+ DescendantsOnly
};
- explicit ChildFrameDisconnector(Node* root, ShouldIncludeRoot shouldIncludeRoot = IncludeRoot)
+ explicit ChildFrameDisconnector(Node* root)
: m_root(root)
{
- // If we know there's no frames to disconnect then don't bother traversing
- // the tree looking for them.
- Frame* frame = root->document()->frame();
- if (frame && !frame->tree()->firstChild())
- return;
- collectDescendant(m_root, shouldIncludeRoot);
}
- ~ChildFrameDisconnector()
- {
- }
+ void disconnect(DisconnectPolicy = RootAndDescendants);
- void disconnect();
-
- static bool nodeHasDisconnector(Node*);
-
private:
- void collectDescendant(Node* root, ShouldIncludeRoot);
- void collectDescendant(ElementShadow*);
+ void collectFrameOwners(Node* root);
+ void collectFrameOwners(ElementShadow*);
+ void disconnectCollectedFrameOwners();
class Target {
public:
@@ -313,21 +302,25 @@
Node* m_root;
};
-inline void ChildFrameDisconnector::collectDescendant(Node* root, ShouldIncludeRoot shouldIncludeRoot)
+inline void ChildFrameDisconnector::collectFrameOwners(Node* root)
{
- for (Node* node = shouldIncludeRoot == IncludeRoot ? root : root->firstChild(); node;
- node = node->traverseNextNode(root)) {
- if (!node->isElementNode())
- continue;
- Element* element = toElement(node);
- if (element->hasCustomCallbacks() && element->isFrameOwnerElement())
- m_list.append(toFrameOwnerElement(element));
- if (ElementShadow* shadow = element->shadow())
- collectDescendant(shadow);
- }
+ if (!root->connectedSubframeCount())
+ return;
+
+ // FIXME: This should just check isElementNode() to avoid the virtual call
+ // and we should not depend on hasCustomCallbacks().
+ if (root->hasCustomCallbacks() && root->isFrameOwnerElement())
+ m_list.append(toFrameOwnerElement(root));
+
+ for (Node* child = root->firstChild(); child; child = child->nextSibling())
+ collectFrameOwners(child);
+
+ ElementShadow* shadow = root->isElementNode() ? toElement(root)->shadow() : 0;
+ if (shadow)
+ collectFrameOwners(shadow);
}
-inline void ChildFrameDisconnector::disconnect()
+inline void ChildFrameDisconnector::disconnectCollectedFrameOwners()
{
// Must disable frame loading in the subtree so an unload handler cannot
// insert more frames and create loaded frames in detached subtrees.
@@ -340,6 +333,21 @@
}
}
+inline void ChildFrameDisconnector::disconnect(DisconnectPolicy policy)
+{
+ if (!m_root->connectedSubframeCount())
+ return;
+
+ if (policy == RootAndDescendants)
+ collectFrameOwners(m_root);
+ else {
+ for (Node* child = m_root->firstChild(); child; child = child->nextSibling())
+ collectFrameOwners(child);
+ }
+
+ disconnectCollectedFrameOwners();
+}
+
} // namespace WebCore
#endif // ContainerNodeAlgorithms_h
Modified: trunk/Source/WebCore/dom/Node.cpp (134816 => 134817)
--- trunk/Source/WebCore/dom/Node.cpp 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-11-15 20:38:34 UTC (rev 134817)
@@ -2824,6 +2824,22 @@
range->textRects(rects);
}
+unsigned Node::connectedSubframeCount() const
+{
+ return hasRareData() ? rareData()->connectedSubframeCount() : 0;
+}
+
+void Node::incrementConnectedSubframeCount()
+{
+ ASSERT(isContainerNode());
+ ensureRareData()->incrementConnectedSubframeCount();
+}
+
+void Node::decrementConnectedSubframeCount()
+{
+ rareData()->decrementConnectedSubframeCount();
+}
+
void Node::registerScopedHTMLStyleChild()
{
setHasScopedHTMLStyleChild(true);
Modified: trunk/Source/WebCore/dom/Node.h (134816 => 134817)
--- trunk/Source/WebCore/dom/Node.h 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/dom/Node.h 2012-11-15 20:38:34 UTC (rev 134817)
@@ -685,6 +685,10 @@
void textRects(Vector<IntRect>&) const;
+ unsigned connectedSubframeCount() const;
+ void incrementConnectedSubframeCount();
+ void decrementConnectedSubframeCount();
+
private:
enum NodeFlags {
IsTextFlag = 1,
Modified: trunk/Source/WebCore/dom/NodeRareData.h (134816 => 134817)
--- trunk/Source/WebCore/dom/NodeRareData.h 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/dom/NodeRareData.h 2012-11-15 20:38:34 UTC (rev 134817)
@@ -27,6 +27,7 @@
#include "DynamicNodeList.h"
#include "MutationObserver.h"
#include "MutationObserverRegistration.h"
+#include "Page.h"
#include "QualifiedName.h"
#include "TagNodeList.h"
#include <wtf/HashSet.h>
@@ -190,6 +191,7 @@
, m_needsFocusAppearanceUpdateSoonAfterAttach(false)
, m_styleAffectedByEmpty(false)
, m_isInCanvasSubtree(false)
+ , m_connectedFrameCount(0)
#if ENABLE(FULLSCREEN_API)
, m_containsFullScreenElement(false)
#endif
@@ -297,6 +299,10 @@
bool isFocused() const { return m_isFocused; }
void setFocused(bool focused) { m_isFocused = focused; }
+ unsigned connectedSubframeCount() const { return m_connectedFrameCount; }
+ void incrementConnectedSubframeCount() { m_connectedFrameCount++; }
+ void decrementConnectedSubframeCount() { ASSERT(m_connectedFrameCount); m_connectedFrameCount--; }
+
virtual void reportMemoryUsage(MemoryObjectInfo*) const;
protected:
@@ -322,6 +328,7 @@
bool m_needsFocusAppearanceUpdateSoonAfterAttach : 1;
bool m_styleAffectedByEmpty : 1;
bool m_isInCanvasSubtree : 1;
+ unsigned m_connectedFrameCount : 10;
#if ENABLE(FULLSCREEN_API)
bool m_containsFullScreenElement : 1;
#endif
@@ -338,6 +345,9 @@
#endif
};
+// Ensure the 10 bits reserved for the m_connectedFrameCount cannot overflow
+COMPILE_ASSERT(Page::maxNumberOfFrames < 1024, Frame_limit_should_fit_in_rare_data_count);
+
} // namespace WebCore
#endif // NodeRareData_h
Modified: trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp (134816 => 134817)
--- trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp 2012-11-15 20:35:40 UTC (rev 134816)
+++ trunk/Source/WebCore/html/HTMLFrameOwnerElement.cpp 2012-11-15 20:38:34 UTC (rev 134817)
@@ -57,13 +57,21 @@
// Disconnected frames should not be allowed to load.
ASSERT(inDocument());
m_contentFrame = frame;
+
+ for (ContainerNode* node = this; node; node = node->parentOrHostNode())
+ node->incrementConnectedSubframeCount();
}
void HTMLFrameOwnerElement::disconnectContentFrame()
{
ASSERT(hasCustomCallbacks());
- // This causes an unload event thus cannot be a part of removedFrom().
+ // This causes an unload event in the subframe so it cannot be a part of
+ // removedFrom() because the unload handler in a same domain frame must be
+ // able to reach upward into the owner document.
if (Frame* frame = contentFrame()) {
+ for (ContainerNode* node = this; node; node = node->parentOrHostNode())
+ node->decrementConnectedSubframeCount();
+
RefPtr<Frame> protect(frame);
frame->loader()->frameDetached();
frame->disconnectOwnerElement();
