<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[213804] releases/WebKitGTK/webkit-2.16</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/213804">213804</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2017-03-13 03:19:03 -0700 (Mon, 13 Mar 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Merge <a href="http://trac.webkit.org/projects/webkit/changeset/213311">r213311</a> - 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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit216LayoutTestsChangeLog">releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit216SourceWebCoreChangeLog">releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit216SourceWebCoredomContainerNodeAlgorithmscpp">releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/ContainerNodeAlgorithms.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit216SourceWebCorehtmlHTMLFrameOwnerElementh">releases/WebKitGTK/webkit-2.16/Source/WebCore/html/HTMLFrameOwnerElement.h</a></li>
<li><a href="#releasesWebKitGTKwebkit216SourceWebCoreloaderFrameLoadercpp">releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit216LayoutTestsfastframesinsertframeunloadhandlerexpectedtxt">releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler-expected.txt</a></li>
<li><a href="#releasesWebKitGTKwebkit216LayoutTestsfastframesinsertframeunloadhandlerhtml">releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit216LayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.16/LayoutTests/ChangeLog (213803 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2017-03-02 Chris Dumez <cdumez@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2017-03-02 Tomas Popela <tpopela@redhat.com>
</span><span class="cx">
</span><span class="cx"> [WK2] Keyboard menu key should show context menu
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit216LayoutTestsfastframesinsertframeunloadhandlerexpectedtxt"></a>
<div class="addfile"><h4>Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler-expected.txt (0 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,9 @@
</span><ins>+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
+
</ins></span></pre></div>
<a id="releasesWebKitGTKwebkit216LayoutTestsfastframesinsertframeunloadhandlerhtml"></a>
<div class="addfile"><h4>Added: releases/WebKitGTK/webkit-2.16/LayoutTests/fast/frames/insert-frame-unload-handler.html (0 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,33 @@
</span><ins>+<!DOCTYPE html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<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 = 'about:blank';
+ a.click();
+
+ setTimeout(finishJSTest, 0);
+ };
+};
+
+topFrame.src = 'about:blank';
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
</ins></span></pre></div>
<a id="releasesWebKitGTKwebkit216SourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/ChangeLog (213803 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2017-03-02 Chris Dumez <cdumez@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2017-03-02 Tomas Popela <tpopela@redhat.com>
</span><span class="cx">
</span><span class="cx"> [WK2] Keyboard menu key should show context menu
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit216SourceWebCoredomContainerNodeAlgorithmscpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/dom/ContainerNodeAlgorithms.cpp (213803 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -297,7 +297,7 @@
</span><span class="cx">
</span><span class="cx"> // Must disable frame loading in the subtree so an unload handler cannot
</span><span class="cx"> // insert more frames and create loaded frames in detached subtrees.
</span><del>- SubframeLoadingDisabler disabler(root);
</del><ins>+ SubframeLoadingDisabler disabler(&root);
</ins><span class="cx">
</span><span class="cx"> bool isFirst = true;
</span><span class="cx"> for (auto& owner : frameOwners) {
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit216SourceWebCorehtmlHTMLFrameOwnerElementh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/html/HTMLFrameOwnerElement.h (213803 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -73,15 +73,17 @@
</span><span class="cx">
</span><span class="cx"> class SubframeLoadingDisabler {
</span><span class="cx"> public:
</span><del>- explicit SubframeLoadingDisabler(ContainerNode& root)
</del><ins>+ explicit SubframeLoadingDisabler(ContainerNode* root)
</ins><span class="cx"> : m_root(root)
</span><span class="cx"> {
</span><del>- disabledSubtreeRoots().add(&m_root);
</del><ins>+ if (m_root)
+ disabledSubtreeRoots().add(m_root);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ~SubframeLoadingDisabler()
</span><span class="cx"> {
</span><del>- disabledSubtreeRoots().remove(&m_root);
</del><ins>+ if (m_root)
+ disabledSubtreeRoots().remove(m_root);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> static bool canLoadFrame(HTMLFrameOwnerElement&);
</span><span class="lines">@@ -93,7 +95,7 @@
</span><span class="cx"> return nodes;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- ContainerNode& m_root;
</del><ins>+ ContainerNode* m_root;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit216SourceWebCoreloaderFrameLoadercpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.16/Source/WebCore/loader/FrameLoader.cpp (213803 => 213804)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2430,6 +2430,11 @@
</span><span class="cx"> // https://html.spec.whatwg.org/multipage/browsers.html#unload-a-document
</span><span class="cx"> IgnoreOpensDuringUnloadCountIncrementer ignoreOpensDuringUnloadCountIncrementer(m_frame.document());
</span><span class="cx">
</span><ins>+ // 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());
+
</ins><span class="cx"> Vector<Ref<Frame>, 16> childrenToDetach;
</span><span class="cx"> childrenToDetach.reserveInitialCapacity(m_frame.tree().childCount());
</span><span class="cx"> for (Frame* child = m_frame.tree().lastChild(); child; child = child->tree().previousSibling())
</span></span></pre>
</div>
</div>
</body>
</html>