<!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>[195288] 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/195288">195288</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-01-19 08:49:58 -0800 (Tue, 19 Jan 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>[GTK] Runtime critical warnings when loading a URL after a session restore
https://bugs.webkit.org/show_bug.cgi?id=153233

Reviewed by Michael Catanzaro.

Source/WebKit2:

This happens when doing a normal load after restoring the back
forward list from session state and the list contained forward
items. In that case the forward items are removed from the list
and we try to reference a WebBackForwardListItem wrapper that
hasn't been created. We create the wrappers on demand, and when
the back forward list is populated from session state, items are
added to the list without creating their wrappers. That was not
possible before, and that's why we assumed that any item that is
removed from the list should have a wrapper already created.

* UIProcess/API/gtk/WebKitBackForwardList.cpp:
(webkitBackForwardListChanged): If we don't have a wrapper for the
removed item, create a new one to be passed to the signal, but
without adding it to the map.

Tools:

Add new test case.

* TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp:
(viewLoadChanged):
(testWebKitWebViewNavigationAfterSessionRestore):
(beforeAll):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2UIProcessAPIgtkWebKitBackForwardListcpp">trunk/Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKit2GtkTestBackForwardListcpp">trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (195287 => 195288)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog        2016-01-19 15:40:41 UTC (rev 195287)
+++ trunk/Source/WebKit2/ChangeLog        2016-01-19 16:49:58 UTC (rev 195288)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2016-01-19  Carlos Garcia Campos  &lt;cgarcia@igalia.com&gt;
+
+        [GTK] Runtime critical warnings when loading a URL after a session restore
+        https://bugs.webkit.org/show_bug.cgi?id=153233
+
+        Reviewed by Michael Catanzaro.
+
+        This happens when doing a normal load after restoring the back
+        forward list from session state and the list contained forward
+        items. In that case the forward items are removed from the list
+        and we try to reference a WebBackForwardListItem wrapper that
+        hasn't been created. We create the wrappers on demand, and when
+        the back forward list is populated from session state, items are
+        added to the list without creating their wrappers. That was not
+        possible before, and that's why we assumed that any item that is
+        removed from the list should have a wrapper already created.
+
+        * UIProcess/API/gtk/WebKitBackForwardList.cpp:
+        (webkitBackForwardListChanged): If we don't have a wrapper for the
+        removed item, create a new one to be passed to the signal, but
+        without adding it to the map.
+
</ins><span class="cx"> 2016-01-19  Ryosuke Niwa  &lt;rniwa@webkit.org&gt;
</span><span class="cx"> 
</span><span class="cx">         CharacterData::setData doesn't need ExceptionCode as an out argument
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessAPIgtkWebKitBackForwardListcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp (195287 => 195288)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp        2016-01-19 15:40:41 UTC (rev 195287)
+++ trunk/Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp        2016-01-19 16:49:58 UTC (rev 195288)
</span><span class="lines">@@ -135,8 +135,15 @@
</span><span class="cx"> 
</span><span class="cx">     WebKitBackForwardListPrivate* priv = backForwardList-&gt;priv;
</span><span class="cx">     for (auto&amp; webItem : webRemovedItems) {
</span><del>-        removedItems = g_list_prepend(removedItems, g_object_ref(priv-&gt;itemsMap.get(webItem.get()).get()));
-        priv-&gt;itemsMap.remove(webItem.get());
</del><ins>+        // After a session restore, we still don't have wrappers for the newly added items, so it would be possible that
+        // the removed items are not in the map. In that case we create a wrapper now to pass it the changed signal, but
+        // without adding it to the item map. See https://bugs.webkit.org/show_bug.cgi?id=153233.
+        GRefPtr&lt;WebKitBackForwardListItem&gt; removedItem = priv-&gt;itemsMap.get(webItem.get());
+        if (removedItem) {
+            removedItems = g_list_prepend(removedItems, g_object_ref(removedItem.get()));
+            priv-&gt;itemsMap.remove(webItem.get());
+        } else
+            removedItems = g_list_prepend(removedItems, webkitBackForwardListItemGetOrCreate(webItem.get()));
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     g_signal_emit(backForwardList, signals[CHANGED], 0, addedItem, removedItems, nullptr);
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (195287 => 195288)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog        2016-01-19 15:40:41 UTC (rev 195287)
+++ trunk/Tools/ChangeLog        2016-01-19 16:49:58 UTC (rev 195288)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2016-01-19  Carlos Garcia Campos  &lt;cgarcia@igalia.com&gt;
+
+        [GTK] Runtime critical warnings when loading a URL after a session restore
+        https://bugs.webkit.org/show_bug.cgi?id=153233
+
+        Reviewed by Michael Catanzaro.
+
+        Add new test case.
+
+        * TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp:
+        (viewLoadChanged):
+        (testWebKitWebViewNavigationAfterSessionRestore):
+        (beforeAll):
+
</ins><span class="cx"> 2016-01-19  Michael Catanzaro  &lt;mcatanzaro@igalia.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [GTK] Remove jhbuild-optional.modules
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKit2GtkTestBackForwardListcpp"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp (195287 => 195288)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp        2016-01-19 15:40:41 UTC (rev 195287)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp        2016-01-19 16:49:58 UTC (rev 195288)
</span><span class="lines">@@ -366,6 +366,38 @@
</span><span class="cx">     webkit_web_view_session_state_unref(state);
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+static void viewLoadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent, GMainLoop* mainLoop)
+{
+    if (loadEvent == WEBKIT_LOAD_FINISHED)
+        g_main_loop_quit(mainLoop);
+}
+
+static void testWebKitWebViewNavigationAfterSessionRestore(BackForwardListTest* test, gconstpointer)
+{
+    // This test checks that a normal load after a session restore with a BackForard list having
+    // forward items doesn't produce any runtime critical warning. See https://bugs.webkit.org/show_bug.cgi?id=153233.
+    GRefPtr&lt;WebKitWebView&gt; view = WEBKIT_WEB_VIEW(webkit_web_view_new());
+    g_signal_connect(view.get(), &quot;load-changed&quot;, G_CALLBACK(viewLoadChanged), test-&gt;m_mainLoop);
+
+    webkit_web_view_load_uri(view.get(), kServer-&gt;getURIForPath(&quot;/Page1&quot;).data());
+    g_main_loop_run(test-&gt;m_mainLoop);
+    webkit_web_view_load_uri(view.get(), kServer-&gt;getURIForPath(&quot;/Page2&quot;).data());
+    g_main_loop_run(test-&gt;m_mainLoop);
+    webkit_web_view_load_uri(view.get(), kServer-&gt;getURIForPath(&quot;/Page3&quot;).data());
+    g_main_loop_run(test-&gt;m_mainLoop);
+    webkit_web_view_go_back(view.get());
+    g_main_loop_run(test-&gt;m_mainLoop);
+
+    WebKitWebViewSessionState* state = webkit_web_view_get_session_state(view.get());
+    webkit_web_view_restore_session_state(test-&gt;m_webView, state);
+    webkit_web_view_session_state_unref(state);
+
+    // A normal load after a session restore should remove the forward list, add the new item and update the current one.
+    test-&gt;m_changedFlags = BackForwardListTest::CurrentItem | BackForwardListTest::AddedItem | BackForwardListTest::RemovedItems;
+    test-&gt;loadURI(kServer-&gt;getURIForPath(&quot;/Page4&quot;).data());
+    test-&gt;waitUntilLoadFinished();
+}
+
</ins><span class="cx"> void beforeAll()
</span><span class="cx"> {
</span><span class="cx">     kServer = new WebKitTestServer();
</span><span class="lines">@@ -375,6 +407,7 @@
</span><span class="cx">     BackForwardListTest::add(&quot;BackForwardList&quot;, &quot;list-limit-and-cache&quot;, testBackForwardListLimitAndCache);
</span><span class="cx">     BackForwardListTest::add(&quot;WebKitWebView&quot;, &quot;session-state&quot;, testWebKitWebViewSessionState);
</span><span class="cx">     BackForwardListTest::add(&quot;WebKitWebView&quot;, &quot;session-state-with-form-data&quot;, testWebKitWebViewSessionStateWithFormData);
</span><ins>+    BackForwardListTest::add(&quot;WebKitWebView&quot;, &quot;navigation-after-session-restore&quot;, testWebKitWebViewNavigationAfterSessionRestore);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void afterAll()
</span></span></pre>
</div>
</div>

</body>
</html>