Changeset 253360 in webkit

Timestamp:

Dec 10, 2019, 6:16:19 PM (7 years ago)

Author:

Chris Dumez

Message:

[macOS] Issue load sooner on swipe back/forward navigation
​https://bugs.webkit.org/show_bug.cgi?id=205087

Reviewed by Tim Horton.

Issue load sooner on swipe back/forward navigation on macOS. We were waiting until the end of
the swipe animation to issue the load. We now issue the load as soon as the user lifts the finger
off the screen and thus commits to navigating. This results in improved perceived performance
when swiping back/forward to navigate.

This patch does not take care of iOS because the ViewGestureController logic is different on that
platform. There is no reason we shouldn't be able to do the same optimization on iOS too though.

To achieve the behavior change on macOS, the following was done:

• Issue the load in ViewGestureController::willEndSwipeGesture() instead of ViewGestureController::endSwipeGesture().
• Add a new SnapshotRemovalTracker::Event::SwipeAnimationEnd event and wait for this event before taking away the snapshot. This makes sure we do not take away the swipe snapshot until the swipe animation is actually over (now that we start the navigation during the animation, instead of after).
• To make sure that layer being swiped away stays the same until the end of the animation, I added a new SwipeAnimation reason for freezing the layer tree. At the beginning of the animation, the UIProcess sends a FreezeLayerTreeDueToSwipeAnimation IPC to the WebProcess to add this layer tree freeze reason. At the end of the animation, the UIProcess sends the UnfreezeLayerTreeDueToSwipeAnimation IPC to remove this freeze reason. Without this change, the layer being swiped away would sometimes start showing the destination site being loaded before the end of the animation. On cross-process navigation, not freezing the layer tree would cause the swipe animation to get interrupted when we have something to paint in the new process before the end of the swipe animation.

• UIProcess/ProvisionalPageProxy.cpp:

(WebKit::ProvisionalPageProxy::initializeWebPage):

• UIProcess/ViewGestureController.cpp:

(WebKit::ViewGestureController::SnapshotRemovalTracker::eventsDescription):
(WebKit::ViewGestureController::willEndSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):

• UIProcess/ViewGestureController.h:
• UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::commitProvisionalPage):

• UIProcess/WebPageProxy.h:

(WebKit::WebPageProxy::isLayerTreeFrozenDueToSwipeAnimation const):

• WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::freezeLayerTreeDueToSwipeAnimation):
(WebKit::WebPage::unfreezeLayerTreeDueToSwipeAnimation):

• WebProcess/WebPage/WebPage.h:
• WebProcess/WebPage/WebPage.messages.in:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/ProvisionalPageProxy.cpp (modified) (1 diff)
• UIProcess/ViewGestureController.cpp (modified) (3 diffs)
• UIProcess/ViewGestureController.h (modified) (1 diff)
• UIProcess/WebPageProxy.cpp (modified) (3 diffs)
• UIProcess/WebPageProxy.h (modified) (2 diffs)
• WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• WebProcess/WebPage/WebPage.h (modified) (2 diffs)
• WebProcess/WebPage/WebPage.messages.in (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-12-10  Chris Dumez  <cdumez@apple.com>
+
+        [macOS] Issue load sooner on swipe back/forward navigation
+        https://bugs.webkit.org/show_bug.cgi?id=205087
+
+        Reviewed by Tim Horton.
+
+        Issue load sooner on swipe back/forward navigation on macOS. We were waiting until the end of
+        the swipe animation to issue the load. We now issue the load as soon as the user lifts the finger
+        off the screen and thus commits to navigating. This results in improved perceived performance
+        when swiping back/forward to navigate.
+
+        This patch does not take care of iOS because the ViewGestureController logic is different on that
+        platform. There is no reason we shouldn't be able to do the same optimization on iOS too though.
+
+        To achieve the behavior change on macOS, the following was done:
+        - Issue the load in ViewGestureController::willEndSwipeGesture() instead of
+          ViewGestureController::endSwipeGesture().
+        - Add a new SnapshotRemovalTracker::Event::SwipeAnimationEnd event and wait for this event before
+          taking away the snapshot. This makes sure we do not take away the swipe snapshot until the swipe
+          animation is actually over (now that we start the navigation during the animation, instead of
+          after).
+        - To make sure that layer being swiped away stays the same until the end of the animation, I added
+          a new SwipeAnimation reason for freezing the layer tree. At the beginning of the animation, the
+          UIProcess sends a FreezeLayerTreeDueToSwipeAnimation IPC to the WebProcess to add this layer tree
+          freeze reason. At the end of the animation, the UIProcess sends the UnfreezeLayerTreeDueToSwipeAnimation
+          IPC to remove this freeze reason. Without this change, the layer being swiped away would sometimes
+          start showing the destination site being loaded before the end of the animation. On cross-process
+          navigation, not freezing the layer tree would cause the swipe animation to get interrupted when
+          we have something to paint in the new process before the end of the swipe animation.
+
+        * UIProcess/ProvisionalPageProxy.cpp:
+        (WebKit::ProvisionalPageProxy::initializeWebPage):
+        * UIProcess/ViewGestureController.cpp:
+        (WebKit::ViewGestureController::SnapshotRemovalTracker::eventsDescription):
+        (WebKit::ViewGestureController::willEndSwipeGesture):
+        (WebKit::ViewGestureController::endSwipeGesture):
+        * UIProcess/ViewGestureController.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::commitProvisionalPage):
+        * UIProcess/WebPageProxy.h:
+        (WebKit::WebPageProxy::isLayerTreeFrozenDueToSwipeAnimation const):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::freezeLayerTreeDueToSwipeAnimation):
+        (WebKit::WebPage::unfreezeLayerTreeDueToSwipeAnimation):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
 2019-12-10  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp

