Changeset 261394 in webkit

Timestamp:

May 8, 2020 10:33:58 AM (4 years ago)

Author:

Chris Dumez

Message:

REGRESSION(r259209) Webview's pending URL is null after restoring session state
​https://bugs.webkit.org/show_bug.cgi?id=211626
<rdar://problem/62992262>

Reviewed by Alex Christensen.

Source/WebKit:

The issue was that WebPageProxy::goToBackForwardItem() would behave differently whether
the page has a running process or not. In particular, when the page did not have a
running process, goToBackForwardItem() would return early and call launchProcessWithItem()
instead. Unlike goToBackForwardItem(), launchProcessWithItem() would fail to set the
pending API request.

To address the issue, I am getting rid of launchProcessWithItem() and merging its logic
into goToBackForwardItem() instead. Both methods shared a lot of code anyway and having
2 separate code paths that may diverge is error prone.

Change is covered by new API test.

• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::goToBackForwardItem):
(WebKit::WebPageProxy::launchProcessWithItem): Deleted.

• UIProcess/WebPageProxy.h:

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp:

(TestWebKitAPI::TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (2 diffs)
• Source/WebKit/UIProcess/WebPageProxy.h (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-05-08  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION(r259209) Webview's pending URL is null after restoring session state
+        https://bugs.webkit.org/show_bug.cgi?id=211626
+        <rdar://problem/62992262>
+
+        Reviewed by Alex Christensen.
+
+        The issue was that WebPageProxy::goToBackForwardItem() would behave differently whether
+        the page has a running process or not. In particular, when the page did not have a
+        running process, goToBackForwardItem() would return early and call launchProcessWithItem()
+        instead. Unlike goToBackForwardItem(), launchProcessWithItem() would fail to set the
+        pending API request.
+
+        To address the issue, I am getting rid of launchProcessWithItem() and merging its logic
+        into goToBackForwardItem() instead. Both methods shared a lot of code anyway and having
+        2 separate code paths that may diverge is error prone.
+
+        Change is covered by new API test.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::goToBackForwardItem):
+        (WebKit::WebPageProxy::launchProcessWithItem): Deleted.
+        * UIProcess/WebPageProxy.h:
+
 2020-05-08  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -1025 +1025 @@
 }
 
-RefPtr<API::Navigation> WebPageProxy::launchProcessWithItem(WebBackForwardListItem& item)
-{
-    RELEASE_LOG_IF_ALLOWED(Loading, "launchProcessWithItem:");
-
-    if (m_isClosed) {
-        RELEASE_LOG_IF_ALLOWED(Loading, "launchProcessWithItem: page is closed");
-        return nullptr;
-    }
-
-    ASSERT(!hasRunningProcess());
-    launchProcess(RegistrableDomain { URL(URL(), item.url()) }, ProcessLaunchReason::InitialProcess);
-
-    if (&item != m_backForwardList->currentItem())
-        m_backForwardList->goToItem(item);
-
-    auto navigation = m_navigationState->createBackForwardNavigation(item, m_backForwardList->currentItem(), FrameLoadType::IndexedBackForward);
-
-    send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), item.itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No, WTF::nullopt));
-    m_process->startResponsivenessTimer();
-
-    return navigation;
-}
-
 void WebPageProxy::setDrawingArea(std::unique_ptr<DrawingAreaProxy>&& drawingArea)
 {
@@ -1655 +1632 @@
     LOG(Loading, "WebPageProxy %p goToBackForwardItem to item URL %s", this, item.url().utf8().data());
 
-    if (!hasRunningProcess())
-        return launchProcessWithItem(item);
+    if (m_isClosed) {
+        RELEASE_LOG_IF_ALLOWED(Loading, "goToBackForwardItem: page is closed");
+        return nullptr;
+    }
+
+    if (!hasRunningProcess()) {
+        launchProcess(RegistrableDomain { URL(URL(), item.url()) }, ProcessLaunchReason::InitialProcess);
+
+        if (&item != m_backForwardList->currentItem())
+            m_backForwardList->goToItem(item);
+    }
 
     RefPtr<API::Navigation> navigation;

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1948 +1948 @@
 
     RefPtr<API::Navigation> launchProcessForReload();
-    RefPtr<API::Navigation> launchProcessWithItem(WebBackForwardListItem&);
 
     void requestNotificationPermission(uint64_t notificationID, const String& originString);

trunk/Tools/ChangeLog

@@ +1 @@
+2020-05-08  Chris Dumez  <cdumez@apple.com>
+
+        REGRESSION(r259209) Webview's pending URL is null after restoring session state
+        https://bugs.webkit.org/show_bug.cgi?id=211626
+        <rdar://problem/62992262>
+
+        Reviewed by Alex Christensen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp:
+        (TestWebKitAPI::TEST):
+
 2020-05-08  Alex Christensen  <achristensen@webkit.org>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit/RestoreSessionState.cpp

@@ -138 +138 @@
     WKPageRestoreFromSessionState(webView.page(), sessionState.get());
 }
+
+TEST(WebKit, PendingURLAfterRestoreSessionState)
+{
+    auto context = adoptWK(WKContextCreateWithConfiguration(nullptr));
+    PlatformWebView webView(context.get());
+    setPageLoaderClient(webView.page());
+    auto data = createSessionStateData(context.get());
+    EXPECT_NOT_NULL(data);
+    auto sessionState = adoptWK(WKSessionStateCreateFromData(data.get()));
+    WKPageRestoreFromSessionState(webView.page(), sessionState.get());
+    auto pendingURL = adoptWK(WKPageCopyPendingAPIRequestURL(webView.page()));
+    EXPECT_NOT_NULL(pendingURL);
+    if (!pendingURL)
+        return;
+
+    auto expectedURL = adoptWK(Util::createURLForResource("simple-form", "html"));
+    EXPECT_WK_STREQ(adoptWK(WKURLCopyString(expectedURL.get())).get(), adoptWK(WKURLCopyString(pendingURL.get())).get());
+}
     
 } // namespace TestWebKitAPI