Title: [213804] releases/WebKitGTK/webkit-2.16
- Revision
- 213804
- Author
- [email protected]
- Date
- 2017-03-13 03:19:03 -0700 (Mon, 13 Mar 2017)
Log Message
Merge r213311 - We should prevent load of subframes inserted during FrameTree deconstruction
https://bugs.webkit.org/show_bug.cgi?id=169095
Reviewed by Brent Fulgham.
Source/WebCore:
When deconstructing the FrameTree, we fire the unload event in each subframe.
Such unload event handler may insert a new frame, we would previously load
such new frame which was unsafe as we would end up with an attached subframe
on a detached tree. To address the issue, we prevent new subframes from loading
while deconstructing the FrameTree and firing the unload events. This new
behavior is consistent with Chrome and should therefore be safe from a
compatibility standpoint.
Test: fast/frames/insert-frame-unload-handler.html
* dom/ContainerNodeAlgorithms.cpp:
(WebCore::disconnectSubframes):
Update SubframeLoadingDisabler call site now that the constructor takes in
a pointer instead of a reference.
* html/HTMLFrameOwnerElement.h:
(WebCore::SubframeLoadingDisabler::SubframeLoadingDisabler):
(WebCore::SubframeLoadingDisabler::~SubframeLoadingDisabler):
Update SubframeLoadingDisabler constructor to take in a pointer instead
of a reference, for convenience.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::detachChildren):
Prevent loads in subframes while detaching the subframes. It would be unsafe
as we copy the list of frames before iterating to fire the unload events.
Therefore, newly inserted frames would not get unloaded.
LayoutTests:
Add layout test coverage. Our behavior on this test is consistent with Chrome.
* fast/frames/insert-frame-unload-handler-expected.txt: Added.
* fast/frames/insert-frame-unload-handler.html: Added.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (213803 => 213804)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-03-13 10:10:58 UTC (rev 213803)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog 2017-03-13 10:19:03 UTC (rev 213804)
@@ -1,3 +1,15 @@
+2017-03-02 Chris Dumez <[email protected]>
+
+ We should prevent load of subframes inserted during FrameTree deconstruction
+ https://bugs.webkit.org/show_bug.cgi?id=169095
+
+ Reviewed by Brent Fulgham.
+
+ Add layout test coverage. Our behavior on this test is consistent with Chrome.
+
+ * fast/frames/insert-frame-unload-handler-expected.txt: Added.
+ * fast/frames/insert-frame-unload-handler.html: Added.
+
2017-03-02 Tomas Popela <[email protected]>
[WK2] Keyboard menu key should show context menu
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler-expected.txt (0 => 213804)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler-expected.txt 2017-03-13 10:19:03 UTC (rev 213804)
@@ -0,0 +1,9 @@
+This test passes if it does not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler.html (0 => 213804)
--- releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler.html (rev 0)
+++ releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler.html 2017-03-13 10:19:03 UTC (rev 213804)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<body>
+<script src=""
+<script>
+description("This test passes if it does not crash.");
+jsTestIsAsync = true;
+
+let topFrame = document.body.appendChild(document.createElement('iframe'));
+let subframe1 = topFrame.contentDocument.body.appendChild(document.createElement('iframe'));
+subframe1.contentWindow._onunload_ = () => {
+ subframe1.contentWindow._onunload_ = null;
+
+ let subframe2 = topFrame.contentDocument.body.appendChild(document.createElement('iframe'));
+ if (!subframe2.contentWindow) {
+ setTimeout(finishJSTest, 0);
+ return;
+ }
+ subframe2.contentWindow._onunload_ = () => {
+ subframe2.contentWindow._onunload_ = null;
+
+ // Navigate top frame.
+ let a = topFrame.contentDocument.createElement('a');
+ a.href = '';
+ a.click();
+
+ setTimeout(finishJSTest, 0);
+ };
+};
+
+topFrame.src = '';
+</script>
+<script src=""
+</body>
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (213803 => 213804)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-03-13 10:10:58 UTC (rev 213803)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog 2017-03-13 10:19:03 UTC (rev 213804)
@@ -1,3 +1,37 @@
+2017-03-02 Chris Dumez <[email protected]>
+
+ We should prevent load of subframes inserted during FrameTree deconstruction
+ https://bugs.webkit.org/show_bug.cgi?id=169095
+
+ Reviewed by Brent Fulgham.
+
+ When deconstructing the FrameTree, we fire the unload event in each subframe.
+ Such unload event handler may insert a new frame, we would previously load
+ such new frame which was unsafe as we would end up with an attached subframe
+ on a detached tree. To address the issue, we prevent new subframes from loading
+ while deconstructing the FrameTree and firing the unload events. This new
+ behavior is consistent with Chrome and should therefore be safe from a
+ compatibility standpoint.
+
+ Test: fast/frames/insert-frame-unload-handler.html
+
+ * dom/ContainerNodeAlgorithms.cpp:
+ (WebCore::disconnectSubframes):
+ Update SubframeLoadingDisabler call site now that the constructor takes in
+ a pointer instead of a reference.
+
+ * html/HTMLFrameOwnerElement.h:
+ (WebCore::SubframeLoadingDisabler::SubframeLoadingDisabler):
+ (WebCore::SubframeLoadingDisabler::~SubframeLoadingDisabler):
+ Update SubframeLoadingDisabler constructor to take in a pointer instead
+ of a reference, for convenience.
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::detachChildren):
+ Prevent loads in subframes while detaching the subframes. It would be unsafe
+ as we copy the list of frames before iterating to fire the unload events.
+ Therefore, newly inserted frames would not get unloaded.
+
2017-03-02 Tomas Popela <[email protected]>
[WK2] Keyboard menu key should show context menu
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (213803 => 213804)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2017-03-13 10:10:58 UTC (rev 213803)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/ContainerNodeAlgorithms.cpp 2017-03-13 10:19:03 UTC (rev 213804)
@@ -297,7 +297,7 @@
// Must disable frame loading in the subtree so an unload handler cannot
// insert more frames and create loaded frames in detached subtrees.
- SubframeLoadingDisabler disabler(root);
+ SubframeLoadingDisabler disabler(&root);
bool isFirst = true;
for (auto& owner : frameOwners) {
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/html/HTMLFrameOwnerElement.h (213803 => 213804)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/html/HTMLFrameOwnerElement.h 2017-03-13 10:10:58 UTC (rev 213803)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/html/HTMLFrameOwnerElement.h 2017-03-13 10:19:03 UTC (rev 213804)
@@ -73,15 +73,17 @@
class SubframeLoadingDisabler {
public:
- explicit SubframeLoadingDisabler(ContainerNode& root)
+ explicit SubframeLoadingDisabler(ContainerNode* root)
: m_root(root)
{
- disabledSubtreeRoots().add(&m_root);
+ if (m_root)
+ disabledSubtreeRoots().add(m_root);
}
~SubframeLoadingDisabler()
{
- disabledSubtreeRoots().remove(&m_root);
+ if (m_root)
+ disabledSubtreeRoots().remove(m_root);
}
static bool canLoadFrame(HTMLFrameOwnerElement&);
@@ -93,7 +95,7 @@
return nodes;
}
- ContainerNode& m_root;
+ ContainerNode* m_root;
};
} // namespace WebCore
Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp (213803 => 213804)
--- releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-03-13 10:10:58 UTC (rev 213803)
+++ releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp 2017-03-13 10:19:03 UTC (rev 213804)
@@ -2430,6 +2430,11 @@
// https://html.spec.whatwg.org/multipage/browsers.html#unload-a-document
IgnoreOpensDuringUnloadCountIncrementer ignoreOpensDuringUnloadCountIncrementer(m_frame.document());
+ // Any subframe inserted by unload event handlers executed in the loop below will not get unloaded
+ // because we create a copy of the subframes list before looping. Therefore, it would be unsafe to
+ // allow loading of subframes at this point.
+ SubframeLoadingDisabler subframeLoadingDisabler(m_frame.document());
+
Vector<Ref<Frame>, 16> childrenToDetach;
childrenToDetach.reserveInitialCapacity(m_frame.tree().childCount());
for (Frame* child = m_frame.tree().lastChild(); child; child = child->tree().previousSibling())
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
