[webkit-changes] [253360] trunk/Source/WebKit

Tue, 10 Dec 2019 18:17:57 -0800

Title: [253360] trunk/Source/WebKit
Revision
253360
Author
[email protected]
Date
2019-12-10 18:16:19 -0800 (Tue, 10 Dec 2019)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (253359 => 253360)


--- trunk/Source/WebKit/ChangeLog	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/ChangeLog	2019-12-11 02:16:19 UTC (rev 253360)
@@ -1,3 +1,51 @@
+2019-12-10  Chris Dumez  <[email protected]>
+
+        [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  <[email protected]>
 
         [Win] Fix MSVC warning C4701: potentially uninitialized local variable 'x' used

Modified: trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp (253359 => 253360)


--- trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp	2019-12-11 02:16:19 UTC (rev 253360)
@@ -143,6 +143,9 @@
     parameters.isProcessSwap = true;
     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());
 }
 
 void ProvisionalPageProxy::loadData(API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, Optional<WebsitePoliciesData>&& websitePolicies)

Modified: trunk/Source/WebKit/UIProcess/ViewGestureController.cpp (253359 => 253360)


--- trunk/Source/WebKit/UIProcess/ViewGestureController.cpp	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/UIProcess/ViewGestureController.cpp	2019-12-11 02:16:19 UTC (rev 253360)
@@ -278,6 +278,9 @@
     if (event & ViewGestureController::SnapshotRemovalTracker::ScrollPositionRestoration)
         description.append("ScrollPositionRestoration ");
 
+    if (event & ViewGestureController::SnapshotRemovalTracker::SwipeAnimationEnd)
+        description.append("SwipeAnimationEnd ");
+
     return description.toString();
 }
 
@@ -553,34 +556,20 @@
 void ViewGestureController::willEndSwipeGesture(WebBackForwardListItem& targetItem, bool cancelled)
 {
     m_webPageProxy.navigationGestureWillEnd(!cancelled, targetItem);
-}
 
-void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem, bool cancelled)
-{
-    ASSERT(m_activeGestureType == ViewGestureType::Swipe);
-    ASSERT(targetItem);
-
-#if PLATFORM(MAC)
-    m_swipeCancellationTracker = nullptr;
-#endif
-
-    if (cancelled) {
-        removeSwipeSnapshot();
-        m_webPageProxy.navigationGestureDidEnd(false, *targetItem);
+    if (cancelled)
         return;
-    }
 
     uint64_t renderTreeSize = 0;
-    if (ViewSnapshot* snapshot = targetItem->snapshot())
+    if (ViewSnapshot* snapshot = targetItem.snapshot())
         renderTreeSize = snapshot->renderTreeSize();
     auto renderTreeSizeThreshold = renderTreeSize * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
 
-    m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
-    m_webPageProxy.goToBackForwardItem(*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)) {
+    if (currentItem && currentItem->itemIsClone(targetItem)) {
         removeSwipeSnapshot();
         return;
     }
@@ -588,7 +577,8 @@
     SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
         | SnapshotRemovalTracker::MainFrameLoad
         | SnapshotRemovalTracker::SubresourceLoads
-        | SnapshotRemovalTracker::ScrollPositionRestoration;
+        | SnapshotRemovalTracker::ScrollPositionRestoration
+        | SnapshotRemovalTracker::SwipeAnimationEnd;
 
     if (renderTreeSizeThreshold) {
         desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
@@ -600,10 +590,30 @@
     // 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())
+    if (ViewSnapshot* snapshot = targetItem.snapshot())
         m_backgroundColorForCurrentSnapshot = snapshot->backgroundColor();
 }
 
+void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem, bool cancelled)
+{
+    ASSERT(m_activeGestureType == ViewGestureType::Swipe);
+    ASSERT(targetItem);
+
+#if PLATFORM(MAC)
+    m_swipeCancellationTracker = nullptr;
+#endif
+
+    if (cancelled) {
+        removeSwipeSnapshot();
+        m_webPageProxy.navigationGestureDidEnd(false, *targetItem);
+        return;
+    }
+
+    m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
+
+    m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::SwipeAnimationEnd);
+}
+
 void ViewGestureController::requestRenderTreeSizeNotificationIfNeeded()
 {
     if (!m_snapshotRemovalTracker.hasOutstandingEvent(SnapshotRemovalTracker::RenderTreeSizeThreshold))

Modified: trunk/Source/WebKit/UIProcess/ViewGestureController.h (253359 => 253360)


--- trunk/Source/WebKit/UIProcess/ViewGestureController.h	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/UIProcess/ViewGestureController.h	2019-12-11 02:16:19 UTC (rev 253360)
@@ -194,7 +194,8 @@
             RepaintAfterNavigation = 1 << 2,
             MainFrameLoad = 1 << 3,
             SubresourceLoads = 1 << 4,
-            ScrollPositionRestoration = 1 << 5
+            ScrollPositionRestoration = 1 << 5,
+            SwipeAnimationEnd = 1 << 6
         };
         typedef uint8_t Events;

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (253359 => 253360)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-12-11 02:16:19 UTC (rev 253360)
@@ -3122,6 +3122,9 @@
         shouldDelayClosingUntilEnteringAcceleratedCompositingMode = ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::Yes;
 #endif
 
