[iOS WK2] When zoomed, fixed elements jump at the start of a scroll, and jump back...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 May 2014 03:56:10 +0000 (03:56 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 May 2014 03:56:10 +0000 (03:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=132978
<rdar://problem/16894428>

Reviewed by Benjamin Poulain.

Source/WebCore:

This was the actual cause of the bug; r168560 changed the rect passed in here
to be the customFixedPositionRect rather than the unobscured rect, but we
used it to call FrameView::rectForViewportConstrainedObjects() which gave back
another bogus rect. So just use the rect passed in.

* page/scrolling/ios/ScrollingTreeScrollingNodeIOS.mm:
(WebCore::ScrollingTreeScrollingNodeIOS::updateLayersAfterViewportChange):

Source/WebKit2:

Move the static function fixedPositionRectFromExposedRect() to a member function on
WebPageProxy so we can call it from more places. Also never give WebCore a customFixedPosition
rect that extends past the document bounds, but allow rubber-banding/pinching in the UI process to
move fixed elements outside the document.

* UIProcess/PageClient.h: Need to expose minimumZoomScale() and contentsSize() to WebPageProxy.
* UIProcess/WebPageProxy.h:
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::minimumZoomScale):
(WebKit::PageClientImpl::contentsSize):
* UIProcess/ios/WKContentView.mm:
(-[WKContentView didUpdateVisibleRect:unobscuredRect:unobscuredRectInScrollViewCoordinates:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:]):
Pass the computeCustomFixedPositionRect(ConstrainedToDocumentRect) to WebCore, but use computeCustomFixedPositionRect()
for the ScrollingCoordinator update.
(adjustedUnexposedEdge): Deleted.
(adjustedUnexposedMaxEdge): Deleted.
(fixedPositionRectFromExposedRect): Deleted.
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::adjustedUnexposedEdge):
(WebKit::adjustedUnexposedMaxEdge):
(WebKit::WebPageProxy::computeCustomFixedPositionRect):
* UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree): This fixes some flashing when the scrolling tree
was being updated while scrolling; we now pass the correct rect.

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

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ios/ScrollingTreeScrollingNodeIOS.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/ios/WKContentView.mm
Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm
Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm

index 0a3ecd9..98ef708 100644 (file)
@@ -1,3 +1,19 @@
+2014-05-15  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] When zoomed, fixed elements jump at the start of a scroll, and jump back at the end.
+        https://bugs.webkit.org/show_bug.cgi?id=132978
+        <rdar://problem/16894428>
+
+        Reviewed by Benjamin Poulain.
+
+        This was the actual cause of the bug; r168560 changed the rect passed in here
+        to be the customFixedPositionRect rather than the unobscured rect, but we
+        used it to call FrameView::rectForViewportConstrainedObjects() which gave back
+        another bogus rect. So just use the rect passed in.
+
+        * page/scrolling/ios/ScrollingTreeScrollingNodeIOS.mm:
+        (WebCore::ScrollingTreeScrollingNodeIOS::updateLayersAfterViewportChange):
+
 2014-05-15  Daniel Bates  <dabates@apple.com>
 
         SVG element may reference arbitrary DOM element before running its insertion logic
index 430db3f..96eadc6 100644 (file)
@@ -137,18 +137,16 @@ void ScrollingTreeScrollingNodeIOS::setScrollLayerPosition(const FloatPoint& scr
     updateChildNodesAfterScroll(scrollPosition);
 }
 
-void ScrollingTreeScrollingNodeIOS::updateLayersAfterViewportChange(const FloatRect& viewportRect, double scale)
+void ScrollingTreeScrollingNodeIOS::updateLayersAfterViewportChange(const FloatRect& viewportRect, double /*scale*/)
 {
     [m_counterScrollingLayer setPosition:viewportRect.location()];
 
     if (!m_children)
         return;
 
-    FloatRect viewportConstrainedObjectsRect = FrameView::rectForViewportConstrainedObjects(enclosingLayoutRect(viewportRect), totalContentsSize(), scale, false, scrollBehaviorForFixedElements());
-
     size_t size = m_children->size();
     for (size_t i = 0; i < size; ++i)
-        m_children->at(i)->parentScrollPositionDidChange(viewportConstrainedObjectsRect, FloatSize());
+        m_children->at(i)->parentScrollPositionDidChange(viewportRect, FloatSize());
 }
 
 void ScrollingTreeScrollingNodeIOS::updateLayersAfterDelegatedScroll(const FloatPoint& scrollPosition)
