Remove swipe snapshot before main document load if scroll position is already restored
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Dec 2015 19:57:07 +0000 (19:57 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 1 Dec 2015 19:57:07 +0000 (19:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=151224

Reviewed by Darin Adler.

* UIProcess/Cocoa/WebViewImpl.h:
* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::WebViewImpl::didRestoreScrollPosition):
* UIProcess/PageClient.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didRestoreScrollPosition):
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::didRestoreScrollPosition):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::didRestoreScrollPosition):
* WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didRestoreScrollPosition):
* WebProcess/WebPage/WebPage.h:
Plumb didRestoreScrollPosition through to ViewGestureController (yikes!).

* UIProcess/ViewGestureController.cpp:
(WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
Cancel waiting for any more loads if we get to firstVisuallyNonEmptyLayout.

(WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
Cancel waiting for scroll position restoration if we make it to main frame load,
because there is a chance we won't be able to restore the old scroll position, and
by main frame load time we've tried as hard as we're going to to restore it.

* UIProcess/mac/ViewGestureControllerMac.mm:
(WebKit::ViewGestureController::endSwipeGesture):
Make some legacy-style-only code clearer that it's legacy-style-only.
Wait for scroll position restoration.

* loader/FrameLoaderClient.h:
* loader/HistoryController.cpp:
(WebCore::HistoryController::restoreScrollPositionAndViewState):
Each time we try to restore the scroll position, see if the requested
scroll position is something we can scroll to by going through ScrollView's
scroll position constraint logic. If we can scroll there, tell our client
(and eventually ViewGestureController) that we successfully restored the
scroll position!

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@192898 268f45cc-cd09-0410-ab3c-d52691b4dbfc

23 files changed:
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoaderClient.h
Source/WebCore/loader/HistoryController.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.h
Source/WebKit2/UIProcess/Cocoa/WebViewImpl.h
Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm
Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.h
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/ViewGestureController.cpp
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/WebPageProxy.messages.in
Source/WebKit2/UIProcess/efl/WebViewEfl.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm
Source/WebKit2/UIProcess/mac/ViewGestureControllerMac.mm
Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.h
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h

index f9bdcfe..c5cf5c8 100644 (file)
@@ -1,3 +1,19 @@
+2015-12-01  Tim Horton  <timothy_horton@apple.com>
+
+        Remove swipe snapshot before main document load if scroll position is already restored
+        https://bugs.webkit.org/show_bug.cgi?id=151224
+
+        Reviewed by Darin Adler.
+
+        * loader/FrameLoaderClient.h:
+        * loader/HistoryController.cpp:
+        (WebCore::HistoryController::restoreScrollPositionAndViewState):
+        Each time we try to restore the scroll position, see if the requested
+        scroll position is something we can scroll to by going through ScrollView's
+        scroll position constraint logic. If we can scroll there, tell our client
+        (and eventually ViewGestureController) that we successfully restored the
+        scroll position!
+
 2015-12-01  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [iOS] Adjacent emoji overlap each other
index 19b8871..77395ed 100644 (file)
@@ -349,6 +349,8 @@ namespace WebCore {
 #endif
 
         virtual void prefetchDNS(const String&) = 0;
+
+        virtual void didRestoreScrollPosition() { }
     };
 
 } // namespace WebCore
index 7e0caba..966efcf 100644 (file)
@@ -153,10 +153,20 @@ void HistoryController::restoreScrollPositionAndViewState()
     // Don't restore scroll point on iOS as FrameLoaderClient::restoreViewState() does that.
     if (view && !view->wasScrolledByUser()) {
         Page* page = m_frame.page();
+        auto desiredScrollPosition = m_currentItem->scrollPoint();
+
         if (page && m_frame.isMainFrame() && m_currentItem->pageScaleFactor())
-            page->setPageScaleFactor(m_currentItem->pageScaleFactor() * page->viewScaleFactor(), m_currentItem->scrollPoint());
+            page->setPageScaleFactor(m_currentItem->pageScaleFactor() * page->viewScaleFactor(), desiredScrollPosition);
         else
-            view->setScrollPosition(m_currentItem->scrollPoint());
+            view->setScrollPosition(desiredScrollPosition);
+
+        // If the scroll position doesn't have to be clamped, consider it successfully restored.
+        if (m_frame.isMainFrame()) {
+            auto adjustedDesiredScrollPosition = view->adjustScrollPositionWithinRange(desiredScrollPosition);
+            if (desiredScrollPosition == adjustedDesiredScrollPosition)
+                m_frame.loader().client().didRestoreScrollPosition();
+        }
+
     }
 #endif
 }
