[iOS WK2] Swiping back just after scrolling can cause some tiles to disappear
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jun 2015 00:29:58 +0000 (00:29 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jun 2015 00:29:58 +0000 (00:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=146329
rdar://problem/21233010

Reviewed by Tim Horton.
Source/WebCore:

Have the Compositing log channel dump the visible rect used for layer flushing.

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::flushPendingLayerChanges):

Source/WebKit2:

When doing a back swipe, views interposed between the WKWebView and the WKContentView
get positions and animations for the swipe. This -_updateVisibleContentRects to
compute bad visible and unobscured rects, so we lose tiles.

Fix by "freezing" the visible and unobscured content rects in the view being
swiped for the duration of the navigation gesture. When swiping the main view,
we just plumb through navigationGestureDidEnd(). When Reader is showing and the
swiped view is different from the navigating view, use the new navigationGestureDidEnd()
override which takes no arguments.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _updateVisibleContentRects]):
(-[WKWebView _navigationGestureDidBegin]):
(-[WKWebView _navigationGestureDidEnd]):
* UIProcess/API/Cocoa/WKWebViewInternal.h:
* UIProcess/API/gtk/PageClientImpl.cpp:
(WebKit::PageClientImpl::navigationGestureDidEnd):
* UIProcess/API/gtk/PageClientImpl.h:
* UIProcess/CoordinatedGraphics/WebView.h:
* UIProcess/PageClient.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::navigationGestureDidEnd):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::navigationGestureDidBegin):
(WebKit::PageClientImpl::navigationGestureDidEnd):
* UIProcess/ios/ViewGestureControllerIOS.mm:
(WebKit::ViewGestureController::beginSwipeGesture):
(WebKit::ViewGestureController::endSwipeGesture):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::navigationGestureDidEnd):

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

16 files changed:
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.h
Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.h
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/ios/ViewGestureControllerIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm

index 66c2ddb..5eb097e 100644 (file)
@@ -1,3 +1,16 @@
+2015-06-25  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Swiping back just after scrolling can cause some tiles to disappear
+        https://bugs.webkit.org/show_bug.cgi?id=146329
+        rdar://problem/21233010
+
+        Reviewed by Tim Horton.
+
+        Have the Compositing log channel dump the visible rect used for layer flushing.
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::flushPendingLayerChanges):
+
 2015-06-25  Brent Fulgham  <bfulgham@apple.com>
 
         [WIN] Enable WEB_TIMING API
index 0295dd0..84ea43b 100644 (file)
@@ -467,12 +467,18 @@ void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
 
     if (GraphicsLayer* rootLayer = rootGraphicsLayer()) {
 #if PLATFORM(IOS)
-        rootLayer->flushCompositingState(frameView.exposedContentRect());
+        FloatRect exposedRect = frameView.exposedContentRect();
+        LOG(Compositing, "RenderLayerCompositor %p flushPendingLayerChanges(%d) %.2f, %.2f, %.2fx%.2f", this, isFlushRoot,
+            exposedRect.x(), exposedRect.y(), exposedRect.width(), exposedRect.height());
+        rootLayer->flushCompositingState(exposedRect);
 #else
         // Having a m_clipLayer indicates that we're doing scrolling via GraphicsLayers.
         IntRect visibleRect = m_clipLayer ? IntRect(IntPoint(), frameView.unscaledVisibleContentSizeIncludingObscuredArea()) : frameView.visibleContentRect();
         if (!frameView.exposedRect().isInfinite())
             visibleRect.intersect(IntRect(frameView.exposedRect()));
+
+        LOG(Compositing, "RenderLayerCompositor %p flushPendingLayerChanges(%d) %d, %d, %dx%d", this, isFlushRoot,
+            visibleRect.x(), visibleRect.y(), visibleRect.width(), visibleRect.height());
         rootLayer->flushCompositingState(visibleRect);
 #endif
     }