@@ -144 +144 @@
     m_process->send(Messages::WebProcess::CreateWebPage(m_webPageID, parameters), 0);
     m_process->addVisitedLinkStoreUser(m_page.visitedLinkStore(), m_page.identifier());
+
+    if (m_page.isLayerTreeFrozenDueToSwipeAnimation())
+        send(Messages::WebPage::FreezeLayerTreeDueToSwipeAnimation());
 }
 

trunk/Source/WebKit/UIProcess/ViewGestureController.cpp

@@ -279 +279 @@
         description.append("ScrollPositionRestoration ");
 
+    if (event & ViewGestureController::SnapshotRemovalTracker::SwipeAnimationEnd)
+        description.append("SwipeAnimationEnd ");
+
     return description.toString();
 }
@@ -554 +557 @@
 {
     m_webPageProxy.navigationGestureWillEnd(!cancelled, targetItem);
+
+    if (cancelled)
+        return;
+
+    uint64_t renderTreeSize = 0;
+    if (ViewSnapshot* snapshot = targetItem.snapshot())
+        renderTreeSize = snapshot->renderTreeSize();
+    auto renderTreeSizeThreshold = renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
+
+    m_webPageProxy.goToBackForwardItem(targetItem);
+
+    auto* currentItem = m_webPageProxy.backForwardList().currentItem();
+    // The main frame will not be navigated so hide the snapshot right away.
+    if (currentItem && currentItem->itemIsClone(targetItem)) {
+        removeSwipeSnapshot();
+        return;
+    }
+
+    SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
+        | SnapshotRemovalTracker::MainFrameLoad
+        | SnapshotRemovalTracker::SubresourceLoads
+        | SnapshotRemovalTracker::ScrollPositionRestoration
+        | SnapshotRemovalTracker::SwipeAnimationEnd;
+
+    if (renderTreeSizeThreshold) {
+        desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
+        m_snapshotRemovalTracker.setRenderTreeSizeThreshold(renderTreeSizeThreshold);
+    }
+
+    m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
+
+    // FIXME: Like on iOS, we should ensure that even if one of the timeouts fires,
+    // we never show the old page content, instead showing the snapshot background color.
+
+    if (ViewSnapshot* snapshot = targetItem.snapshot())
+        m_backgroundColorForCurrentSnapshot = snapshot->backgroundColor();
 }
 
@@ -571 +610 @@
     }
 
-    uint64_t renderTreeSize = 0;
-    if (ViewSnapshot* snapshot = targetItem->snapshot())
-        renderTreeSize = snapshot->renderTreeSize();
-    auto renderTreeSizeThreshold = renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
-
     m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