+    if (m_isLayerTreeFrozenDueToSwipeAnimation)
+        send(Messages::WebPage::UnfreezeLayerTreeDueToSwipeAnimation());
+
     processDidTerminate(ProcessTerminationReason::NavigationSwap);
 
     m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
@@ -8417,6 +8420,10 @@
 void WebPageProxy::navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem& item)
 {
     PageClientProtector protector(pageClient());
+    if (willNavigate) {
+        m_isLayerTreeFrozenDueToSwipeAnimation = true;
+        send(Messages::WebPage::FreezeLayerTreeDueToSwipeAnimation());
+    }
 
     pageClient().navigationGestureWillEnd(willNavigate, item);
 
@@ -8430,6 +8437,11 @@
     pageClient().navigationGestureDidEnd(willNavigate, item);
 
     m_navigationClient->didEndNavigationGesture(*this, willNavigate, item);
+
+    if (m_isLayerTreeFrozenDueToSwipeAnimation) {
+        m_isLayerTreeFrozenDueToSwipeAnimation = false;
+        send(Messages::WebPage::UnfreezeLayerTreeDueToSwipeAnimation());
+    }
 }
 
 void WebPageProxy::navigationGestureDidEnd()

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (253359 => 253360)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2019-12-11 02:16:19 UTC (rev 253360)
@@ -1290,6 +1290,8 @@
     void endColorPicker();
 #endif
 
+    bool isLayerTreeFrozenDueToSwipeAnimation() const { return m_isLayerTreeFrozenDueToSwipeAnimation; }
+
     WebCore::IntSize minimumSizeForAutoLayout() const { return m_minimumSizeForAutoLayout; }
     void setMinimumSizeForAutoLayout(const WebCore::IntSize&);
 
@@ -2639,6 +2641,7 @@
     COMPtr<ID3D11Device1> m_device;
 #endif
     bool m_isQuotaIncreaseDenied { false };
+    bool m_isLayerTreeFrozenDueToSwipeAnimation { false };
     
     String m_overriddenMediaType;

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (253359 => 253360)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2019-12-11 02:16:19 UTC (rev 253360)
@@ -4665,6 +4665,16 @@
 }
 #endif
 
+void WebPage::freezeLayerTreeDueToSwipeAnimation()
+{
+    freezeLayerTree(LayerTreeFreezeReason::SwipeAnimation);
+}
+
+void WebPage::unfreezeLayerTreeDueToSwipeAnimation()
+{
+    unfreezeLayerTree(LayerTreeFreezeReason::SwipeAnimation);
+}
+
 void WebPage::beginPrinting(FrameIdentifier frameID, const PrintInfo& printInfo)
 {
     WebFrame* frame = WebProcess::singleton().webFrame(frameID);

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (253359 => 253360)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2019-12-11 02:16:19 UTC (rev 253360)
@@ -729,6 +729,7 @@
         PageSuspended           = 1 << 3,
         Printing                = 1 << 4,
         ProcessSwap             = 1 << 5,
+        SwipeAnimation          = 1 << 6,
     };
     void freezeLayerTree(LayerTreeFreezeReason);
     void unfreezeLayerTree(LayerTreeFreezeReason);
@@ -736,6 +737,9 @@
     void markLayersVolatile(Function<void(bool)>&& completionHandler = { });
     void cancelMarkLayersVolatile();
 
+    void freezeLayerTreeDueToSwipeAnimation();
+    void unfreezeLayerTreeDueToSwipeAnimation();
+
     NotificationPermissionRequestManager* notificationPermissionRequestManager();
 
     void pageDidScroll();

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (253359 => 253360)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in	2019-12-11 01:52:02 UTC (rev 253359)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in	2019-12-11 02:16:19 UTC (rev 253360)
@@ -380,6 +380,9 @@
     # Notification
     DidReceiveNotificationPermissionDecision(uint64_t notificationID, bool allowed)
 
+    FreezeLayerTreeDueToSwipeAnimation()
+    UnfreezeLayerTreeDueToSwipeAnimation()
+
     # Printing.
     BeginPrinting(WebCore::FrameIdentifier frameID, struct WebKit::PrintInfo printInfo)
     EndPrinting()

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to