<!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>[192844] trunk</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/192844">192844</a></dd>

<dt>Author</dt> <dd>jiewen_tan@apple.com</dd>

<dt>Date</dt> <dd>2015-11-30 16:33:47 -0800 (Mon, 30 Nov 2015)</dd>

</dl>

<h3>Log Message</h3>

<pre>Null dereference loading Blink layout test http/tests/misc/detach-during-notifyDone.html

https://bugs.webkit.org/show_bug.cgi?id=149309

&lt;rdar://problem/22748363&gt;

Reviewed by Brent Fulgham.

Source/WebCore:

A weird order of event execution introduced by the test case will kill the webpage in a

subframe of the page while executing its |frame.loader().checkLoadCompleteForThisFrame()|.

Therefore, any frames comes after the failing subframe will have no page. Check it before

calling to those frames' |frame.loader().checkLoadCompleteForThisFrame()|, otherwise the

assertion in |frame.loader().checkLoadCompleteForThisFrame()| will fail.

Test: http/tests/misc/detach-during-notifyDone.html

* loader/FrameLoader.cpp:

(WebCore::FrameLoader::checkLoadComplete):

Source/WebKit/mac:

* WebView/WebDataSource.mm:

(WebDataSourcePrivate::~WebDataSourcePrivate):

Refine the assertion to treat &lt;rdar://problem/9673866&gt;.

Source/WebKit2:

Callback of bundle clients could kill the documentloader. Therefore, make a copy

of the navigationID before invoking the callback.

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:

(WebKit::WebFrameLoaderClient::dispatchDidChangeLocationWithinPage):

(WebKit::WebFrameLoaderClient::dispatchDidPushStateWithinPage):

(WebKit::WebFrameLoaderClient::dispatchDidReplaceStateWithinPage):

(WebKit::WebFrameLoaderClient::dispatchDidPopStateWithinPage):

(WebKit::WebFrameLoaderClient::dispatchDidFailLoad):

(WebKit::WebFrameLoaderClient::dispatchDidFinishDocumentLoad):

(WebKit::WebFrameLoaderClient::dispatchDidFinishLoad):

LayoutTests:

The test case is from Blink <a href="http://trac.webkit.org/projects/webkit/changeset/175601">r175601</a>:

https://codereview.chromium.org/317513002

The test case will generate a set of weird ordering events that affects the documentLoader:

1. The subframe finishes loading, and since the frame’s testRunner is not set to wait until

done, WebKitTestRunner stops the load (by calling WKBundlePageStopLoading()).

2. This causes the in-progress XHR to be aborted, which causes its readyState to become DONE

(this bug doesn’t always reproduce because sometimes the XHR has already finished before the

frame finishes loading).

3. The onreadystatechange callback is executed, which sets innerHTML on the parent frame.

4. Setting innerHTML disconnects the subframe, nulling out its DocumentLoader.

5. We return to WebFrameLoaderClient::dispatchDidFinishLoad() from step #1, but now the

FrameLoader’s DocumentLoader is null. And WebKit crashes here.

Note that steps 2-4 happen synchronously inside WebFrameLoaderClient::dispatchDidFinishLoad().

* http/tests/misc/detach-during-notifyDone-expected.txt: Added.

* http/tests/misc/detach-during-notifyDone.html: Added.

* http/tests/misc/resources/detached-frame.html: Added.</pre>

<h3>Modified Paths</h3>

<ul>

<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>

<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>

<li><a href="#trunkSourceWebCoreloaderFrameLoadercpp">trunk/Source/WebCore/loader/FrameLoader.cpp</a></li>

<li><a href="#trunkSourceWebKitmacChangeLog">trunk/Source/WebKit/mac/ChangeLog</a></li>

<li><a href="#trunkSourceWebKitmacWebViewWebDataSourcemm">trunk/Source/WebKit/mac/WebView/WebDataSource.mm</a></li>

<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>

