Attachment 349404 Details for Bug 189482 – Patch

[patch] Patch

bug-189482-20180911100043.patch (text/plain), 5.47 KB, created by Chris Dumez on 2018-09-11 10:00:44 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-09-11 10:00:44 PDT

Size: 5.47 KB

patch

obsolete

>Subversion Revision: 235896
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e60abee4189f1442afc52c598099305a20175ed1..ead0af74e10221ed6de78e7d3f649789b111cf29 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1 +1,31 @@
>+2018-09-11  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(PSON): "Swipe back" snapshot is missing when navigating back cross-process
>+        https://bugs.webkit.org/show_bug.cgi?id=189482
>+        <rdar://problem/44317222>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The issue was that when we swap process on navigation, we:
>+        1. Call processDidTerminate() which calls resetState() and clears the displayed content
>+        2. Reattach the WebPageProxy to a new WebProcess
>+        3. Trigger the navigation
>+        4. Take the navigation snapshot
>+
>+        When the navigation snapshot occurs at step 4, the view is already blank in the case of
>+        process swap. To avoid the issue, we take the navigation snapshot earlier, before calling
>+        processDidTerminate() and we suppress the next navigation snapshot as it would be blank.
>+
>+        At some point, we will likely want to display the previous' WebProcess content longer,
>+        until the new WebProcess has something interesting to display. Once this is implemented,
>+        we'll likely be able to drop the early snapshotting logic.
>+
>+        * UIProcess/WebPageProxy.cpp:
>+        (WebKit::WebPageProxy::recordAutomaticNavigationSnapshot):
>+        (WebKit::WebPageProxy::continueNavigationInNewProcess):
>+        (WebKit::WebPageProxy::restoreFromSessionState):
>+        (WebKit::WebPageProxy::didCommitLoadForFrame):
>+        * UIProcess/WebPageProxy.h:
>+        (WebKit::WebPageProxy::suppressNextAutomaticNavigationSnapshot):
>+
> == Rolled over to ChangeLog-2018-09-11 ==
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
>index 01b1bbbbeedf7e435a9e78581bf04367c9ca8ace..b27b201e8aa75890844476cedc96c6aac61d0d82 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
>+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
>@@ -1196,7 +1196,7 @@ RefPtr<API::Navigation> WebPageProxy::reload(OptionSet<WebCore::ReloadOption> op
> 
> void WebPageProxy::recordAutomaticNavigationSnapshot()
> {
>-    if (m_suppressAutomaticNavigationSnapshotting)
>+    if (m_shouldSuppressNextAutomaticNavigationSnapshot)
>         return;
> 
>     if (WebBackForwardListItem* item = m_backForwardList->currentItem())
>@@ -2455,6 +2455,12 @@ void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, R
> {
>     LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString());
> 
>+    // Because we're about to tear down the drawing area and swap to a new process, this is our last opportunity to snapshot what's currently displayed before
>+    // we navigate. Do the navigation snapshot now and suppress the next one since the view will be blank then.
>+    // FIXME: We should be able to drop this logic if we kept displaying the previous content a while longer when we swap process on navigation.
>+    recordAutomaticNavigationSnapshot();
>+    suppressNextAutomaticNavigationSnapshot();
>+
>     Ref<WebProcessProxy> previousProcess = m_process.copyRef();
>     std::optional<uint64_t> navigatedFrameIdentifierInPreviousProcess;
>     if (m_mainFrame)
>@@ -2629,7 +2635,7 @@ RefPtr<API::Navigation> WebPageProxy::restoreFromSessionState(SessionState sessi
> 
>         // The back / forward list was restored from a sessionState so we don't want to snapshot the current
>         // page when navigating away. Suppress navigation snapshotting until the next load has committed
>-        m_suppressAutomaticNavigationSnapshotting = true;
>+        suppressNextAutomaticNavigationSnapshot();
>     }
> 
>     // FIXME: Navigating should be separate from state restoration.
>@@ -3625,7 +3631,7 @@ void WebPageProxy::didCommitLoadForFrame(uint64_t frameID, uint64_t navigationID
> 
>     if (frame->isMainFrame()) {
>         m_pageLoadState.didCommitLoad(transaction, webCertificateInfo, markPageInsecure);
>-        m_suppressAutomaticNavigationSnapshotting = false;
>+        m_shouldSuppressNextAutomaticNavigationSnapshot = false;
>     } else if (markPageInsecure)
>         m_pageLoadState.didDisplayOrRunInsecureContent(transaction);
> 
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index 41933a149ccd5c9d3feb7e5831ba2408381b9bb8..a0c1a82a1282498120b4eed9e348293934cb5414 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -1145,6 +1145,7 @@ public:
>     bool shouldRecordNavigationSnapshots() const { return m_shouldRecordNavigationSnapshots; }
>     void setShouldRecordNavigationSnapshots(bool shouldRecordSnapshots) { m_shouldRecordNavigationSnapshots = shouldRecordSnapshots; }
>     void recordAutomaticNavigationSnapshot();
>+    void suppressNextAutomaticNavigationSnapshot() { m_shouldSuppressNextAutomaticNavigationSnapshot = true; }
>     void recordNavigationSnapshot(WebBackForwardListItem&);
>     void requestAssistedNodeInformation(Function<void(const AssistedNodeInformation&, CallbackBase::Error)>&&);
> 
>@@ -2158,7 +2159,7 @@ private:
>     bool m_waitingForDidUpdateActivityState { false };
> 
>     bool m_shouldScaleViewToFitDocument { false };
>-    bool m_suppressAutomaticNavigationSnapshotting { false };
>+    bool m_shouldSuppressNextAutomaticNavigationSnapshot { false };
> 
> #if PLATFORM(COCOA)
>     HashMap<String, String> m_temporaryPDFFiles;

Actions: View | Formatted Diff | Diff

Attachments on bug 189482: 349404