<!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>[196807] 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/196807">196807</a></dd>
<dt>Author</dt> <dd>aestes@apple.com</dd>
<dt>Date</dt> <dd>2016-02-19 01:29:44 -0800 (Fri, 19 Feb 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Revert to dispatching the popstate event synchronously
https://bugs.webkit.org/show_bug.cgi?id=153297
rdar://problem/24092294

Reviewed by Brent Fulgham.

Source/WebCore:

<a href="http://trac.webkit.org/projects/webkit/changeset/192369">r192369</a> made the popstate event dispatch asynchronously, which matches what the HTML5 spec says to do. However,
due to compatibility regressions we need to revert back to dispatching synchronously. This change reverts
<a href="http://trac.webkit.org/projects/webkit/changeset/192369">r192369</a>'s changes to Document.cpp, but retains the new tests.

Firing popstate synchronously makes both fast/loader/remove-iframe-during-history-navigation-different.html and
fast/loader/remove-iframe-during-history-navigation-same.html crash, because their onpopstate handlers remove
frames from the document that will later be accessed by HistoryController::recursiveGoToItem().

To prevent the crashes, this change does two things:
1. Keep a reference to the current frame inside FrameLoader::loadSameDocumentItem(), since calling
   loadInSameDocument() might otherwise delete it.
2. Handle a null frame when iterating a HistoryItem's child frames in HistoryController::recursiveGoToItem(),
   since calling goToItem() on one frame might cause another frame to be deleted.

Covered by existing tests. fast/loader/stateobjects/popstate-is-asynchronous.html was renamed to
fast/loader/stateobjects/popstate-is-synchronous.html and modified to expect synchronous dispatch.

* dom/Document.cpp:
(WebCore::Document::enqueuePopstateEvent):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadSameDocumentItem):
* loader/HistoryController.cpp:
(WebCore::HistoryController::recursiveGoToItem):

LayoutTests:

Renamed and modified this test to expect synchronous dispatch.

* fast/loader/stateobjects/popstate-is-synchronous-expected.txt: Renamed from LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt.
* fast/loader/stateobjects/popstate-is-synchronous.html: Renamed from LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html.</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="#trunkSourceWebCoredomDocumentcpp">trunk/Source/WebCore/dom/Document.cpp</a></li>
<li><a href="#trunkSourceWebCoreloaderFrameLoadercpp">trunk/Source/WebCore/loader/FrameLoader.cpp</a></li>
<li><a href="#trunkSourceWebCoreloaderHistoryControllercpp">trunk/Source/WebCore/loader/HistoryController.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastloaderstateobjectspopstateissynchronousexpectedtxt">trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastloaderstateobjectspopstateissynchronoushtml">trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous.html</a></li>
</ul>