index 2e04f7a..6f5a54b 100644 (file)
@@ -1,3 +1,45 @@
+2015-06-25  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Swiping back just after scrolling can cause some tiles to disappear
+        https://bugs.webkit.org/show_bug.cgi?id=146329
+        rdar://problem/21233010
+
+        Reviewed by Tim Horton.
+        
+        When doing a back swipe, views interposed between the WKWebView and the WKContentView
+        get positions and animations for the swipe. This -_updateVisibleContentRects to
+        compute bad visible and unobscured rects, so we lose tiles.
+        
+        Fix by "freezing" the visible and unobscured content rects in the view being
+        swiped for the duration of the navigation gesture. When swiping the main view,
+        we just plumb through navigationGestureDidEnd(). When Reader is showing and the
+        swiped view is different from the navigating view, use the new navigationGestureDidEnd()
+        override which takes no arguments.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _updateVisibleContentRects]):
+        (-[WKWebView _navigationGestureDidBegin]):
+        (-[WKWebView _navigationGestureDidEnd]):
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        * UIProcess/API/gtk/PageClientImpl.cpp:
+        (WebKit::PageClientImpl::navigationGestureDidEnd):
+        * UIProcess/API/gtk/PageClientImpl.h:
+        * UIProcess/CoordinatedGraphics/WebView.h:
+        * UIProcess/PageClient.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::navigationGestureDidEnd):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::navigationGestureDidBegin):
+        (WebKit::PageClientImpl::navigationGestureDidEnd):
+        * UIProcess/ios/ViewGestureControllerIOS.mm:
+        (WebKit::ViewGestureController::beginSwipeGesture):
+        (WebKit::ViewGestureController::endSwipeGesture):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::navigationGestureDidEnd):
+
 2015-06-25  Joseph Pecoraro  <pecoraro@apple.com>
 
         [Mac] Web Inspector: Window dragging on toolbar should behave more like native window dragging
index c118522..ece894d 100644 (file)
@@ -83,6 +83,7 @@
 #import <wtf/HashMap.h>
 #import <wtf/MathExtras.h>
 #import <wtf/NeverDestroyed.h>
+#import <wtf/Optional.h>
 #import <wtf/RetainPtr.h>
 
 #if PLATFORM(IOS)
@@ -189,6 +190,8 @@ WKWebView* fromWebPageProxy(WebKit::WebPageProxy& page)
     uint64_t _resizeAnimationTransformTransactionID;
     RetainPtr<UIView> _resizeAnimationView;
     CGFloat _lastAdjustmentForScroller;
+    Optional<CGRect> _frozenVisibleContentRect;
+    Optional<CGRect> _frozenUnobscuredContentRect;
 
     BOOL _needsToRestoreExposedRect;
     WebCore::FloatRect _exposedRectToRestore;
@@ -1559,10 +1562,10 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
         return;
 
     CGRect fullViewRect = self.bounds;
-    CGRect visibleRectInContentCoordinates = [self convertRect:fullViewRect toView:_contentView.get()];
+    CGRect visibleRectInContentCoordinates = _frozenVisibleContentRect ? _frozenVisibleContentRect.value() : [self convertRect:fullViewRect toView:_contentView.get()];
 
     CGRect unobscuredRect = UIEdgeInsetsInsetRect(fullViewRect, [self _computedContentInset]);
-    CGRect unobscuredRectInContentCoordinates = [self convertRect:unobscuredRect toView:_contentView.get()];
+    CGRect unobscuredRectInContentCoordinates = _frozenUnobscuredContentRect ? _frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()];
 
     CGFloat scaleFactor = contentZoomScale(self);
 
@@ -1710,7 +1713,25 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
     return _allowsBackForwardNavigationGestures;
 }
 