<li><a href="#trunkSourceWebKit2WebProcessWebCoreSupportWebFrameLoaderClientcpp">trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp</a></li>

</ul>

<h3>Added Paths</h3>

<ul>

<li><a href="#trunkLayoutTestshttptestsmiscdetachduringnotifyDoneexpectedtxt">trunk/LayoutTests/http/tests/misc/detach-during-notifyDone-expected.txt</a></li>

<li><a href="#trunkLayoutTestshttptestsmiscdetachduringnotifyDonehtml">trunk/LayoutTests/http/tests/misc/detach-during-notifyDone.html</a></li>

<li><a href="#trunkLayoutTestshttptestsmiscresourcesdetachedframehtml">trunk/LayoutTests/http/tests/misc/resources/detached-frame.html</a></li>

</ul>

</div>

<div id="patch">

<h3>Diff</h3>

<a id="trunkLayoutTestsChangeLog"></a>

<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/LayoutTests/ChangeLog        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/LayoutTests/ChangeLog        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -1,3 +1,30 @@

</span><ins>+2015-11-30  Jiewen Tan  &lt;jiewen_tan@apple.com&gt;

+

+        Null dereference loading Blink layout test http/tests/misc/detach-during-notifyDone.html

+        https://bugs.webkit.org/show_bug.cgi?id=149309

+        &lt;rdar://problem/22748363&gt;

+

+        Reviewed by Brent Fulgham.

+

+        The test case is from Blink r175601:

+        https://codereview.chromium.org/317513002

+        The test case will generate a set of weird ordering events that affects the documentLoader:

+        1. The subframe finishes loading, and since the frame’s testRunner is not set to wait until

+        done, WebKitTestRunner stops the load (by calling WKBundlePageStopLoading()).

+        2. This causes the in-progress XHR to be aborted, which causes its readyState to become DONE

+        (this bug doesn’t always reproduce because sometimes the XHR has already finished before the

+        frame finishes loading).

+        3. The onreadystatechange callback is executed, which sets innerHTML on the parent frame.

+        4. Setting innerHTML disconnects the subframe, nulling out its DocumentLoader.

+        5. We return to WebFrameLoaderClient::dispatchDidFinishLoad() from step #1, but now the

+        FrameLoader’s DocumentLoader is null. And WebKit crashes here.

+

+        Note that steps 2-4 happen synchronously inside WebFrameLoaderClient::dispatchDidFinishLoad().

+

+        * http/tests/misc/detach-during-notifyDone-expected.txt: Added.

+        * http/tests/misc/detach-during-notifyDone.html: Added.

+        * http/tests/misc/resources/detached-frame.html: Added.

+

</ins><span class="cx"> 2015-11-30  Commit Queue  &lt;commit-queue@webkit.org&gt;

</span><span class="cx"> 

<span class="cx">         Unreviewed, rolling out r192819.

</span></span></pre></div>

<a id="trunkLayoutTestshttptestsmiscdetachduringnotifyDoneexpectedtxt"></a>