index ed21cf0..86bd90e 100644 (file)
@@ -1,3 +1,43 @@
+2015-12-01  Tim Horton  <timothy_horton@apple.com>
+
+        Remove swipe snapshot before main document load if scroll position is already restored
+        https://bugs.webkit.org/show_bug.cgi?id=151224
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/Cocoa/WebViewImpl.h:
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (WebKit::WebViewImpl::didRestoreScrollPosition):
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didRestoreScrollPosition):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/WebPageProxy.messages.in:
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::didRestoreScrollPosition):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::didRestoreScrollPosition):
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didRestoreScrollPosition):
+        * WebProcess/WebPage/WebPage.h:
+        Plumb didRestoreScrollPosition through to ViewGestureController (yikes!).
+
+        * UIProcess/ViewGestureController.cpp:
+        (WebKit::ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame):
+        Cancel waiting for any more loads if we get to firstVisuallyNonEmptyLayout.
+
+        (WebKit::ViewGestureController::didReachMainFrameLoadTerminalState):
+        Cancel waiting for scroll position restoration if we make it to main frame load,
+        because there is a chance we won't be able to restore the old scroll position, and
+        by main frame load time we've tried as hard as we're going to to restore it.
+
+        * UIProcess/mac/ViewGestureControllerMac.mm:
+        (WebKit::ViewGestureController::endSwipeGesture):
+        Make some legacy-style-only code clearer that it's legacy-style-only.
+        Wait for scroll position restoration.
+
 2015-12-01  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         REGRESSION(r192834): [GTK] Test /webkit2/WebKitWebView/editor-state/typing-attributes times out after r192834
index 61303d7..78fc688 100644 (file)
@@ -139,6 +139,8 @@ private:
     virtual void refView() override;
     virtual void derefView() override;
 
+    virtual void didRestoreScrollPosition() override { }
+
 #if ENABLE(VIDEO)
     virtual bool decicePolicyForInstallMissingMediaPluginsPermissionRequest(InstallMissingMediaPluginsPermissionRequest&) override;
 #endif
index 2a8975e..588f238 100644 (file)
@@ -413,6 +413,8 @@ public:
     void gestureEventWasNotHandledByWebCore(NSEvent *);
     void gestureEventWasNotHandledByWebCoreFromViewOnly(NSEvent *);
 
+    void didRestoreScrollPosition();
+
     void setTotalHeightOfBanners(CGFloat totalHeightOfBanners) { m_totalHeightOfBanners = totalHeightOfBanners; }
     CGFloat totalHeightOfBanners() const { return m_totalHeightOfBanners; }
 
index e2cdd9a..2969e72 100644 (file)
@@ -3183,6 +3183,12 @@ void WebViewImpl::gestureEventWasNotHandledByWebCoreFromViewOnly(NSEvent *event)
 #endif
 }
 
+void WebViewImpl::didRestoreScrollPosition()
+{
+    if (m_gestureController)
+        m_gestureController->didRestoreScrollPosition();
+}
+
 void WebViewImpl::doneWithKeyEvent(NSEvent *event, bool eventWasHandled)
 {
     if ([event type] != NSKeyDown)
index b529ad7..76ad677 100644 (file)
@@ -214,6 +214,8 @@ protected:
     virtual void didChangeBackgroundColor() override { }
     virtual void didFailLoadForMainFrame() override { }
 
+    virtual void didRestoreScrollPosition() override { }
+
     WebViewClient m_client;
     RefPtr<WebPageProxy> m_page;
     DefaultUndoController m_undoController;
index 73f182d..dcbc6d1 100644 (file)
@@ -355,6 +355,8 @@ public:
 #if USE(GSTREAMER)
     virtual bool decicePolicyForInstallMissingMediaPluginsPermissionRequest(InstallMissingMediaPluginsPermissionRequest&) = 0;
 #endif
+
+    virtual void didRestoreScrollPosition() = 0;
 };
 
 } // namespace WebKit