-#endif
+- (void)_navigationGestureDidBegin
+{
+    // During a back/forward swipe, there's a view interposed between this view and the content view that has
+    // an offset and animation on it, which results in computing incorrect rectangles. Work around by using
+    // frozen rects during swipes.
+    CGRect fullViewRect = self.bounds;
+    CGRect unobscuredRect = UIEdgeInsetsInsetRect(fullViewRect, [self _computedContentInset]);
+
+    _frozenVisibleContentRect = [self convertRect:fullViewRect toView:_contentView.get()];
+    _frozenUnobscuredContentRect = [self convertRect:unobscuredRect toView:_contentView.get()];
+}
+
+- (void)_navigationGestureDidEnd
+{
+    _frozenVisibleContentRect = Nullopt;
+    _frozenUnobscuredContentRect = Nullopt;
+}
+
+#endif // PLATFORM(IOS)
 
 #pragma mark OS X-specific methods
 
@@ -1800,7 +1821,7 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
 {
     return [_wkView performDragOperation:sender];
 }
-#endif
+#endif // PLATFORM(MAC)
 
 @end
 
index df3e7d7..a3849ff 100644 (file)
@@ -103,6 +103,9 @@ struct PrintInfo;
 
 - (void)_updateScrollViewBackground;
 
+- (void)_navigationGestureDidBegin;
+- (void)_navigationGestureDidEnd;
+
 @property (nonatomic, readonly) UIEdgeInsets _computedContentInset;
 #else
 @property (nonatomic, setter=_setIgnoresNonWheelEvents:) BOOL _ignoresNonWheelEvents;
index 33cece6..c3bc63e 100644 (file)
@@ -410,6 +410,10 @@ void PageClientImpl::navigationGestureDidEnd(bool, WebBackForwardListItem&)
 {
 }
 
+void PageClientImpl::navigationGestureDidEnd()
+{
+}
+
 void PageClientImpl::willRecordNavigationSnapshot(WebBackForwardListItem&)
 {
 }
index 2adb966..5ac50a2 100644 (file)
@@ -122,6 +122,7 @@ private:
     virtual void navigationGestureDidBegin() override;
     virtual void navigationGestureWillEnd(bool, WebBackForwardListItem&) override;
     virtual void navigationGestureDidEnd(bool, WebBackForwardListItem&) override;
+    virtual void navigationGestureDidEnd() override;
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override;
 
     virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
index 1089d63..97d8790 100644 (file)
@@ -203,6 +203,7 @@ protected:
     virtual void navigationGestureDidBegin() override { };
     virtual void navigationGestureWillEnd(bool, WebBackForwardListItem&) override { };
     virtual void navigationGestureDidEnd(bool, WebBackForwardListItem&) override { };
+    virtual void navigationGestureDidEnd() override { };
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override { };
 
     virtual void didChangeBackgroundColor() override { }
index de24d30..d2afacc 100644 (file)
@@ -307,6 +307,7 @@ public:
     virtual void navigationGestureDidBegin() = 0;
     virtual void navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem&) = 0;
     virtual void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&) = 0;
+    virtual void navigationGestureDidEnd() = 0;
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) = 0;
 
     virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() = 0;
index 62d6326..ceb2a77 100644 (file)
@@ -5746,6 +5746,11 @@ void WebPageProxy::navigationGestureDidEnd(bool willNavigate, WebBackForwardList
     m_loaderClient->navigationGestureDidEnd(*this, willNavigate, item);
 }
 
+void WebPageProxy::navigationGestureDidEnd()
+{
+    m_pageClient.navigationGestureDidEnd();
+}
+
 void WebPageProxy::willRecordNavigationSnapshot(WebBackForwardListItem& item)
 {
     m_pageClient.willRecordNavigationSnapshot(item);
index 8de146b..18bf50b 100644 (file)
@@ -1011,6 +1011,7 @@ public:
     void navigationGestureDidBegin();
     void navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem&);
     void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&);
+    void navigationGestureDidEnd();
     void navigationGestureSnapshotWasRemoved();
     void willRecordNavigationSnapshot(WebBackForwardListItem&);
 
index a0a448d..69b8026 100644 (file)
@@ -175,6 +175,7 @@ private:
     virtual void navigationGestureDidBegin() override;
     virtual void navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem&) override;
     virtual void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&) override;
+    virtual void navigationGestureDidEnd() override;
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override;
 
     virtual void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
