Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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:


Canonical link: https://commits.webkit.org/218300@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@253360 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Dec 11, 2019
1 parent ff036bd commit c255774
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 21 deletions.
48 changes: 48 additions & 0 deletions Source/WebKit/ChangeLog
@@ -1,3 +1,51 @@
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>

[Win] Fix MSVC warning C4701: potentially uninitialized local variable 'x' used
Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/UIProcess/ProvisionalPageProxy.cpp
Expand Up @@ -143,6 +143,9 @@ void ProvisionalPageProxy::initializeWebPage()
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)
Expand Down
50 changes: 30 additions & 20 deletions Source/WebKit/UIProcess/ViewGestureController.cpp
Expand Up @@ -278,6 +278,9 @@ String ViewGestureController::SnapshotRemovalTracker::eventsDescription(Events e
if (event & ViewGestureController::SnapshotRemovalTracker::ScrollPositionRestoration)
description.append("ScrollPositionRestoration ");

if (event & ViewGestureController::SnapshotRemovalTracker::SwipeAnimationEnd)
description.append("SwipeAnimationEnd ");

return description.toString();
}

Expand Down Expand Up @@ -553,42 +556,29 @@ void ViewGestureController::forceRepaintIfNeeded()
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;
}

SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
| SnapshotRemovalTracker::MainFrameLoad
| SnapshotRemovalTracker::SubresourceLoads
| SnapshotRemovalTracker::ScrollPositionRestoration;
| SnapshotRemovalTracker::ScrollPositionRestoration
| SnapshotRemovalTracker::SwipeAnimationEnd;

if (renderTreeSizeThreshold) {
desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
Expand All @@ -600,10 +590,30 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
// 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))
Expand Down
3 changes: 2 additions & 1 deletion Source/WebKit/UIProcess/ViewGestureController.h
Expand Up @@ -194,7 +194,8 @@ class ViewGestureController : private IPC::MessageReceiver {
RepaintAfterNavigation = 1 << 2,
MainFrameLoad = 1 << 3,
SubresourceLoads = 1 << 4,
ScrollPositionRestoration = 1 << 5
ScrollPositionRestoration = 1 << 5,
SwipeAnimationEnd = 1 << 6
};
typedef uint8_t Events;

Expand Down
12 changes: 12 additions & 0 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Expand Up @@ -3122,6 +3122,9 @@ void WebPageProxy::commitProvisionalPage(FrameIdentifier frameID, uint64_t navig
shouldDelayClosingUntilEnteringAcceleratedCompositingMode = ShouldDelayClosingUntilEnteringAcceleratedCompositingMode::Yes;
#endif

if (m_isLayerTreeFrozenDueToSwipeAnimation)
send(Messages::WebPage::UnfreezeLayerTreeDueToSwipeAnimation());

processDidTerminate(ProcessTerminationReason::NavigationSwap);

m_process->removeMessageReceiver(Messages::WebPageProxy::messageReceiverName(), m_webPageID);
Expand Down Expand Up @@ -8417,6 +8420,10 @@ void WebPageProxy::navigationGestureDidBegin()
void WebPageProxy::navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem& item)
{
PageClientProtector protector(pageClient());
if (willNavigate) {
m_isLayerTreeFrozenDueToSwipeAnimation = true;
send(Messages::WebPage::FreezeLayerTreeDueToSwipeAnimation());
}

pageClient().navigationGestureWillEnd(willNavigate, item);

Expand All @@ -8430,6 +8437,11 @@ void WebPageProxy::navigationGestureDidEnd(bool willNavigate, WebBackForwardList
pageClient().navigationGestureDidEnd(willNavigate, item);

m_navigationClient->didEndNavigationGesture(*this, willNavigate, item);

if (m_isLayerTreeFrozenDueToSwipeAnimation) {
m_isLayerTreeFrozenDueToSwipeAnimation = false;
send(Messages::WebPage::UnfreezeLayerTreeDueToSwipeAnimation());
}
}

void WebPageProxy::navigationGestureDidEnd()
Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/UIProcess/WebPageProxy.h
Expand Up @@ -1290,6 +1290,8 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
void endColorPicker();
#endif

bool isLayerTreeFrozenDueToSwipeAnimation() const { return m_isLayerTreeFrozenDueToSwipeAnimation; }

WebCore::IntSize minimumSizeForAutoLayout() const { return m_minimumSizeForAutoLayout; }
void setMinimumSizeForAutoLayout(const WebCore::IntSize&);

Expand Down Expand Up @@ -2639,6 +2641,7 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
COMPtr<ID3D11Device1> m_device;
#endif
bool m_isQuotaIncreaseDenied { false };
bool m_isLayerTreeFrozenDueToSwipeAnimation { false };

String m_overriddenMediaType;

Expand Down
10 changes: 10 additions & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Expand Up @@ -4665,6 +4665,16 @@ void WebPage::effectiveAppearanceDidChange(bool useDarkAppearance, bool useEleva
}
#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);
Expand Down
4 changes: 4 additions & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.h
Expand Up @@ -729,13 +729,17 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
PageSuspended = 1 << 3,
Printing = 1 << 4,
ProcessSwap = 1 << 5,
SwipeAnimation = 1 << 6,
};
void freezeLayerTree(LayerTreeFreezeReason);
void unfreezeLayerTree(LayerTreeFreezeReason);

void markLayersVolatile(Function<void(bool)>&& completionHandler = { });
void cancelMarkLayersVolatile();

void freezeLayerTreeDueToSwipeAnimation();
void unfreezeLayerTreeDueToSwipeAnimation();

NotificationPermissionRequestManager* notificationPermissionRequestManager();

void pageDidScroll();
Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Expand Up @@ -380,6 +380,9 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
# Notification
DidReceiveNotificationPermissionDecision(uint64_t notificationID, bool allowed)

FreezeLayerTreeDueToSwipeAnimation()
UnfreezeLayerTreeDueToSwipeAnimation()

# Printing.
BeginPrinting(WebCore::FrameIdentifier frameID, struct WebKit::PrintInfo printInfo)
EndPrinting()
Expand Down

0 comments on commit c255774

Please sign in to comment.