<h3>Removed Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastloaderstateobjectspopstateisasynchronousexpectedtxt">trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastloaderstateobjectspopstateisasynchronoushtml">trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/LayoutTests/ChangeLog        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2016-02-18  Andy Estes  &lt;aestes@apple.com&gt;
+
+        Revert to dispatching the popstate event synchronously
+        https://bugs.webkit.org/show_bug.cgi?id=153297
+        rdar://problem/24092294
+
+        Reviewed by Brent Fulgham.
+
+        Renamed and modified this test to expect synchronous dispatch.
+
+        * fast/loader/stateobjects/popstate-is-synchronous-expected.txt: Renamed from LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt.
+        * fast/loader/stateobjects/popstate-is-synchronous.html: Renamed from LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html.
+
</ins><span class="cx"> 2016-02-18  Philippe Normand  &lt;pnormand@igalia.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [GStreamer] Bump internal jhbuild versions to 1.6.3
</span></span></pre></div>
<a id="trunkLayoutTestsfastloaderstateobjectspopstateisasynchronousexpectedtxt"></a>
<div class="delfile"><h4>Deleted: trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -1,13 +0,0 @@
</span><del>-Tests that popstate events fire asynchronously.
-
-On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
-
-onload fired
-popstate fired
-Setting hash to #foo
-Set hash to #foo
-popstate fired
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
</del></span></pre></div>
<a id="trunkLayoutTestsfastloaderstateobjectspopstateisasynchronoushtml"></a>
<div class="delfile"><h4>Deleted: trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -1,40 +0,0 @@
</span><del>-&lt;!DOCTYPE html&gt;
-&lt;html&gt;
-&lt;body&gt;
-&lt;link rel=&quot;stylesheet&quot; href=&quot;../../../resources/js-test-style.css&quot; type=&quot;text/css&quot;&gt;
-&lt;script src=&quot;../../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
-&lt;div id=&quot;description&quot;&gt;&lt;/div&gt;
-&lt;pre id=&quot;console&quot;&gt;&lt;/pre&gt;
-&lt;script&gt;
-description('Tests that popstate events fire asynchronously.');
-
-window.onload = function()
-{
-    debug('onload fired');
-    history.pushState(&quot;test&quot;, &quot;test&quot;);
-    history.back();
-}
-
-var initialPopState = true;
-
-window.onpopstate = function()
-{
-    debug('popstate fired');
-
-    if (initialPopState) {
-        initialPopState = false;
-
-        // This should not be re-entrant; there should be no other log lines
-        // between the &quot;Setting...&quot; and &quot;Set...&quot; lines.
-        debug('Setting hash to #foo');
-        location.hash = '#foo';
-        debug('Set hash to #foo');
-    } else
-        finishJSTest();
-}
-
-var successfullyParsed = true;
-var jsTestIsAsync = true;
-&lt;/script&gt;
-&lt;script src=&quot;../../../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
-&lt;/html&gt;
</del></span></pre></div>
<a id="trunkLayoutTestsfastloaderstateobjectspopstateissynchronousexpectedtxtfromrev196805trunkLayoutTestsfastloaderstateobjectspopstateisasynchronousexpectedtxt"></a>
<div class="copfile"><h4>Copied: trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous-expected.txt (from rev 196805, trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous-expected.txt) (0 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous-expected.txt                                (rev 0)
+++ trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous-expected.txt        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -0,0 +1,13 @@
</span><ins>+Tests that popstate events fire synchronously during fragment navigation.
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+onload fired
+popstate fired
+Setting hash to #foo
+popstate fired
+Set hash to #foo
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastloaderstateobjectspopstateissynchronoushtmlfromrev196805trunkLayoutTestsfastloaderstateobjectspopstateisasynchronoushtml"></a>
<div class="copfile"><h4>Copied: trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous.html (from rev 196805, trunk/LayoutTests/fast/loader/stateobjects/popstate-is-asynchronous.html) (0 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous.html                                (rev 0)
+++ trunk/LayoutTests/fast/loader/stateobjects/popstate-is-synchronous.html        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -0,0 +1,40 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;body&gt;
+&lt;link rel=&quot;stylesheet&quot; href=&quot;../../../resources/js-test-style.css&quot; type=&quot;text/css&quot;&gt;
+&lt;script src=&quot;../../../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;div id=&quot;description&quot;&gt;&lt;/div&gt;
+&lt;pre id=&quot;console&quot;&gt;&lt;/pre&gt;
+&lt;script&gt;
+description('Tests that popstate events fire synchronously during fragment navigation.');
+
+window.onload = function()
+{
+    debug('onload fired');
+    history.pushState(&quot;test&quot;, &quot;test&quot;);
+    history.back();
+}
+
+var initialPopState = true;
+
+window.onpopstate = function()
+{
+    debug('popstate fired');
+
+    if (!initialPopState) {
+        window.setTimeout(finishJSTest, 0);
+        return;
+    }
+
+    initialPopState = false;
+
+    debug('Setting hash to #foo');
+    location.hash = '#foo';
+    debug('Set hash to #foo');
+}
+
+var successfullyParsed = true;
+var jsTestIsAsync = true;
+&lt;/script&gt;
+&lt;script src=&quot;../../../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/Source/WebCore/ChangeLog        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2016-02-18  Andy Estes  &lt;aestes@apple.com&gt;
+
+        Revert to dispatching the popstate event synchronously
+        https://bugs.webkit.org/show_bug.cgi?id=153297
+        rdar://problem/24092294
+
+        Reviewed by Brent Fulgham.
+
+        r192369 made the popstate event dispatch asynchronously, which matches what the HTML5 spec says to do. However,
+        due to compatibility regressions we need to revert back to dispatching synchronously. This change reverts
+        r192369's changes to Document.cpp, but retains the new tests.
+
+        Firing popstate synchronously makes both fast/loader/remove-iframe-during-history-navigation-different.html and
+        fast/loader/remove-iframe-during-history-navigation-same.html crash, because their onpopstate handlers remove
+        frames from the document that will later be accessed by HistoryController::recursiveGoToItem().
+
+        To prevent the crashes, this change does two things:
+        1. Keep a reference to the current frame inside FrameLoader::loadSameDocumentItem(), since calling
+           loadInSameDocument() might otherwise delete it.
+        2. Handle a null frame when iterating a HistoryItem's child frames in HistoryController::recursiveGoToItem(),
+           since calling goToItem() on one frame might cause another frame to be deleted.
+
+        Covered by existing tests. fast/loader/stateobjects/popstate-is-asynchronous.html was renamed to
+        fast/loader/stateobjects/popstate-is-synchronous.html and modified to expect synchronous dispatch.
+
+        * dom/Document.cpp:
+        (WebCore::Document::enqueuePopstateEvent):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadSameDocumentItem):
+        * loader/HistoryController.cpp:
+        (WebCore::HistoryController::recursiveGoToItem):
+
</ins><span class="cx"> 2016-02-19  Carlos Garcia Campos  &lt;cgarcia@igalia.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Unreviewed. Fix GObject DOM bindings API break after r196769.
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Document.cpp (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Document.cpp        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/Source/WebCore/dom/Document.cpp        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -5511,7 +5511,7 @@
</span><span class="cx"> 
</span><span class="cx"> void Document::enqueuePopstateEvent(RefPtr&lt;SerializedScriptValue&gt;&amp;&amp; stateObject)
</span><span class="cx"> {
</span><del>-    enqueueWindowEvent(PopStateEvent::create(WTFMove(stateObject), m_domWindow ? m_domWindow-&gt;history() : nullptr));
</del><ins>+    dispatchWindowEvent(PopStateEvent::create(WTFMove(stateObject), m_domWindow ? m_domWindow-&gt;history() : nullptr));
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void Document::addMediaCanStartListener(MediaCanStartListener* listener)
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderFrameLoadercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/FrameLoader.cpp        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -3248,6 +3248,8 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(item.documentSequenceNumber() == history().currentItem()-&gt;documentSequenceNumber());
</span><span class="cx"> 
</span><ins>+    Ref&lt;Frame&gt; protect(m_frame);
+
</ins><span class="cx">     // Save user view state to the current history item here since we don't do a normal load.
</span><span class="cx">     // FIXME: Does form state need to be saved here too?
</span><span class="cx">     history().saveScrollPositionAndViewStateToItem(history().currentItem());
</span></span></pre></div>
<a id="trunkSourceWebCoreloaderHistoryControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/loader/HistoryController.cpp (196806 => 196807)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/loader/HistoryController.cpp        2016-02-19 08:56:53 UTC (rev 196806)
+++ trunk/Source/WebCore/loader/HistoryController.cpp        2016-02-19 09:29:44 UTC (rev 196807)
</span><span class="lines">@@ -754,9 +754,8 @@
</span><span class="cx"> 
</span><span class="cx">         HistoryItem* fromChildItem = fromItem-&gt;childItemWithTarget(childFrameName);
</span><span class="cx">         ASSERT(fromChildItem);
</span><del>-        Frame* childFrame = m_frame.tree().child(childFrameName);
-        ASSERT(childFrame);
-        childFrame-&gt;loader().history().recursiveGoToItem(const_cast&lt;HistoryItem&amp;&gt;(childItem.get()), fromChildItem, type);
</del><ins>+        if (Frame* childFrame = m_frame.tree().child(childFrameName))
+            childFrame-&gt;loader().history().recursiveGoToItem(const_cast&lt;HistoryItem&amp;&gt;(childItem.get()), fromChildItem, type);
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre>
</div>
</div>

</body>
</html>