index ca72a77..c68c290 100644 (file)
@@ -685,6 +685,7 @@ Vector<String> PageClientImpl::mimeTypesWithCustomContentProviders()
 
 void PageClientImpl::navigationGestureDidBegin()
 {
+    [m_webView _navigationGestureDidBegin];
     NavigationState::fromWebPage(*m_webView->_page).navigationGestureDidBegin();
 }
 
@@ -696,6 +697,12 @@ void PageClientImpl::navigationGestureWillEnd(bool willNavigate, WebBackForwardL
 void PageClientImpl::navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem& item)
 {
     NavigationState::fromWebPage(*m_webView->_page).navigationGestureDidEnd(willNavigate, item);
+    [m_webView _navigationGestureDidEnd];
+}
+
+void PageClientImpl::navigationGestureDidEnd()
+{
+    [m_webView _navigationGestureDidEnd];
 }
 
 void PageClientImpl::willRecordNavigationSnapshot(WebBackForwardListItem& item)
index ffcb98b..9f1b458 100644 (file)
@@ -184,6 +184,8 @@ void ViewGestureController::beginSwipeGesture(_UINavigationInteractiveTransition
 
     m_webPageProxyForBackForwardListForCurrentSwipe = m_alternateBackForwardListSourceView.get() ? m_alternateBackForwardListSourceView.get()->_page : &m_webPageProxy;
     m_webPageProxyForBackForwardListForCurrentSwipe->navigationGestureDidBegin();
+    if (&m_webPageProxy != m_webPageProxyForBackForwardListForCurrentSwipe)
+        m_webPageProxy.navigationGestureDidBegin();
 
     auto& backForwardList = m_webPageProxyForBackForwardListForCurrentSwipe->backForwardList();
 
@@ -285,6 +287,8 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
         RefPtr<WebPageProxy> webPageProxyForBackForwardListForCurrentSwipe = m_webPageProxyForBackForwardListForCurrentSwipe;
         removeSwipeSnapshot();
         webPageProxyForBackForwardListForCurrentSwipe->navigationGestureDidEnd(false, *targetItem);
+        if (&m_webPageProxy != webPageProxyForBackForwardListForCurrentSwipe)
+            m_webPageProxy.navigationGestureDidEnd();
         return;
     }
 
@@ -293,6 +297,9 @@ void ViewGestureController::endSwipeGesture(WebBackForwardListItem* targetItem,
         m_snapshotRemovalTargetRenderTreeSize = snapshot->renderTreeSize() * swipeSnapshotRemovalRenderTreeSizeTargetFraction;
 
     m_webPageProxyForBackForwardListForCurrentSwipe->navigationGestureDidEnd(true, *targetItem);
+    if (&m_webPageProxy != m_webPageProxyForBackForwardListForCurrentSwipe)
+        m_webPageProxy.navigationGestureDidEnd();
+
     m_webPageProxyForBackForwardListForCurrentSwipe->goToBackForwardItem(targetItem);
 
     if (auto drawingArea = m_webPageProxy.drawingArea()) {
index ac67496..da64d1f 100644 (file)
@@ -186,6 +186,7 @@ private:
     virtual void navigationGestureDidBegin() override;
     virtual void navigationGestureWillEnd(bool willNavigate, WebBackForwardListItem&) override;
     virtual void navigationGestureDidEnd(bool willNavigate, WebBackForwardListItem&) override;
+    virtual void navigationGestureDidEnd() override;
     virtual void willRecordNavigationSnapshot(WebBackForwardListItem&) override;
 
     NSView *activeView() const;
index a448d9e..b41aca5 100644 (file)
@@ -765,6 +765,10 @@ void PageClientImpl::navigationGestureDidEnd(bool willNavigate, WebBackForwardLi
 #endif
 }
 
+void PageClientImpl::navigationGestureDidEnd()
+{
+}
+
 void PageClientImpl::willRecordNavigationSnapshot(WebBackForwardListItem& item)
 {
 #if WK_API_ENABLED