index 2ea2dd7..c5f56d8 100644 (file)
@@ -27,6 +27,7 @@
 #import "ViewGestureController.h"
 
 #import "Logging.h"
+#import "RemoteLayerTreeDrawingAreaProxy.h"
 #import "ViewGestureControllerMessages.h"
 #import "WebPageProxy.h"
 #import "WebProcessProxy.h"
@@ -86,12 +87,9 @@ void ViewGestureController::didFirstVisuallyNonEmptyLayoutForMainFrame()
     if (!m_snapshotRemovalTracker.eventOccurred(SnapshotRemovalTracker::VisuallyNonEmptyLayout))
         return;
 
+    m_snapshotRemovalTracker.cancelOutstandingEvent(SnapshotRemovalTracker::MainFrameLoad);
+    m_snapshotRemovalTracker.cancelOutstandingEvent(SnapshotRemovalTracker::SubresourceLoads);
     m_snapshotRemovalTracker.startWatchdog(swipeSnapshotRemovalWatchdogAfterFirstVisuallyNonEmptyLayoutDuration);
-
-    // FIXME: Ideally, this would be sufficient to cancel waiting for the main frame load,
-    // and would make snapshot removal faster, but we need didRestoreScrollPosition
-    // to be usable/accurate on all platforms before doing that, or we get a flash
-    // of the new page in the wrong position.
 }
 
 void ViewGestureController::didRepaintAfterNavigation()
@@ -119,6 +117,17 @@ void ViewGestureController::didReachMainFrameLoadTerminalState()
     // enough for us too.
     m_snapshotRemovalTracker.cancelOutstandingEvent(SnapshotRemovalTracker::VisuallyNonEmptyLayout);
 
+    // With Web-process scrolling, we check if the scroll position restoration succeeded by comparing the
+    // requested and actual scroll position. It's possible that we will never succeed in restoring
+    // the exact scroll position we wanted, in the case of a dynamic page, but we know that by
+    // main frame load time that we've gotten as close as we're going to get, so stop waiting.
+    // We don't want to do this with UI-side scrolling because scroll position restoration is baked into the transaction.
+    // FIXME: It seems fairly dirty to type-check the DrawingArea like this.
+    if (auto drawingArea = m_webPageProxy.drawingArea()) {
+        if (is<RemoteLayerTreeDrawingAreaProxy>(drawingArea))
+            m_snapshotRemovalTracker.cancelOutstandingEvent(SnapshotRemovalTracker::ScrollPositionRestoration);
+    }
+
     checkForActiveLoads();
 }
 
index efb34d2..59dcf08 100644 (file)
@@ -6085,4 +6085,9 @@ void WebPageProxy::setShouldScaleViewToFitDocument(bool shouldScaleViewToFitDocu
     m_process->send(Messages::WebPage::SetShouldScaleViewToFitDocument(shouldScaleViewToFitDocument), m_pageID);
 }
 
+void WebPageProxy::didRestoreScrollPosition()
+{
+    m_pageClient.didRestoreScrollPosition();
+}
+
 } // namespace WebKit
index fb92a72..ac3d6b9 100644 (file)
@@ -1071,6 +1071,8 @@ public:
 
     void didLayout(uint32_t layoutMilestones);
 
+    void didRestoreScrollPosition();
+
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, uint64_t pageID, Ref<API::PageConfiguration>&&);
     void platformInitialize();
index 64f0fe9..cc9f410 100644 (file)
@@ -445,4 +445,6 @@ messages -> WebPageProxy {
 #if ENABLE(VIDEO) && USE(GSTREAMER)
     RequestInstallMissingMediaPlugins(String details, String description)
 #endif
+
+    DidRestoreScrollPosition()
 }
index f417faf..d1ef83f 100644 (file)
@@ -93,6 +93,8 @@ private:
     virtual void refView() override final { }
     virtual void derefView() override final { }
 
+    virtual void didRestoreScrollPosition() override final { }
+
 #if USE(GSTREAMER)
     virtual bool decicePolicyForInstallMissingMediaPluginsPermissionRequest(InstallMissingMediaPluginsPermissionRequest&) override final { return false; };
 #endif
index b8dbae6..b7a6dd5 100644 (file)
@@ -191,6 +191,8 @@ private:
     virtual void refView() override;
     virtual void derefView() override;
 
