Title: [195288] trunk
- Revision
- 195288
- Author
- [email protected]
- Date
- 2016-01-19 08:49:58 -0800 (Tue, 19 Jan 2016)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (195287 => 195288)
--- 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)
@@ -1,3 +1,25 @@
+2016-01-19 Carlos Garcia Campos <[email protected]>
+
+ [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 <[email protected]>
CharacterData::setData doesn't need ExceptionCode as an out argument
Modified: trunk/Source/WebKit2/UIProcess/API/gtk/WebKitBackForwardList.cpp (195287 => 195288)
--- 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)
@@ -135,8 +135,15 @@
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()));
}
g_signal_emit(backForwardList, signals[CHANGED], 0, addedItem, removedItems, nullptr);
Modified: trunk/Tools/ChangeLog (195287 => 195288)
--- trunk/Tools/ChangeLog 2016-01-19 15:40:41 UTC (rev 195287)
+++ trunk/Tools/ChangeLog 2016-01-19 16:49:58 UTC (rev 195288)
@@ -1,3 +1,17 @@
+2016-01-19 Carlos Garcia Campos <[email protected]>
+
+ [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 <[email protected]>
[GTK] Remove jhbuild-optional.modules
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestBackForwardList.cpp (195287 => 195288)
--- 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)
@@ -366,6 +366,38 @@
webkit_web_view_session_state_unref(state);
}
+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()
{
kServer = new WebKitTestServer();
@@ -375,6 +407,7 @@
BackForwardListTest::add("BackForwardList", "list-limit-and-cache", testBackForwardListLimitAndCache);
BackForwardListTest::add("WebKitWebView", "session-state", testWebKitWebViewSessionState);
BackForwardListTest::add("WebKitWebView", "session-state-with-form-data", testWebKitWebViewSessionStateWithFormData);
+ BackForwardListTest::add("WebKitWebView", "navigation-after-session-restore", testWebKitWebViewNavigationAfterSessionRestore);
}
void afterAll()
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