<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/misc/detach-during-notifyDone-expected.txt (0 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/LayoutTests/http/tests/misc/detach-during-notifyDone-expected.txt                                (rev 0)

+++ trunk/LayoutTests/http/tests/misc/detach-during-notifyDone-expected.txt        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -0,0 +1 @@

<ins>+ PASS. WebKit didn't crash.

</ins></span></pre></div>

<a id="trunkLayoutTestshttptestsmiscdetachduringnotifyDonehtml"></a>

<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/misc/detach-during-notifyDone.html (0 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/LayoutTests/http/tests/misc/detach-during-notifyDone.html                                (rev 0)

+++ trunk/LayoutTests/http/tests/misc/detach-during-notifyDone.html        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -0,0 +1,8 @@

</span><ins>+&lt;iframe&gt;&lt;/iframe&gt;&lt;iframe src=resources/detached-frame.html&gt;&lt;/iframe&gt;

+&lt;script&gt;

+if (window.testRunner) {

+    testRunner.dumpAsText();

+    testRunner.waitUntilDone();

+}

+&lt;/script&gt;

+PASS. WebKit didn't crash.

</ins></span></pre></div>

<a id="trunkLayoutTestshttptestsmiscresourcesdetachedframehtml"></a>

<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/misc/resources/detached-frame.html (0 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/LayoutTests/http/tests/misc/resources/detached-frame.html                                (rev 0)

+++ trunk/LayoutTests/http/tests/misc/resources/detached-frame.html        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -0,0 +1,15 @@

</span><ins>+&lt;script&gt;

+function test() {

+    var xhr = new XMLHttpRequest;

+    xhr.open(&quot;GET&quot;, &quot;detached-frame.html&quot;);

+    xhr.send();

+    xhr.onreadystatechange = function() {

+        if (xhr.readyState == xhr.DONE) {

+            var parentWindow = window.parent;

+            parentWindow.document.body.innerHTML += &quot;&quot;;

+            parentWindow.testRunner.notifyDone();

+        }

+    }

+}

+&lt;/script&gt;

+&lt;body onload=&quot;test()&quot;&gt;

</ins><span class="cx">\ No newline at end of file

</span></span></pre></div>

<a id="trunkSourceWebCoreChangeLog"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/Source/WebCore/ChangeLog        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -1,3 +1,22 @@

</span><ins>+2015-11-30  Jiewen Tan  &lt;jiewen_tan@apple.com&gt;

+

+        Null dereference loading Blink layout test http/tests/misc/detach-during-notifyDone.html

+        https://bugs.webkit.org/show_bug.cgi?id=149309

+        &lt;rdar://problem/22748363&gt;

+

+        Reviewed by Brent Fulgham.

+

+        A weird order of event execution introduced by the test case will kill the webpage in a

+        subframe of the page while executing its |frame.loader().checkLoadCompleteForThisFrame()|.

+        Therefore, any frames comes after the failing subframe will have no page. Check it before

+        calling to those frames' |frame.loader().checkLoadCompleteForThisFrame()|, otherwise the

+        assertion in |frame.loader().checkLoadCompleteForThisFrame()| will fail.

+

+        Test: http/tests/misc/detach-during-notifyDone.html

+

+        * loader/FrameLoader.cpp:

+        (WebCore::FrameLoader::checkLoadComplete):

+

</ins><span class="cx"> 2015-11-30  Commit Queue  &lt;commit-queue@webkit.org&gt;

</span><span class="cx"> 

<span class="cx">         Unreviewed, rolling out r192819.

</span></span></pre></div>

<a id="trunkSourceWebCoreloaderFrameLoadercpp"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebCore/loader/FrameLoader.cpp        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/Source/WebCore/loader/FrameLoader.cpp        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -2431,8 +2431,10 @@

</span><span class="cx">         frames.append(*frame);

</span><span class="cx"> 

<span class="cx">     // To process children before their parents, iterate the vector backwards.

</span><del>-    for (unsigned i = frames.size(); i; --i)

-        frames[i - 1]-&gt;loader().checkLoadCompleteForThisFrame();

</del><ins>+    for (auto frame = frames.rbegin(); frame != frames.rend(); ++frame) {

+        if ((*frame)-&gt;page())

+            (*frame)-&gt;loader().checkLoadCompleteForThisFrame();

+    }

</ins><span class="cx"> }

</span><span class="cx"> 

</span><span class="cx"> int FrameLoader::numPendingOrLoadingRequests(bool recurse) const

</span></span></pre></div>

<a id="trunkSourceWebKitmacChangeLog"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit/mac/ChangeLog (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit/mac/ChangeLog        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/Source/WebKit/mac/ChangeLog        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -1 +1,13 @@

</span><ins>+2015-11-30  Jiewen Tan  &lt;jiewen_tan@apple.com&gt;

+

+        Null dereference loading Blink layout test http/tests/misc/detach-during-notifyDone.html

+        https://bugs.webkit.org/show_bug.cgi?id=149309

+        &lt;rdar://problem/22748363&gt;

+

+        Reviewed by Brent Fulgham.

+

+        * WebView/WebDataSource.mm:

+        (WebDataSourcePrivate::~WebDataSourcePrivate):

+        Refine the assertion to treat &lt;rdar://problem/9673866&gt;.

+

</ins><span class="cx"> == Rolled over to ChangeLog-2015-11-21 ==

</span></span></pre></div>

<a id="trunkSourceWebKitmacWebViewWebDataSourcemm"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit/mac/WebView/WebDataSource.mm (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit/mac/WebView/WebDataSource.mm        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/Source/WebKit/mac/WebView/WebDataSource.mm        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -92,7 +92,12 @@

</span><span class="cx">     ~WebDataSourcePrivate()

</span><span class="cx">     {

</span><span class="cx">         if (loader) {

</span><del>-            ASSERT(!loader-&gt;isLoading());

</del><ins>+            // We might run in to infinite recursion if we're stopping loading as the result of detaching from the frame.

+            // Therefore, DocumentLoader::detachFromFrame() did some smart things to stop the recursion.

+            // As a result of breaking the resursion, DocumentLoader::m_subresourceLoader

+            // and DocumentLoader::m_plugInStreamLoaders might not be empty at this time.

+            // See &lt;rdar://problem/9673866&gt; for more details.

+            ASSERT(!loader-&gt;isLoading() || loader-&gt;isStopping());

</ins><span class="cx">             loader-&gt;detachDataSource();

</span><span class="cx">         }

</span><span class="cx">     }

</span></span></pre></div>

<a id="trunkSourceWebKit2ChangeLog"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/ChangeLog        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/Source/WebKit2/ChangeLog        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -1,3 +1,23 @@

</span><ins>+2015-11-30  Jiewen Tan  &lt;jiewen_tan@apple.com&gt;

+

+        Null dereference loading Blink layout test http/tests/misc/detach-during-notifyDone.html

+        https://bugs.webkit.org/show_bug.cgi?id=149309

+        &lt;rdar://problem/22748363&gt;

+

+        Reviewed by Brent Fulgham.

+

+        Callback of bundle clients could kill the documentloader. Therefore, make a copy

+        of the navigationID before invoking the callback.

+

+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:

+        (WebKit::WebFrameLoaderClient::dispatchDidChangeLocationWithinPage):

+        (WebKit::WebFrameLoaderClient::dispatchDidPushStateWithinPage):

+        (WebKit::WebFrameLoaderClient::dispatchDidReplaceStateWithinPage):

+        (WebKit::WebFrameLoaderClient::dispatchDidPopStateWithinPage):

+        (WebKit::WebFrameLoaderClient::dispatchDidFailLoad):

+        (WebKit::WebFrameLoaderClient::dispatchDidFinishDocumentLoad):

+        (WebKit::WebFrameLoaderClient::dispatchDidFinishLoad):

+

</ins><span class="cx"> 2015-11-30  Tim Horton  &lt;timothy_horton@apple.com&gt;

</span><span class="cx"> 

</span><span class="cx">         Get rid of the !USE(ASYNC_NSTEXTINPUTCLIENT) codepath

</span></span></pre></div>

<a id="trunkSourceWebKit2WebProcessWebCoreSupportWebFrameLoaderClientcpp"></a>

<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp (192843 => 192844)</h4>

<pre class="diff"><span>

<span class="info">--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp        2015-12-01 00:09:42 UTC (rev 192843)

+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp        2015-12-01 00:33:47 UTC (rev 192844)

</span><span class="lines">@@ -327,12 +327,13 @@

</span><span class="cx"> 

</span><span class="cx">     RefPtr&lt;API::Object&gt; userData;

</span><span class="cx"> 

</span><ins>+    auto navigationID = static_cast&lt;WebDocumentLoader&amp;&gt;(*m_frame-&gt;coreFrame()-&gt;loader().documentLoader()).navigationID();

+