+    virtual void didRestoreScrollPosition() override;
+
     WKContentView *m_contentView;
     WKWebView *m_webView;
     RetainPtr<WKEditorUndoTargetObjC> m_undoTarget;
index 7fbf527..a099036 100644 (file)
@@ -752,6 +752,10 @@ void PageClientImpl::derefView()
     [m_webView release];
 }
 
+void PageClientImpl::didRestoreScrollPosition()
+{
+}
+
 } // namespace WebKit
 
 #endif // PLATFORM(IOS)
index fa6fdf8..be1453b 100644 (file)
@@ -237,6 +237,8 @@ private:
 
     virtual void refView() override;
     virtual void derefView() override;
+
+    virtual void didRestoreScrollPosition() override;
 };
 
 } // namespace WebKit
index e5272b2..896b3d6 100644 (file)
@@ -827,6 +827,11 @@ _WKRemoteObjectRegistry *PageClientImpl::remoteObjectRegistry()
 }
 #endif
 
+void PageClientImpl::didRestoreScrollPosition()
+{
+    m_impl->didRestoreScrollPosition();
+}
+
 } // namespace WebKit
 
 #endif // PLATFORM(MAC)
index 1beeff2..c282deb 100644 (file)
@@ -734,10 +734,11 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
 
     m_swipeCancellationTracker = nullptr;
 
+#if ENABLE(LEGACY_SWIPE_SHADOW_STYLE)
     CALayer *rootLayer = m_webPageProxy.acceleratedCompositingRootLayer();
-
-    [rootLayer setShadowOpacity:0];
-    [rootLayer setShadowRadius:0];
+    rootLayer.shadowOpacity = 0;
+    rootLayer.shadowRadius = 0;
+#endif
 
     if (cancelled) {
         removeSwipeSnapshot();
@@ -754,11 +755,10 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
     m_webPageProxy.navigationGestureDidEnd(true, *targetItem);
     m_webPageProxy.goToBackForwardItem(targetItem);
 
-    // FIXME: We should be able to include scroll position restoration here,
-    // and then can address the FIXME in didFirstVisuallyNonEmptyLayoutForMainFrame.
     SnapshotRemovalTracker::Events desiredEvents = SnapshotRemovalTracker::VisuallyNonEmptyLayout
         | SnapshotRemovalTracker::MainFrameLoad
-        | SnapshotRemovalTracker::SubresourceLoads;
+        | SnapshotRemovalTracker::SubresourceLoads
+        | SnapshotRemovalTracker::ScrollPositionRestoration;
     if (renderTreeSize)
         desiredEvents |= SnapshotRemovalTracker::RenderTreeSizeThreshold;
     m_snapshotRemovalTracker.start(desiredEvents, [this] { this->forceRepaintIfNeeded(); });
index 2e744a0..a12b905 100644 (file)
@@ -1726,4 +1726,13 @@ void WebFrameLoaderClient::prefetchDNS(const String& hostname)
     WebProcess::singleton().prefetchDNS(hostname);
 }
 
+void WebFrameLoaderClient::didRestoreScrollPosition()
+{
+    WebPage* webPage = m_frame->page();
+    if (!webPage)
+        return;
+
+    webPage->didRestoreScrollPosition();
+}
+
 } // namespace WebKit
index 657ffdb..fff0309 100644 (file)
@@ -248,6 +248,8 @@ private:
 
     void prefetchDNS(const String&) override;
 
+    void didRestoreScrollPosition() override;
+
     WebFrame* m_frame;
     RefPtr<PluginView> m_pluginView;
     bool m_hasSentResponseToPluginView;
index 47375a5..d15cb7f 100644 (file)
@@ -5083,4 +5083,9 @@ void WebPage::dispatchDidLayout(WebCore::LayoutMilestones milestones)
     send(Messages::WebPageProxy::DidLayout(milestones));
 }
 
+void WebPage::didRestoreScrollPosition()
+{
+    send(Messages::WebPageProxy::DidRestoreScrollPosition());
+}
+
 } // namespace WebKit
index c6f9dc6..6665131 100644 (file)
@@ -916,6 +916,8 @@ public:
 
     void dispatchDidLayout(WebCore::LayoutMilestones);
 
+    void didRestoreScrollPosition();
+
 private:
     WebPage(uint64_t pageID, const WebPageCreationParameters&);