Changeset 195288 in webkit

Timestamp:

Jan 19, 2016 8:49:58 AM (8 years ago)

Author:

Carlos Garcia Campos

Message:

[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):

Location:

trunk

Files:

• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp (modified) (2 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2016-01-19  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [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.
+
 2016-01-19  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp

@@ -136 +136 @@
     WebKitBackForwardListPrivate* priv = backForwardList->priv;
     for (auto& webItem : webRemovedItems) {
-        removedItems = g_list_prepend(removedItems, g_object_ref(priv->itemsMap.get(webItem.get()).get()));
-        priv->itemsMap.remove(webItem.get());
+        // 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<WebKitBackForwardListItem> removedItem = priv->itemsMap.get(webItem.get());
+        if (removedItem) {
+            removedItems = g_list_prepend(removedItems, g_object_ref(removedItem.get()));
+            priv->itemsMap.remove(webItem.get());
+        } else
+            removedItems = g_list_prepend(removedItems, webkitBackForwardListItemGetOrCreate(webItem.get()));
     }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2016-01-19  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [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):
+
 2016-01-19  Michael Catanzaro  <mcatanzaro@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp

@@ -367 +367 @@
 }
 
+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<WebKitWebView> view = WEBKIT_WEB_VIEW(webkit_web_view_new());
+    g_signal_connect(view.get(), "load-changed", G_CALLBACK(viewLoadChanged), test->m_mainLoop);
+
+    webkit_web_view_load_uri(view.get(), kServer->getURIForPath("/Page1").data());
+    g_main_loop_run(test->m_mainLoop);
+    webkit_web_view_load_uri(view.get(), kServer->getURIForPath("/Page2").data());
+    g_main_loop_run(test->m_mainLoop);
+    webkit_web_view_load_uri(view.get(), kServer->getURIForPath("/Page3").data());
+    g_main_loop_run(test->m_mainLoop);
+    webkit_web_view_go_back(view.get());
+    g_main_loop_run(test->m_mainLoop);
+
+    WebKitWebViewSessionState* state = webkit_web_view_get_session_state(view.get());
+    webkit_web_view_restore_session_state(test->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->m_changedFlags = BackForwardListTest::CurrentItem | BackForwardListTest::AddedItem | BackForwardListTest::RemovedItems;
+    test->loadURI(kServer->getURIForPath("/Page4").data());
+    test->waitUntilLoadFinished();
+}
+
 void beforeAll()
 {
@@ -376 +408 @@
     BackForwardListTest::add("WebKitWebView", "session-state", testWebKitWebViewSessionState);
     BackForwardListTest::add("WebKitWebView", "session-state-with-form-data", testWebKitWebViewSessionStateWithFormData);
+    BackForwardListTest::add("WebKitWebView", "navigation-after-session-restore", testWebKitWebViewNavigationAfterSessionRestore);
 }