Title: [140856] trunk
- Revision
- 140856
- Author
- espr...@chromium.org
- Date
- 2013-01-25 12:13:32 -0800 (Fri, 25 Jan 2013)
Log Message
Consider all ancestors not just parentElement when disconnecting frames
https://bugs.webkit.org/show_bug.cgi?id=107769
Reviewed by Eric Seidel.
Source/WebCore:
Previous we only used the parentElement of the frame owner to decide if
we should disconnect the frame, but this means if you reparent a subtree
that contains multiple frames from inside an unload handler we'll disconnect
the subframes even though they're now in a different part of the document.
We can fix this by using containsIncludingShadowDOM, and also simplify the
code by removing ChildFrameDisconnector::Target.
Test: fast/frames/unload-reparent-sibling-frame.html
* dom/ContainerNodeAlgorithms.cpp:
* dom/ContainerNodeAlgorithms.h:
(ChildFrameDisconnector):
(ChildFrameDisconnector::Target): Removed.
(WebCore::ChildFrameDisconnector::collectFrameOwners):
(WebCore::ChildFrameDisconnector::disconnectCollectedFrameOwners):
LayoutTests:
Add a test for moving frames around inside unload handlers.
* fast/frames/unload-reparent-sibling-frame-expected.txt: Added.
* fast/frames/unload-reparent-sibling-frame.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (140855 => 140856)
--- trunk/LayoutTests/ChangeLog 2013-01-25 19:54:27 UTC (rev 140855)
+++ trunk/LayoutTests/ChangeLog 2013-01-25 20:13:32 UTC (rev 140856)
@@ -1,3 +1,15 @@
+2013-01-25 Elliott Sprehn <espr...@chromium.org>
+
+ Consider all ancestors not just parentElement when disconnecting frames
+ https://bugs.webkit.org/show_bug.cgi?id=107769
+
+ Reviewed by Eric Seidel.
+
+ Add a test for moving frames around inside unload handlers.
+
+ * fast/frames/unload-reparent-sibling-frame-expected.txt: Added.
+ * fast/frames/unload-reparent-sibling-frame.html: Added.
+
2013-01-25 Tony Chang <t...@chromium.org>
Re-layout child blocks when border/padding of the box-sizing:border-box parent is updated
Added: trunk/LayoutTests/fast/frames/unload-reparent-sibling-frame-expected.txt (0 => 140856)
--- trunk/LayoutTests/fast/frames/unload-reparent-sibling-frame-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/unload-reparent-sibling-frame-expected.txt 2013-01-25 20:13:32 UTC (rev 140856)
@@ -0,0 +1,21 @@
+Reparented sibling frames from unload handlers should load.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS frame1.contentDocument is not null
+PASS frame2.contentDocument is not null
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+
+--------
+Frame: 'frame1'
+--------
+frame1
+
+--------
+Frame: 'frame2'
+--------
+frame2
Added: trunk/LayoutTests/fast/frames/unload-reparent-sibling-frame.html (0 => 140856)
--- trunk/LayoutTests/fast/frames/unload-reparent-sibling-frame.html (rev 0)
+++ trunk/LayoutTests/fast/frames/unload-reparent-sibling-frame.html 2013-01-25 20:13:32 UTC (rev 140856)
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+
+<script src=""
+
+<div id="container">
+ <div id="inside">
+ </div>
+</div>
+
+<script>
+description('Reparented sibling frames from unload handlers should load.');
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.dumpChildFramesAsText();
+}
+
+function onframeload(text)
+{
+ this.contentDocument.body.innerHTML = this.id;
+}
+
+function runTest()
+{
+}
+
+var container = document.getElementById('container');
+var inside = document.getElementById('inside');
+
+var frame1 = document.createElement('iframe');
+frame1.id = 'frame1';
+frame1._onload_ = onframeload;
+
+var frame2 = document.createElement('iframe');
+frame2.id = 'frame2';
+frame2._onload_ = onframeload;
+
+_onload_ = function() {
+ inside.appendChild(frame1);
+ inside.appendChild(frame2);
+
+ frame1.contentWindow._onunload_ = function() {
+ document.body.appendChild(inside);
+ };
+
+ container.parentNode.removeChild(container);
+ shouldNotBe('frame1.contentDocument', 'null');
+ shouldNotBe('frame2.contentDocument', 'null');
+ isSuccessfullyParsed();
+};
+</script>
Modified: trunk/Source/WebCore/ChangeLog (140855 => 140856)
--- trunk/Source/WebCore/ChangeLog 2013-01-25 19:54:27 UTC (rev 140855)
+++ trunk/Source/WebCore/ChangeLog 2013-01-25 20:13:32 UTC (rev 140856)
@@ -1,3 +1,27 @@
+2013-01-25 Elliott Sprehn <espr...@chromium.org>
+
+ Consider all ancestors not just parentElement when disconnecting frames
+ https://bugs.webkit.org/show_bug.cgi?id=107769
+
+ Reviewed by Eric Seidel.
+
+ Previous we only used the parentElement of the frame owner to decide if
+ we should disconnect the frame, but this means if you reparent a subtree
+ that contains multiple frames from inside an unload handler we'll disconnect
+ the subframes even though they're now in a different part of the document.
+
+ We can fix this by using containsIncludingShadowDOM, and also simplify the
+ code by removing ChildFrameDisconnector::Target.
+
+ Test: fast/frames/unload-reparent-sibling-frame.html
+
+ * dom/ContainerNodeAlgorithms.cpp:
+ * dom/ContainerNodeAlgorithms.h:
+ (ChildFrameDisconnector):
+ (ChildFrameDisconnector::Target): Removed.
+ (WebCore::ChildFrameDisconnector::collectFrameOwners):
+ (WebCore::ChildFrameDisconnector::disconnectCollectedFrameOwners):
+
2013-01-25 Tony Chang <t...@chromium.org>
Re-layout child blocks when border/padding of the box-sizing:border-box parent is updated
Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (140855 => 140856)
--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2013-01-25 19:54:27 UTC (rev 140855)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2013-01-25 20:13:32 UTC (rev 140856)
@@ -115,12 +115,6 @@
collectFrameOwners(root);
}
-void ChildFrameDisconnector::Target::disconnect()
-{
- ASSERT(isValid());
- toFrameOwnerElement(m_owner.get())->disconnectContentFrame();
-}
-
#ifndef NDEBUG
unsigned assertConnectedSubrameCountIsConsistent(Node* node)
{
Modified: trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h (140855 => 140856)
--- trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h 2013-01-25 19:54:27 UTC (rev 140855)
+++ trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h 2013-01-25 20:13:32 UTC (rev 140856)
@@ -277,23 +277,7 @@
void collectFrameOwners(ElementShadow*);
void disconnectCollectedFrameOwners();
- class Target {
- public:
- Target(HTMLFrameOwnerElement* element)
- : m_owner(element)
- , m_ownerParent(element->parentNode())
- {
- }
-
- bool isValid() const { return m_owner->parentNode() == m_ownerParent; }
- void disconnect();
-
- private:
- RefPtr<HTMLFrameOwnerElement> m_owner;
- ContainerNode* m_ownerParent;
- };
-
- Vector<Target, 10> m_list;
+ Vector<RefPtr<HTMLFrameOwnerElement>, 10> m_frameOwners;
Node* m_root;
};
@@ -309,7 +293,7 @@
// 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));
+ m_frameOwners.append(toFrameOwnerElement(root));
for (Node* child = root->firstChild(); child; child = child->nextSibling())
collectFrameOwners(child);
@@ -324,11 +308,13 @@
// Must disable frame loading in the subtree so an unload handler cannot
// insert more frames and create loaded frames in detached subtrees.
SubframeLoadingDisabler disabler(m_root);
- unsigned size = m_list.size();
- for (unsigned i = 0; i < size; ++i) {
- Target& target = m_list[i];
- if (target.isValid())
- target.disconnect();
+
+ for (unsigned i = 0; i < m_frameOwners.size(); ++i) {
+ HTMLFrameOwnerElement* owner = m_frameOwners[i].get();
+ // Don't need to traverse up the tree for the first owner since no
+ // script could have moved it.
+ if (!i || m_root->containsIncludingShadowDOM(owner))
+ owner->disconnectContentFrame();
}
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