index 9e1523a..bf04943 100644 (file)
@@ -1,3 +1,37 @@
+2014-05-15  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] When zoomed, fixed elements jump at the start of a scroll, and jump back at the end.
+        https://bugs.webkit.org/show_bug.cgi?id=132978
+        <rdar://problem/16894428>
+
+        Reviewed by Benjamin Poulain.
+        
+        Move the static function fixedPositionRectFromExposedRect() to a member function on
+        WebPageProxy so we can call it from more places. Also never give WebCore a customFixedPosition
+        rect that extends past the document bounds, but allow rubber-banding/pinching in the UI process to
+        move fixed elements outside the document.
+        
+        * UIProcess/PageClient.h: Need to expose minimumZoomScale() and contentsSize() to WebPageProxy.
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::minimumZoomScale):
+        (WebKit::PageClientImpl::contentsSize):
+        * UIProcess/ios/WKContentView.mm:
+        (-[WKContentView didUpdateVisibleRect:unobscuredRect:unobscuredRectInScrollViewCoordinates:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:]):
+        Pass the computeCustomFixedPositionRect(ConstrainedToDocumentRect) to WebCore, but use computeCustomFixedPositionRect()
+        for the ScrollingCoordinator update.
+        (adjustedUnexposedEdge): Deleted.
+        (adjustedUnexposedMaxEdge): Deleted.
+        (fixedPositionRectFromExposedRect): Deleted.
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::adjustedUnexposedEdge):
+        (WebKit::adjustedUnexposedMaxEdge):
+        (WebKit::WebPageProxy::computeCustomFixedPositionRect):
+        * UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:
+        (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree): This fixes some flashing when the scrolling tree
+        was being updated while scrolling; we now pass the correct rect.
+
 2014-05-15  Mark Rowe  <mrowe@apple.com>
 
         <https://webkit.org/b/132976> Move discovery of sharing services off the main thread
index 6bb5634..ecb7892 100644 (file)
@@ -257,6 +257,8 @@ public:
     virtual void showPlaybackTargetPicker(bool hasVideo, const WebCore::IntRect& elementRect) = 0;
     virtual void zoomToRect(WebCore::FloatRect, double minimumScale, double maximumScale) = 0;
     virtual void didChangeViewportMetaTagWidth(float) = 0;
+    virtual double minimumZoomScale() const = 0;
+    virtual WebCore::FloatSize contentsSize() const = 0;
 
 #if ENABLE(INSPECTOR)
     virtual void showInspectorIndication() = 0;
index 66896db..2936a6f 100644 (file)
@@ -589,6 +589,9 @@ public:
     uint64_t nextVisibleContentRectUpdateID() const { return m_lastVisibleContentRectUpdate.updateID() + 1; }
     uint64_t lastVisibleContentRectUpdateID() const { return m_lastVisibleContentRectUpdate.updateID(); }
 
+    enum class UnobscuredRectConstraint { ConstrainedToDocumentRect, Unconstrained };
+    WebCore::FloatRect computeCustomFixedPositionRect(const WebCore::FloatRect& unobscuredContentRect, double displayedContentScale, UnobscuredRectConstraint = UnobscuredRectConstraint::Unconstrained) const;
+
     void dynamicViewportSizeUpdate(const WebCore::FloatSize& minimumLayoutSize, const WebCore::FloatRect& targetExposedContentRect, const WebCore::FloatRect& targetUnobscuredRect, const WebCore::FloatRect& targetUnobscuredRectInScrollViewCoordinates, double targetScale);
     
     void setViewportConfigurationMinimumLayoutSize(const WebCore::FloatSize&);
index 2f5b269..8a536c5 100644 (file)
@@ -123,6 +123,8 @@ private:
 
     virtual bool handleRunOpenPanel(WebPageProxy*, WebFrameProxy*, WebOpenPanelParameters*, WebOpenPanelResultListenerProxy*) override;
     virtual void didChangeViewportMetaTagWidth(float) override;
+    virtual double minimumZoomScale() const override;
+    virtual WebCore::FloatSize contentsSize() const override;
 
 #if ENABLE(INSPECTOR)
     virtual void showInspectorIndication() override;
index 5d16bcc..52b5da5 100644 (file)
@@ -187,6 +187,19 @@ void PageClientImpl::didChangeViewportMetaTagWidth(float newWidth)
     [m_webView _setViewportMetaTagWidth:newWidth];
 }
 
+double PageClientImpl::minimumZoomScale() const
+{
+    if (UIScrollView *scroller = [m_webView scrollView])
+        return scroller.minimumZoomScale;
+
+    return 1;
+}
+
+WebCore::FloatSize PageClientImpl::contentsSize() const
+{
+    return FloatSize([m_contentView bounds].size);
+}
+
 void PageClientImpl::setCursor(const Cursor&)
 {
     notImplemented();
index 9b0d6ec..8e58b4f 100644 (file)
@@ -292,40 +292,6 @@ private:
     }
 }
 
