REGRESSION (iOS 13): Top fixed element on apple.com flickers in size while pinching in
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Sep 2019 20:18:07 +0000 (20:18 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Sep 2019 20:18:07 +0000 (20:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201668
rdar://problem/51934041

Reviewed by Frédéric Wang.

Source/WebCore:

When computing the new layout viewport rect in ScrollingTreeFrameScrollingNode, use
"StickToDocumentBounds" mode, not "StickToViewportBounds", because otherwise we'll compute
a layout viewport that has negative top/left offsets which causes fixed elements to jump outside
the viewport. The only code that should be moving things outside the viewport (a temporary effect
that happens when pinching) is the 'isBelowMinimumScale' path in WebPageProxy::computeCustomFixedPositionRect().

With this change ScrollingTreeFrameScrollingNode no longer needs m_behaviorForFixed; it can be removed later.

Not currently testable, since it involves pinching in past minimum zoom and transients state.

* page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
(WebCore::ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition const):

Source/WebKit:

The UI process can have transient state that pushes scrolling-tree-managed layers into custom
locations while pinch-zooming. We have to apply this state both when the visible rects
in the UI process change (existing code in -[WKContentView didUpdateVisibleRect:...]) and when
we get new layers from the web process (added in RemoteLayerTreeDrawingAreaProxy::commitLayerTree()
in this patch).

Move some code into WebPageProxy to create functions that we can call from both places.

For manual testing, visit a page with fixed banners, pinch in slightly, then pinch out and,
while keeping your fingers down, move the contents around.

* UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentView.mm:
(-[WKContentView didUpdateVisibleRect:unobscuredRect:contentInsets:unobscuredRectInScrollViewCoordinates:obscuredInsets:unobscuredSafeAreaInsets:inputViewBounds:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]):
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::unconstrainedLayoutViewportRect const):
(WebKit::WebPageProxy::adjustLayersForLayoutViewport):

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

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTreeFrameScrollingNode.cpp
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/ios/WKContentView.mm
Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

index 913766b..c110d62 100644 (file)
         (WebCore::XPath::isValidContextNode):
         * xml/XPathUtil.h:
 
         (WebCore::XPath::isValidContextNode):
         * xml/XPathUtil.h:
 
+2019-09-11  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (iOS 13): Top fixed element on apple.com flickers in size while pinching in
+        https://bugs.webkit.org/show_bug.cgi?id=201668
+        rdar://problem/51934041
+
+        Reviewed by Frédéric Wang.
+
+        When computing the new layout viewport rect in ScrollingTreeFrameScrollingNode, use
+        "StickToDocumentBounds" mode, not "StickToViewportBounds", because otherwise we'll compute
+        a layout viewport that has negative top/left offsets which causes fixed elements to jump outside
+        the viewport. The only code that should be moving things outside the viewport (a temporary effect
+        that happens when pinching) is the 'isBelowMinimumScale' path in WebPageProxy::computeCustomFixedPositionRect().
+
+        With this change ScrollingTreeFrameScrollingNode no longer needs m_behaviorForFixed; it can be removed later.
+
+        Not currently testable, since it involves pinching in past minimum zoom and transients state.
+
+        * page/scrolling/ScrollingTreeFrameScrollingNode.cpp:
+        (WebCore::ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition const):
+
 2019-09-11  Truitt Savell  <tsavell@apple.com>
 
         Unreviewed, rolling out r249753.
 2019-09-11  Truitt Savell  <tsavell@apple.com>
 
         Unreviewed, rolling out r249753.
index 11c7269..dbd24d6 100644 (file)
@@ -102,12 +102,12 @@ FloatRect ScrollingTreeFrameScrollingNode::layoutViewportForScrollPosition(const
     LayoutRect visualViewport(FrameView::visibleDocumentRect(visibleContentRect, headerHeight(), footerHeight(), totalContentsSize(), scale));
     LayoutRect layoutViewport(m_layoutViewport);
 
     LayoutRect visualViewport(FrameView::visibleDocumentRect(visibleContentRect, headerHeight(), footerHeight(), totalContentsSize(), scale));
     LayoutRect layoutViewport(m_layoutViewport);
 
-    LOG_WITH_STREAM(Scrolling, stream << "\nScrolling thread: " << "(visibleContentOrigin " << visibleContentOrigin << ", visualViewportSize " << visualViewportSize << ") fixed behavior " << m_behaviorForFixed);
+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeFrameScrollingNode " << scrollingNodeID() << " layoutViewportForScrollPosition: " << "(visibleContentOrigin " << visibleContentOrigin << ", visualViewportSize " << visualViewportSize << ") fixed behavior " << m_behaviorForFixed);
     LOG_WITH_STREAM(Scrolling, stream << "  layoutViewport: " << layoutViewport);
     LOG_WITH_STREAM(Scrolling, stream << "  visualViewport: " << visualViewport);
     LOG_WITH_STREAM(Scrolling, stream << "  scroll positions: min: " << minLayoutViewportOrigin() << " max: "<< maxLayoutViewportOrigin());
 
     LOG_WITH_STREAM(Scrolling, stream << "  layoutViewport: " << layoutViewport);
     LOG_WITH_STREAM(Scrolling, stream << "  visualViewport: " << visualViewport);
     LOG_WITH_STREAM(Scrolling, stream << "  scroll positions: min: " << minLayoutViewportOrigin() << " max: "<< maxLayoutViewportOrigin());
 
-    LayoutPoint newLocation = FrameView::computeLayoutViewportOrigin(LayoutRect(visualViewport), LayoutPoint(minLayoutViewportOrigin()), LayoutPoint(maxLayoutViewportOrigin()), layoutViewport, m_behaviorForFixed);
+    LayoutPoint newLocation = FrameView::computeLayoutViewportOrigin(LayoutRect(visualViewport), LayoutPoint(minLayoutViewportOrigin()), LayoutPoint(maxLayoutViewportOrigin()), layoutViewport, StickToDocumentBounds);
 
     if (layoutViewport.location() != newLocation) {
         layoutViewport.setLocation(newLocation);
 
     if (layoutViewport.location() != newLocation) {
         layoutViewport.setLocation(newLocation);
index be2c74c..9a6218d 100644 (file)
         https://bugs.webkit.org/show_bug.cgi?id=201655
         https://trac.webkit.org/changeset/249768
 
         https://bugs.webkit.org/show_bug.cgi?id=201655
         https://trac.webkit.org/changeset/249768
 
+2019-09-11  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (iOS 13): Top fixed element on apple.com flickers in size while pinching in
+        https://bugs.webkit.org/show_bug.cgi?id=201668
+        rdar://problem/51934041
+
+        Reviewed by Frédéric Wang.
+
+        The UI process can have transient state that pushes scrolling-tree-managed layers into custom
+        locations while pinch-zooming. We have to apply this state both when the visible rects
+        in the UI process change (existing code in -[WKContentView didUpdateVisibleRect:...]) and when
+        we get new layers from the web process (added in RemoteLayerTreeDrawingAreaProxy::commitLayerTree()
+        in this patch).
+
+        Move some code into WebPageProxy to create functions that we can call from both places.
+
+        For manual testing, visit a page with fixed banners, pinch in slightly, then pinch out and,
+        while keeping your fingers down, move the contents around.
+
+        * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
+        (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/WKContentView.mm:
+        (-[WKContentView didUpdateVisibleRect:unobscuredRect:contentInsets:unobscuredRectInScrollViewCoordinates:obscuredInsets:unobscuredSafeAreaInsets:inputViewBounds:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]):
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::unconstrainedLayoutViewportRect const):
+        (WebKit::WebPageProxy::adjustLayersForLayoutViewport):
+
 2019-09-11  Antti Koivisto  <antti@apple.com>
 
         REGRESSION (245006): can't scroll in "read more" view in Eventbrite app
 2019-09-11  Antti Koivisto  <antti@apple.com>
 
         REGRESSION (245006): can't scroll in "read more" view in Eventbrite app
index b6ecb02..728d8b0 100644 (file)
@@ -218,6 +218,9 @@ void RemoteLayerTreeDrawingAreaProxy::commitLayerTree(const RemoteLayerTreeTrans
 
 #if ENABLE(ASYNC_SCROLLING)
     m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositionsAfterCommit();
 
 #if ENABLE(ASYNC_SCROLLING)
     m_webPageProxy.scrollingCoordinatorProxy()->applyScrollingTreeLayerPositionsAfterCommit();
+#if PLATFORM(IOS_FAMILY)
+    m_webPageProxy.adjustLayersForLayoutViewport(m_webPageProxy.unconstrainedLayoutViewportRect());
+#endif
 
     // Handle requested scroll position updates from the scrolling tree transaction after didCommitLayerTree()
     // has updated the view size based on the content size.
 
     // Handle requested scroll position updates from the scrolling tree transaction after didCommitLayerTree()
     // has updated the view size based on the content size.
index 6abe827..3ab7b2b 100644 (file)
@@ -661,6 +661,9 @@ public:
 
     WebCore::FloatRect computeCustomFixedPositionRect(const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& currentCustomFixedPositionRect, double displayedContentScale, WebCore::FrameView::LayoutViewportConstraint = WebCore::FrameView::LayoutViewportConstraint::Unconstrained) const;
 
 
     WebCore::FloatRect computeCustomFixedPositionRect(const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& currentCustomFixedPositionRect, double displayedContentScale, WebCore::FrameView::LayoutViewportConstraint = WebCore::FrameView::LayoutViewportConstraint::Unconstrained) const;
 
+    WebCore::FloatRect unconstrainedLayoutViewportRect() const;
+    void adjustLayersForLayoutViewport(const WebCore::FloatRect& layoutViewport);
+
     void scrollingNodeScrollViewWillStartPanGesture();
     void scrollingNodeScrollViewDidScroll();
     void scrollingNodeScrollWillStartScroll();
     void scrollingNodeScrollViewWillStartPanGesture();
     void scrollingNodeScrollViewDidScroll();
     void scrollingNodeScrollWillStartScroll();
index 5939b39..621ea93 100644 (file)
@@ -36,7 +36,6 @@
 #import "PageClientImplIOS.h"
 #import "PrintInfo.h"
 #import "RemoteLayerTreeDrawingAreaProxy.h"
 #import "PageClientImplIOS.h"
 #import "PrintInfo.h"
 #import "RemoteLayerTreeDrawingAreaProxy.h"
-#import "RemoteScrollingCoordinatorProxy.h"
 #import "SmartMagnificationController.h"
 #import "UIKitSPI.h"
 #import "VersionChecks.h"
 #import "SmartMagnificationController.h"
 #import "UIKitSPI.h"
 #import "VersionChecks.h"
@@ -404,8 +403,6 @@ static WebCore::FloatBoxExtent floatBoxExtent(UIEdgeInsets insets)
         velocityData = { 0, 0, 0, timestamp };
     }
 
         velocityData = { 0, 0, 0, timestamp };
     }
 
-    WebKit::RemoteScrollingCoordinatorProxy* scrollingCoordinator = _page->scrollingCoordinatorProxy();
-
     CGRect unobscuredContentRectRespectingInputViewBounds = [self _computeUnobscuredContentRectRespectingInputViewBounds:unobscuredContentRect inputViewBounds:inputViewBounds];
     WebCore::FloatRect fixedPositionRectForLayout = _page->computeCustomFixedPositionRect(unobscuredContentRect, unobscuredContentRectRespectingInputViewBounds, _page->customFixedPositionRect(), zoomScale, WebCore::FrameView::LayoutViewportConstraint::ConstrainedToDocumentRect);
 
     CGRect unobscuredContentRectRespectingInputViewBounds = [self _computeUnobscuredContentRectRespectingInputViewBounds:unobscuredContentRect inputViewBounds:inputViewBounds];
     WebCore::FloatRect fixedPositionRectForLayout = _page->computeCustomFixedPositionRect(unobscuredContentRect, unobscuredContentRectRespectingInputViewBounds, _page->customFixedPositionRect(), zoomScale, WebCore::FrameView::LayoutViewportConstraint::ConstrainedToDocumentRect);
 
@@ -430,12 +427,13 @@ static WebCore::FloatBoxExtent floatBoxExtent(UIEdgeInsets insets)
     LOG_WITH_STREAM(VisibleRects, stream << "-[WKContentView didUpdateVisibleRect]" << visibleContentRectUpdateInfo.dump());
 
     bool wasStableState = _page->inStableState();
     LOG_WITH_STREAM(VisibleRects, stream << "-[WKContentView didUpdateVisibleRect]" << visibleContentRectUpdateInfo.dump());
 
     bool wasStableState = _page->inStableState();
+
     _page->updateVisibleContentRects(visibleContentRectUpdateInfo);
 
     _page->updateVisibleContentRects(visibleContentRectUpdateInfo);
 
-    _sizeChangedSinceLastVisibleContentRectUpdate = NO;
+    auto layoutViewport = _page->unconstrainedLayoutViewportRect();
+    _page->adjustLayersForLayoutViewport(layoutViewport);
 
 
-    WebCore::FloatRect layoutViewport = _page->computeCustomFixedPositionRect(_page->unobscuredContentRect(), _page->unobscuredContentRectRespectingInputViewBounds(), _page->customFixedPositionRect(), zoomScale, WebCore::FrameView::LayoutViewportConstraint::Unconstrained);
-    scrollingCoordinator->viewportChangedViaDelegatedScrolling(_page->unobscuredContentRect().location(), layoutViewport, zoomScale);
+    _sizeChangedSinceLastVisibleContentRectUpdate = NO;
 
     drawingArea->updateDebugIndicator();
     
 
     drawingArea->updateDebugIndicator();
     
index c69373c..289ace5 100644 (file)
@@ -46,6 +46,7 @@
 #import "RemoteLayerTreeDrawingAreaProxy.h"
 #import "RemoteLayerTreeDrawingAreaProxyMessages.h"
 #import "RemoteLayerTreeTransaction.h"
 #import "RemoteLayerTreeDrawingAreaProxy.h"
 #import "RemoteLayerTreeDrawingAreaProxyMessages.h"
 #import "RemoteLayerTreeTransaction.h"
+#import "RemoteScrollingCoordinatorProxy.h"
 #import "UIKitSPI.h"
 #import "UserData.h"
 #import "VersionChecks.h"
 #import "UIKitSPI.h"
 #import "UserData.h"
 #import "VersionChecks.h"
@@ -274,6 +275,19 @@ WebCore::FloatRect WebPageProxy::computeCustomFixedPositionRect(const FloatRect&
     return layoutViewportRect;
 }
 
     return layoutViewportRect;
 }
 
+FloatRect WebPageProxy::unconstrainedLayoutViewportRect() const
+{
+    return computeCustomFixedPositionRect(unobscuredContentRect(), unobscuredContentRectRespectingInputViewBounds(), customFixedPositionRect(), displayedContentScale(), FrameView::LayoutViewportConstraint::Unconstrained);
+}
+
+void WebPageProxy::adjustLayersForLayoutViewport(const FloatRect& layoutViewport)
+{
+    if (!m_scrollingCoordinatorProxy)
+        return;
+
+    m_scrollingCoordinatorProxy->viewportChangedViaDelegatedScrolling(unobscuredContentRect().location(), layoutViewport, displayedContentScale());
+}
+
 void WebPageProxy::scrollingNodeScrollViewWillStartPanGesture()
 {
     pageClient().scrollingNodeScrollViewWillStartPanGesture();
 void WebPageProxy::scrollingNodeScrollViewWillStartPanGesture()
 {
     pageClient().scrollingNodeScrollViewWillStartPanGesture();