-    m_webPageProxy.goToBackForwardItem(*targetItem);
-
-    auto* currentItem = m_webPageProxy.backForwardList().currentItem();
-    // The main frame will not be navigated so hide the snapshot right away.
-    if (currentItem && currentItem->itemIsClone(*targetItem)) {
-        removeSwipeSnapshot();
-        return;
-    }
-
-    SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
-        | SnapshotRemovalTracker::MainFrameLoad
-        | SnapshotRemovalTracker::SubresourceLoads
-        | SnapshotRemovalTracker::ScrollPositionRestoration;
-
-    if (renderTreeSizeThreshold) {
-        desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
-        m_snapshotRemovalTracker.setRenderTreeSizeThreshold(renderTreeSizeThreshold);
-    }
-
-    m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
-
-    // FIXME: Like on iOS, we should ensure that even if one of the timeouts fires,
-    // we never show the old page content, instead showing the snapshot background color.
-
-    if (ViewSnapshot* snapshot = targetItem->snapshot())
-        m_backgroundColorForCurrentSnapshot = snapshot->backgroundColor();
+
+    m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::SwipeAnimationEnd);
 }
 

trunk/Source/WebKit/UIProcess/ViewGestureController.h

@@ -195 +195 @@
             MainFrameLoad = 1 << 3,
             SubresourceLoads = 1 << 4,
-            ScrollPositionRestoration = 1 << 5
+            ScrollPositionRestoration = 1 << 5,
+            SwipeAnimationEnd = 1 << 6
         };
         typedef uint8_t Events;

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -3123 +3123 @@
 #endif
 
+    if (m_isLayerTreeFrozenDueToSwipeAnimation)
+        send(Messages::WebPage::UnfreezeLayerTreeDueToSwipeAnimation());
+
     processDidTerminate(ProcessTerminationReason::NavigationSwap);
 
@@ -8418 +8421 @@
 {
     PageClientProtector protector(pageClient());
+    if (willNavigate) {
+        m_isLayerTreeFrozenDueToSwipeAnimation = true;
+        send(Messages::WebPage::FreezeLayerTreeDueToSwipeAnimation());
+    }
 
     pageClient().navigationGestureWillEnd(willNavigate, item);
@@ -8431 +8438 @@
 
     m_navigationClient->didEndNavigationGesture(*this, willNavigate, item);
+
+    if (m_isLayerTreeFrozenDueToSwipeAnimation) {
+        m_isLayerTreeFrozenDueToSwipeAnimation = false;
+        send(Messages::WebPage::UnfreezeLayerTreeDueToSwipeAnimation());
+    }
 }
 

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1291 +1291 @@
 #endif
 
+    bool isLayerTreeFrozenDueToSwipeAnimation() const { return m_isLayerTreeFrozenDueToSwipeAnimation; }
+
     WebCore::IntSize minimumSizeForAutoLayout() const { return m_minimumSizeForAutoLayout; }
     void setMinimumSizeForAutoLayout(const WebCore::IntSize&);
@@ -2640 +2642 @@
 #endif
     bool m_isQuotaIncreaseDenied { false };
+    bool m_isLayerTreeFrozenDueToSwipeAnimation { false };
     
     String m_overriddenMediaType;

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -4666 +4666 @@
 #endif
 
+void WebPage::freezeLayerTreeDueToSwipeAnimation()
+{
+    freezeLayerTree(LayerTreeFreezeReason::SwipeAnimation);
+}
+
+void WebPage::unfreezeLayerTreeDueToSwipeAnimation()
+{
+    unfreezeLayerTree(LayerTreeFreezeReason::SwipeAnimation);
+}
+
 void WebPage::beginPrinting(FrameIdentifier frameID, const PrintInfo& printInfo)
 {

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -730 +730 @@
         Printing                = 1 << 4,
         ProcessSwap             = 1 << 5,
+        SwipeAnimation          = 1 << 6,
     };
     void freezeLayerTree(LayerTreeFreezeReason);
@@ -736 +737 @@
     void markLayersVolatile(Function<void(bool)>&& completionHandler = { });
     void cancelMarkLayersVolatile();
+
+    void freezeLayerTreeDueToSwipeAnimation();
+    void unfreezeLayerTreeDueToSwipeAnimation();
 
     NotificationPermissionRequestManager* notificationPermissionRequestManager();

trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

@@ -381 +381 @@
     DidReceiveNotificationPermissionDecision(uint64_t notificationID, bool allowed)
 
+    FreezeLayerTreeDueToSwipeAnimation()
+    UnfreezeLayerTreeDueToSwipeAnimation()
+
     # Printing.
     BeginPrinting(WebCore::FrameIdentifier frameID, struct WebKit::PrintInfo printInfo)