-static inline float adjustedUnexposedEdge(float documentEdge, float exposedRectEdge, float factor)
-{
-    if (exposedRectEdge < documentEdge)
-        return documentEdge - factor * (documentEdge - exposedRectEdge);
-    
-    return exposedRectEdge;
-}
-
-static inline float adjustedUnexposedMaxEdge(float documentEdge, float exposedRectEdge, float factor)
-{
-    if (exposedRectEdge > documentEdge)
-        return documentEdge + factor * (exposedRectEdge - documentEdge);
-    
-    return exposedRectEdge;
-}
-
-static inline FloatRect fixedPositionRectFromExposedRect(CGRect unobscuredRect, CGSize documentSize, CGFloat scale, CGFloat minimumScale)
-{
-    FloatRect constrainedUnobscuredRect = unobscuredRect;
-    FloatRect documentRect = FloatRect(FloatPoint(), FloatSize(documentSize));
-    
-    if (scale < minimumScale) {
-        const CGFloat slope = 12;
-        CGFloat factor = std::max<CGFloat>(1 - slope * (minimumScale - scale), 0);
-            
-        constrainedUnobscuredRect.setX(adjustedUnexposedEdge(documentRect.x(), constrainedUnobscuredRect.x(), factor));
-        constrainedUnobscuredRect.setY(adjustedUnexposedEdge(documentRect.y(), constrainedUnobscuredRect.y(), factor));
-        constrainedUnobscuredRect.setWidth(adjustedUnexposedMaxEdge(documentRect.maxX(), constrainedUnobscuredRect.maxX(), factor) - constrainedUnobscuredRect.x());
-        constrainedUnobscuredRect.setHeight(adjustedUnexposedMaxEdge(documentRect.maxY(), constrainedUnobscuredRect.maxY(), factor) - constrainedUnobscuredRect.y());
-    }
-    
-    return FrameView::rectForViewportConstrainedObjects(enclosingLayoutRect(constrainedUnobscuredRect), roundedLayoutSize(FloatSize(documentSize)), scale, false, StickToViewportBounds);
-}
-
 - (void)didUpdateVisibleRect:(CGRect)visibleRect unobscuredRect:(CGRect)unobscuredRect unobscuredRectInScrollViewCoordinates:(CGRect)unobscuredRectInScrollViewCoordinates
     scale:(CGFloat)zoomScale minimumScale:(CGFloat)minimumScale inStableState:(BOOL)isStableState isChangingObscuredInsetsInteractively:(BOOL)isChangingObscuredInsetsInteractively
 {
@@ -344,11 +310,12 @@ static inline FloatRect fixedPositionRectFromExposedRect(CGRect unobscuredRect,
         filteredScale = _page->displayedContentScale();
     }
 
-    FloatRect customFixedPositionRect = fixedPositionRectFromExposedRect(unobscuredRect, [self bounds].size, zoomScale, minimumScale);
-    _page->updateVisibleContentRects(VisibleContentRectUpdateInfo(_page->nextVisibleContentRectUpdateID(), visibleRect, unobscuredRect, unobscuredRectInScrollViewCoordinates, customFixedPositionRect, filteredScale, isStableState, isChangingObscuredInsetsInteractively, timestamp, velocityData.horizontalVelocity, velocityData.verticalVelocity, velocityData.scaleChangeRate));
-    
+    FloatRect fixedPositionRectForLayout = _page->computeCustomFixedPositionRect(unobscuredRect, zoomScale, WebPageProxy::UnobscuredRectConstraint::ConstrainedToDocumentRect);
+    _page->updateVisibleContentRects(VisibleContentRectUpdateInfo(_page->nextVisibleContentRectUpdateID(), visibleRect, unobscuredRect, unobscuredRectInScrollViewCoordinates, fixedPositionRectForLayout,
+        filteredScale, isStableState, isChangingObscuredInsetsInteractively, timestamp, velocityData.horizontalVelocity, velocityData.verticalVelocity, velocityData.scaleChangeRate));
+
     RemoteScrollingCoordinatorProxy* scrollingCoordinator = _page->scrollingCoordinatorProxy();
-    scrollingCoordinator->viewportChangedViaDelegatedScrolling(scrollingCoordinator->rootScrollingNodeID(), customFixedPositionRect, zoomScale);
+    scrollingCoordinator->viewportChangedViaDelegatedScrolling(scrollingCoordinator->rootScrollingNodeID(), _page->computeCustomFixedPositionRect(_page->unobscuredContentRect(), zoomScale), zoomScale);
 
     if (auto drawingArea = _page->drawingArea())
         drawingArea->updateDebugIndicator();
index 37b9cbc..6410070 100644 (file)
@@ -41,6 +41,7 @@
 #import "WebPageMessages.h"
 #import "WebProcessProxy.h"
 #import "WebVideoFullscreenManagerProxy.h"
+#import <WebCore/FrameView.h>
 #import <WebCore/NotImplemented.h>
 #import <WebCore/SharedBuffer.h>
 #import <WebCore/UserAgent.h>
@@ -194,6 +195,46 @@ bool WebPageProxy::updateVisibleContentRects(const VisibleContentRectUpdateInfo&
     return true;
 }
 
+static inline float adjustedUnexposedEdge(float documentEdge, float exposedRectEdge, float factor)
+{
+    if (exposedRectEdge < documentEdge)
+        return documentEdge - factor * (documentEdge - exposedRectEdge);
+    
+    return exposedRectEdge;
+}
+
+static inline float adjustedUnexposedMaxEdge(float documentEdge, float exposedRectEdge, float factor)
+{
+    if (exposedRectEdge > documentEdge)
+        return documentEdge + factor * (exposedRectEdge - documentEdge);
+    
+    return exposedRectEdge;
+}
+
+WebCore::FloatRect WebPageProxy::computeCustomFixedPositionRect(const FloatRect& unobscuredContentRect, double displayedContentScale, UnobscuredRectConstraint constraint) const
+{
+    FloatRect constrainedUnobscuredRect = unobscuredContentRect;
+    FloatSize contentsSize = m_pageClient.contentsSize();
+    FloatRect documentRect = FloatRect(FloatPoint(), contentsSize);
+
+    if (constraint == UnobscuredRectConstraint::ConstrainedToDocumentRect)
+        constrainedUnobscuredRect.intersect(documentRect);
+
+    double minimumScale = m_pageClient.minimumZoomScale();
+    
+    if (displayedContentScale < minimumScale) {
+        const CGFloat slope = 12;
+        CGFloat factor = std::max<CGFloat>(1 - slope * (minimumScale - displayedContentScale), 0);
+            
+        constrainedUnobscuredRect.setX(adjustedUnexposedEdge(documentRect.x(), constrainedUnobscuredRect.x(), factor));
+        constrainedUnobscuredRect.setY(adjustedUnexposedEdge(documentRect.y(), constrainedUnobscuredRect.y(), factor));
+        constrainedUnobscuredRect.setWidth(adjustedUnexposedMaxEdge(documentRect.maxX(), constrainedUnobscuredRect.maxX(), factor) - constrainedUnobscuredRect.x());
+        constrainedUnobscuredRect.setHeight(adjustedUnexposedMaxEdge(documentRect.maxY(), constrainedUnobscuredRect.maxY(), factor) - constrainedUnobscuredRect.y());
+    }
+    
+    return FrameView::rectForViewportConstrainedObjects(enclosingLayoutRect(constrainedUnobscuredRect), roundedLayoutSize(contentsSize), displayedContentScale, false, StickToViewportBounds);
+}
+
 void WebPageProxy::dynamicViewportSizeUpdate(const FloatSize& minimumLayoutSize, const FloatRect& targetExposedContentRect, const FloatRect& targetUnobscuredRect, const FloatRect& targetUnobscuredRectInScrollViewCoordinates,  double targetScale)
 {
     m_process->send(Messages::WebPage::DynamicViewportSizeUpdate(minimumLayoutSize, targetExposedContentRect, targetUnobscuredRect, targetUnobscuredRectInScrollViewCoordinates, targetScale), m_pageID);
index 85346ff..89b200d 100644 (file)
@@ -134,8 +134,9 @@ void RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const RemoteLayerTreeTrans
 
 #if PLATFORM(IOS)
     if (fixedOrStickyLayerChanged) {
+        FloatRect customFixedPositionRect = m_webPageProxy->computeCustomFixedPositionRect(m_webPageProxy->unobscuredContentRect(), m_webPageProxy->displayedContentScale());
         // If we got a new layer for a fixed or sticky node, its position from the WebProcess is probably stale. We need to re-run the "viewport" changed logic to udpate it with our UI-side state.
-        m_webPageProxy->scrollingCoordinatorProxy()->viewportChangedViaDelegatedScrolling(m_webPageProxy->scrollingCoordinatorProxy()->rootScrollingNodeID(), m_webPageProxy->unobscuredContentRect(), m_webPageProxy->displayedContentScale());
+        m_webPageProxy->scrollingCoordinatorProxy()->viewportChangedViaDelegatedScrolling(m_webPageProxy->scrollingCoordinatorProxy()->rootScrollingNodeID(), customFixedPositionRect, m_webPageProxy->displayedContentScale());
     }
 #endif
 #endif // ENABLE(ASYNC_SCROLLING)