Attachment 359442 Details for Bug 193494 – Patch for landing

[patch] Patch for landing

bug-193494-20190117193640.patch (text/plain), 39.74 KB, created by Wenson Hsieh on 2019-01-17 19:36:41 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-01-17 19:36:41 PST

Size: 39.74 KB

patch

obsolete

>Subversion Revision: 240102
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2dd9d50fe636288195f458eb51848519ec65a993..63a3af885851f9650a4754a4fe15f7df1ca923b8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,43 @@
>+2019-01-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas
>+        https://bugs.webkit.org/show_bug.cgi?id=193494
>+        <rdar://problem/46859627>
>+
>+        Reviewed by Simon Fraser and Tim Horton.
>+
>+        When computing the content offset to scroll to when revealing a given rect in content coordinates, we currently
>+        just use the unobscured content rect. As a result, when scrolling to reveal a rect, we'll clamp the final scroll
>+        position such that only content is visible. For example, when asked to reveal the rect `(0, 0, 1, 1)`, we adjust
>+        the scroll position to be the origin.
>+
>+        However, consider the case where a client (e.g. Mail on iOS) has added a content inset to the web view's scroll
>+        view. If we're asked to reveal a rect that is outside the content area but within a content inset, we will still
>+        end up clamping the scroll position to the unobscured rect. This manifests in a bug where selecting text and
>+        autoscrolling in iOS Mail compose while the scroll view is scrolled all the way to the top to reveal the To/Cc/
>+        Subject fields causes the content offset to jump to the origin, rather than staying at (0, -topContentInset).
>+
>+        To fix this, we teach `RenderLayer::scrollRectToVisible` about content insets that are visible. Rather than use
>+        the content rects as-is, expand to encompass visible content insets as well. This ensures that revealing a
>+        position which is already visible won't cause us to scroll away the content inset area and only show the
>+        unobscured rect.
>+
>+        Tests:  editing/selection/ios/autoscroll-with-top-content-inset.html
>+                fast/scrolling/ios/scroll-into-view-with-top-content-inset.html
>+
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::unobscuredContentRectExpandedByContentInsets const):
>+
>+        Introduce a helper method that expands the unobscured content rect to include surrounding content insets.
>+
>+        * page/FrameView.h:
>+        * page/Page.h:
>+        (WebCore::Page::contentInsets const):
>+        (WebCore::Page::setContentInsets):
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::scrollRectToVisible):
>+        (WebCore::RenderLayer::getRectToExpose const):
>+
> 2019-01-16  Justin Fan  <justin_fan@apple.com>
> 
>         [WebGPU] Update vertex-buffer-triangle-strip.html to actually use vertex buffer
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e492f8af27f8e1849826495a30f68393b87ff200..9561f9f31f0ded6cd814fb5ded88e4078011f3e8 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,48 @@
>+2019-01-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas
>+        https://bugs.webkit.org/show_bug.cgi?id=193494
>+        <rdar://problem/46859627>
>+
>+        Reviewed by Simon Fraser and Tim Horton.
>+
>+        Adds `contentInsets` to `VisibleContentRectUpdateInfo`. This keeps track of the visible content insets
>+        surrounding the unobscured content rect. See WebCore ChangeLog for more details.
>+
>+        * Shared/VisibleContentRectUpdateInfo.cpp:
>+        (WebKit::VisibleContentRectUpdateInfo::encode const):
>+        (WebKit::VisibleContentRectUpdateInfo::decode):
>+        (WebKit::operator<<):
>+        * Shared/VisibleContentRectUpdateInfo.h:
>+        (WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo):
>+        (WebKit::VisibleContentRectUpdateInfo::contentInsets const):
>+        (WebKit::operator==):
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _computedObscuredInset]):
>+        (-[WKWebView _computedContentInset]):
>+        (-[WKWebView _computedUnobscuredSafeAreaInset]):
>+
>+        We don't care about source compatibility with iOS 10 and below anymore, so we should change these >= iOS 11
>+        target checks to simply `PLATFORM(IOS)`.
>+
>+        (-[WKWebView _updateVisibleContentRects]):
>+
>+        Compute the visible content insets on all sides of the unobscured content rect. These insets are scaled to
>+        content coordinates.
>+
>+        * UIProcess/ios/WKContentView.h:
>+        * UIProcess/ios/WKContentView.mm:
>+        (floatBoxExtent):
>+
>+        Add a helper to convert `UIEdgeInsets` to `WebCore::FloatBoxExtent`, and use it in a few places below.
>+
>+        (-[WKContentView didUpdateVisibleRect:unobscuredRect:contentInsets:unobscuredRectInScrollViewCoordinates:obscuredInsets:unobscuredSafeAreaInsets:inputViewBounds:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]):
>+        (-[WKContentView didUpdateVisibleRect:unobscuredRect:unobscuredRectInScrollViewCoordinates:obscuredInsets:unobscuredSafeAreaInsets:inputViewBounds:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]): Deleted.
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::updateVisibleContentRects):
>+
>+        Update the Page's content insets.
>+
> 2019-01-16  Youenn Fablet  <youenn@apple.com>
> 
>         Add a new SPI for controlling getUserMedia
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 8dd924d797c1673bd07cc84268bb48be155d9487..06698156db4b5c210ee00868d1dc47fa8c07dc41 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -2014,6 +2014,14 @@ void FrameView::viewportContentsChanged()
> #endif
> }
> 
>+IntRect FrameView::unobscuredContentRectExpandedByContentInsets() const
>+{
>+    FloatRect unobscuredContentRect = this->unobscuredContentRect();
>+    if (auto* page = frame().page())
>+        unobscuredContentRect.expand(page->contentInsets());
>+    return IntRect(unobscuredContentRect);
>+}
>+
> bool FrameView::fixedElementsLayoutRelativeToFrame() const
> {
>     return frame().settings().fixedElementsLayoutRelativeToFrame();
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index 44298462742d52dcd7c698d4c9c84b9bce997a7f..0c781ee62a3637f37ee67bac1e8361781fb955af 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -326,6 +326,8 @@ public:
>     // Static function can be called from another thread.
>     WEBCORE_EXPORT static LayoutRect rectForViewportConstrainedObjects(const LayoutRect& visibleContentRect, const LayoutSize& totalContentsSize, float frameScaleFactor, bool fixedElementsLayoutRelativeToFrame, ScrollBehaviorForFixedElements);
> #endif
>+
>+    IntRect unobscuredContentRectExpandedByContentInsets() const;
>     
>     bool fixedElementsLayoutRelativeToFrame() const;
> 
>diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h
>index 522be986c1bb14f97a28a8a5ddaad8c29352ee96..b06531f9329c41866f91e0bc18ec9295693a0101 100644
>--- a/Source/WebCore/page/Page.h
>+++ b/Source/WebCore/page/Page.h
>@@ -347,6 +347,9 @@ public:
>     const FloatBoxExtent& obscuredInsets() const { return m_obscuredInsets; }
>     void setObscuredInsets(const FloatBoxExtent& obscuredInsets) { m_obscuredInsets = obscuredInsets; }
> 
>+    const FloatBoxExtent& contentInsets() const { return m_contentInsets; }
>+    void setContentInsets(const FloatBoxExtent& insets) { m_contentInsets = insets; }
>+
>     const FloatBoxExtent& unobscuredSafeAreaInsets() const { return m_unobscuredSafeAreaInsets; }
>     WEBCORE_EXPORT void setUnobscuredSafeAreaInsets(const FloatBoxExtent&);
> 
>@@ -791,6 +794,7 @@ private:
> 
>     float m_topContentInset { 0 };
>     FloatBoxExtent m_obscuredInsets;
>+    FloatBoxExtent m_contentInsets;
>     FloatBoxExtent m_unobscuredSafeAreaInsets;
>     FloatBoxExtent m_fullscreenInsets;
>     Seconds m_fullscreenAutoHideDuration { 0_s };
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 6c2a423b6776fc990f0a7e0f340661bccd0f237d..2f2817e0dde975dd3de9ff4b9299addffe3d1f90 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -2551,7 +2551,7 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& absoluteRect, bool insid
> #if !PLATFORM(IOS_FAMILY)
>             LayoutRect viewRect = frameView.visibleContentRect();
> #else
>-            LayoutRect viewRect = frameView.unobscuredContentRect();
>+            LayoutRect viewRect = frameView.unobscuredContentRectExpandedByContentInsets();
> #endif
>             // Move the target rect into "scrollView contents" coordinates.
>             LayoutRect targetRect = absoluteRect;
>@@ -2591,7 +2591,7 @@ void RenderLayer::updateCompositingLayersAfterScroll()
>     }
> }
> 
>-LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, const LayoutRect &exposeRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY) const
>+LayoutRect RenderLayer::getRectToExpose(const LayoutRect& visibleRect, const LayoutRect& exposeRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY) const
> {
>     FrameView& frameView = renderer().view().frameView();
>     if (renderer().isRenderView() && insideFixed) {
>diff --git a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp
>index 2ee8d96c4ea0cc0d34b22160fcdc10a09e26043e..e9a883d28bfe6671cd2856446174114d3750e169 100644
>--- a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp
>+++ b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp
>@@ -37,6 +37,7 @@ void VisibleContentRectUpdateInfo::encode(IPC::Encoder& encoder) const
> {
>     encoder << m_exposedContentRect;
>     encoder << m_unobscuredContentRect;
>+    encoder << m_contentInsets;
>     encoder << m_unobscuredContentRectRespectingInputViewBounds;
>     encoder << m_unobscuredRectInScrollViewCoordinates;
>     encoder << m_customFixedPositionRect;
>@@ -61,6 +62,8 @@ bool VisibleContentRectUpdateInfo::decode(IPC::Decoder& decoder, VisibleContentR
>         return false;
>     if (!decoder.decode(result.m_unobscuredContentRect))
>         return false;
>+    if (!decoder.decode(result.m_contentInsets))
>+        return false;
>     if (!decoder.decode(result.m_unobscuredContentRectRespectingInputViewBounds))
>         return false;
>     if (!decoder.decode(result.m_unobscuredRectInScrollViewCoordinates))
>@@ -114,6 +117,7 @@ TextStream& operator<<(TextStream& ts, const VisibleContentRectUpdateInfo& info)
> 
>     ts.dumpProperty("exposedContentRect", info.exposedContentRect());
>     ts.dumpProperty("unobscuredContentRect", info.unobscuredContentRect());
>+    ts.dumpProperty("contentInsets", info.contentInsets());
>     ts.dumpProperty("unobscuredContentRectRespectingInputViewBounds", info.unobscuredContentRectRespectingInputViewBounds());
>     ts.dumpProperty("unobscuredRectInScrollViewCoordinates", info.unobscuredRectInScrollViewCoordinates());
>     ts.dumpProperty("customFixedPositionRect", info.customFixedPositionRect());
>diff --git a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h
>index 91c1822a74acdef890fe561205fd1ba87d533ec4..814ca54c21d3f5335672e1ce23b5f6cb8d5f8eac 100644
>--- a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h
>+++ b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h
>@@ -45,9 +45,10 @@ class VisibleContentRectUpdateInfo {
> public:
>     VisibleContentRectUpdateInfo() = default;
> 
>-    VisibleContentRectUpdateInfo(const WebCore::FloatRect& exposedContentRect, const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& customFixedPositionRect, const WebCore::FloatBoxExtent& obscuredInsets, const WebCore::FloatBoxExtent& unobscuredSafeAreaInsets, double scale, bool inStableState, bool isFirstUpdateForNewViewSize, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView, MonotonicTime timestamp, double horizontalVelocity, double verticalVelocity, double scaleChangeRate, uint64_t lastLayerTreeTransactionId)
>+    VisibleContentRectUpdateInfo(const WebCore::FloatRect& exposedContentRect, const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatBoxExtent& contentInsets, const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& customFixedPositionRect, const WebCore::FloatBoxExtent& obscuredInsets, const WebCore::FloatBoxExtent& unobscuredSafeAreaInsets, double scale, bool inStableState, bool isFirstUpdateForNewViewSize, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView, MonotonicTime timestamp, double horizontalVelocity, double verticalVelocity, double scaleChangeRate, uint64_t lastLayerTreeTransactionId)
>         : m_exposedContentRect(exposedContentRect)
>         , m_unobscuredContentRect(unobscuredContentRect)
>+        , m_contentInsets(contentInsets)
>         , m_unobscuredContentRectRespectingInputViewBounds(unobscuredContentRectRespectingInputViewBounds)
>         , m_unobscuredRectInScrollViewCoordinates(unobscuredRectInScrollViewCoordinates)
>         , m_customFixedPositionRect(customFixedPositionRect)
>@@ -69,6 +70,7 @@ public:
> 
>     const WebCore::FloatRect& exposedContentRect() const { return m_exposedContentRect; }
>     const WebCore::FloatRect& unobscuredContentRect() const { return m_unobscuredContentRect; }
>+    const WebCore::FloatBoxExtent& contentInsets() const { return m_contentInsets; }
>     const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates() const { return m_unobscuredRectInScrollViewCoordinates; }
>     const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds() const { return m_unobscuredContentRectRespectingInputViewBounds; }
>     const WebCore::FloatRect& customFixedPositionRect() const { return m_customFixedPositionRect; }
>@@ -97,6 +99,7 @@ public:
> private:
>     WebCore::FloatRect m_exposedContentRect;
>     WebCore::FloatRect m_unobscuredContentRect;
>+    WebCore::FloatBoxExtent m_contentInsets;
>     WebCore::FloatRect m_unobscuredContentRectRespectingInputViewBounds;
>     WebCore::FloatRect m_unobscuredRectInScrollViewCoordinates;
>     WebCore::FloatRect m_customFixedPositionRect; // When visual viewports are enabled, this is the layout viewport.
>@@ -121,6 +124,7 @@ inline bool operator==(const VisibleContentRectUpdateInfo& a, const VisibleConte
>     return a.scale() == b.scale()
>         && a.exposedContentRect() == b.exposedContentRect()
>         && a.unobscuredContentRect() == b.unobscuredContentRect()
>+        && a.contentInsets() == b.contentInsets()
>         && a.unobscuredContentRectRespectingInputViewBounds() == b.unobscuredContentRectRespectingInputViewBounds()
>         && a.customFixedPositionRect() == b.customFixedPositionRect()
>         && a.obscuredInsets() == b.obscuredInsets()
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 4a441b0cdd8a6d7d15d8f1a1f3ae05fc8b61cb56..ae0e250b40f7eebf76929c2df487e554d474bfa6 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -1734,7 +1734,7 @@ - (UIEdgeInsets)_computedObscuredInset
>     if (_haveSetObscuredInsets)
>         return _obscuredInsets;
> 
>-#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
>+#if PLATFORM(IOS)
>     if (self._safeAreaShouldAffectObscuredInsets)
>         return UIEdgeInsetsAdd(UIEdgeInsetsZero, self._scrollViewSystemContentInset, self._effectiveObscuredInsetEdgesAffectedBySafeArea);
> #endif
>@@ -1749,7 +1749,7 @@ - (UIEdgeInsets)_computedContentInset
> 
>     UIEdgeInsets insets = [_scrollView contentInset];
> 
>-#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
>+#if PLATFORM(IOS)
>     if (self._safeAreaShouldAffectObscuredInsets)
>         insets = UIEdgeInsetsAdd(insets, self._scrollViewSystemContentInset, self._effectiveObscuredInsetEdgesAffectedBySafeArea);
> #endif
>@@ -1762,7 +1762,7 @@ - (UIEdgeInsets)_computedUnobscuredSafeAreaInset
>     if (_haveSetUnobscuredSafeAreaInsets)
>         return _unobscuredSafeAreaInsets;
> 
>-#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
>+#if PLATFORM(IOS)
>     if (!self._safeAreaShouldAffectObscuredInsets)
>         return self.safeAreaInsets;
> #endif
>@@ -2975,6 +2975,26 @@ - (void)_updateVisibleContentRects
>     CGRect unobscuredRectInContentCoordinates = _frozenUnobscuredContentRect ? _frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()];
>     unobscuredRectInContentCoordinates = CGRectIntersection(unobscuredRectInContentCoordinates, [self _contentBoundsExtendedForRubberbandingWithScale:scaleFactor]);
> 
>+    // The following logic computes the extent to which the bottom, top, left and right content insets are visible.
>+    auto scrollViewInsets = [_scrollView contentInset];
>+    auto scrollViewBounds = [_scrollView bounds];
>+    auto scrollViewContentSize = [_scrollView contentSize];
>+    auto scrollViewOriginIncludingInset = UIEdgeInsetsInsetRect(scrollViewBounds, computedContentInsetUnadjustedForKeyboard).origin;
>+    auto maximumVerticalScrollExtentWithoutRevealingBottomContentInset = scrollViewContentSize.height - CGRectGetHeight(scrollViewBounds);
>+    auto maximumHorizontalScrollExtentWithoutRevealingRightContentInset = scrollViewContentSize.width - CGRectGetWidth(scrollViewBounds);
>+    auto contentInsets = UIEdgeInsetsZero;
>+    if (scrollViewInsets.left > 0 && scrollViewOriginIncludingInset.x < 0)
>+        contentInsets.left = std::min(-scrollViewOriginIncludingInset.x, scrollViewInsets.left) / scaleFactor;
>+
>+    if (scrollViewInsets.top > 0 && scrollViewOriginIncludingInset.y < 0)
>+        contentInsets.top = std::min(-scrollViewOriginIncludingInset.y, scrollViewInsets.top) / scaleFactor;
>+
>+    if (scrollViewInsets.right > 0 && scrollViewOriginIncludingInset.x > maximumHorizontalScrollExtentWithoutRevealingRightContentInset)
>+        contentInsets.right = std::min(scrollViewOriginIncludingInset.x - maximumHorizontalScrollExtentWithoutRevealingRightContentInset, scrollViewInsets.right) / scaleFactor;
>+
>+    if (scrollViewInsets.bottom > 0 && scrollViewOriginIncludingInset.y > maximumVerticalScrollExtentWithoutRevealingBottomContentInset)
>+        contentInsets.bottom = std::min(scrollViewOriginIncludingInset.y - maximumVerticalScrollExtentWithoutRevealingBottomContentInset, scrollViewInsets.bottom) / scaleFactor;
>+
> #if ENABLE(CSS_SCROLL_SNAP) && ENABLE(ASYNC_SCROLLING)
>     if (inStableState) {
>         WebKit::RemoteScrollingCoordinatorProxy* coordinator = _page->scrollingCoordinatorProxy();
>@@ -2994,6 +3014,7 @@ - (void)_updateVisibleContentRects
> 
>     [_contentView didUpdateVisibleRect:visibleRectInContentCoordinates
>         unobscuredRect:unobscuredRectInContentCoordinates
>+        contentInsets:contentInsets
>         unobscuredRectInScrollViewCoordinates:unobscuredRect
>         obscuredInsets:_obscuredInsets
>         unobscuredSafeAreaInsets:[self _computedUnobscuredSafeAreaInset]
>diff --git a/Source/WebKit/UIProcess/ios/WKContentView.h b/Source/WebKit/UIProcess/ios/WKContentView.h
>index a0607e3d1abc75289817991a4b2e61ef8292c169..522aca19d02b089688cc823233c9e01e683ebc56 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentView.h
>+++ b/Source/WebKit/UIProcess/ios/WKContentView.h
>@@ -71,6 +71,7 @@ class WebProcessPool;
> 
> - (void)didUpdateVisibleRect:(CGRect)visibleRect
>     unobscuredRect:(CGRect)unobscuredRect
>+    contentInsets:(UIEdgeInsets)contentInsets
>     unobscuredRectInScrollViewCoordinates:(CGRect)unobscuredRectInScrollViewCoordinates
>     obscuredInsets:(UIEdgeInsets)obscuredInsets
>     unobscuredSafeAreaInsets:(UIEdgeInsets)unobscuredSafeAreaInsets
>diff --git a/Source/WebKit/UIProcess/ios/WKContentView.mm b/Source/WebKit/UIProcess/ios/WKContentView.mm
>index ced85e8c9918a7b567167d97ae88db60fafaf3a5..304c23f401bff648e83286d23f3db5e5e4225508 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentView.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentView.mm
>@@ -365,6 +365,11 @@ - (void)_didExitStableState
>     [_textSelectionAssistant deactivateSelection];
> }
> 
>+static WebCore::FloatBoxExtent floatBoxExtent(UIEdgeInsets insets)
>+{
>+    return WebCore::FloatBoxExtent(insets.top, insets.right, insets.bottom, insets.left);
>+}
>+
> - (CGRect)_computeUnobscuredContentRectRespectingInputViewBounds:(CGRect)unobscuredContentRect inputViewBounds:(CGRect)inputViewBounds
> {
>     // The input view bounds are in window coordinates, but the unobscured rect is in content coordinates. Account for this by converting input view bounds to content coordinates.
>@@ -376,6 +381,7 @@ - (CGRect)_computeUnobscuredContentRectRespectingInputViewBounds:(CGRect)unobscu
> 
> - (void)didUpdateVisibleRect:(CGRect)visibleContentRect
>     unobscuredRect:(CGRect)unobscuredContentRect
>+    contentInsets:(UIEdgeInsets)contentInsets
>     unobscuredRectInScrollViewCoordinates:(CGRect)unobscuredRectInScrollViewCoordinates
>     obscuredInsets:(UIEdgeInsets)obscuredInsets
>     unobscuredSafeAreaInsets:(UIEdgeInsets)unobscuredSafeAreaInsets
>@@ -404,11 +410,12 @@ - (void)didUpdateVisibleRect:(CGRect)visibleContentRect
>     WebKit::VisibleContentRectUpdateInfo visibleContentRectUpdateInfo(
>         visibleContentRect,
>         unobscuredContentRect,
>+        floatBoxExtent(contentInsets),
>         unobscuredRectInScrollViewCoordinates,
>         unobscuredContentRectRespectingInputViewBounds,
>         fixedPositionRectForLayout,
>-        WebCore::FloatBoxExtent(obscuredInsets.top, obscuredInsets.right, obscuredInsets.bottom, obscuredInsets.left),
>-        WebCore::FloatBoxExtent(unobscuredSafeAreaInsets.top, unobscuredSafeAreaInsets.right, unobscuredSafeAreaInsets.bottom, unobscuredSafeAreaInsets.left),
>+        floatBoxExtent(obscuredInsets),
>+        floatBoxExtent(unobscuredSafeAreaInsets),
>         zoomScale,
>         isStableState,
>         _sizeChangedSinceLastVisibleContentRectUpdate,
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 06696c065cc7748c1cbde00545e37364ea694e53..9b09a0afcbb2aa86700f7da2b54ea97875cb0cea 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -3016,6 +3016,7 @@ void WebPage::updateVisibleContentRects(const VisibleContentRectUpdateInfo& visi
>         viewportConfigurationChanged();
> 
>     frameView.setUnobscuredContentSize(visibleContentRectUpdateInfo.unobscuredContentRect().size());
>+    m_page->setContentInsets(visibleContentRectUpdateInfo.contentInsets());
>     m_page->setObscuredInsets(visibleContentRectUpdateInfo.obscuredInsets());
>     m_page->setUnobscuredSafeAreaInsets(visibleContentRectUpdateInfo.unobscuredSafeAreaInsets());
>     m_page->setEnclosedInScrollableAncestorView(visibleContentRectUpdateInfo.enclosedInScrollableAncestorView());
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 077d393294b53bee78a3c494e8655d76d98290d8..18edb5f98eb86cd76f8e45185203b02abec3f83a 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-01-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas
>+        https://bugs.webkit.org/show_bug.cgi?id=193494
>+        <rdar://problem/46859627>
>+
>+        Reviewed by Simon Fraser and Tim Horton.
>+
>+        Add a new test option to add a top content inset to the test runner's WKWebView's scroll view, and automatically
>+        scroll to reveal this top content inset area when beginning the test (i.e., scroll to (0, -topContentInset)).
>+
>+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::contentOffsetX const):
>+        (WTR::UIScriptController::contentOffsetY const):
>+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
>+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
>+        (WTR::UIScriptController::contentOffsetX const):
>+        (WTR::UIScriptController::contentOffsetY const):
>+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
>+
>+        Also add new UIScriptController methods to ask for the content offset of the platform scroll view.
>+
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::updateTestOptionsFromTestHeader):
>+        * WebKitTestRunner/TestOptions.h:
>+        (WTR::TestOptions::hasSameInitializationOptions const):
>+        * WebKitTestRunner/ios/TestControllerIOS.mm:
>+        (WTR::TestController::platformResetStateToConsistentValues):
>+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::contentOffsetX const):
>+        (WTR::UIScriptController::contentOffsetY const):
>+
> 2019-01-16  Youenn Fablet  <youenn@apple.com>
> 
>         Add a new SPI for controlling getUserMedia
>diff --git a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>index 398034a4c20e39acaef269ce94589484824c2b96..8a1125f4ef9198452669e0419c589a8902e8a670 100644
>--- a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>+++ b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>@@ -211,6 +211,16 @@ static CGPoint contentOffsetBoundedInValidRange(UIScrollView *scrollView, CGPoin
>     return contentOffset;
> }
> 
>+double UIScriptController::contentOffsetX() const
>+{
>+    return [gWebScrollView contentOffset].x;
>+}
>+
>+double UIScriptController::contentOffsetY() const
>+{
>+    return [gWebScrollView contentOffset].y;
>+}
>+
> void UIScriptController::scrollToOffset(long x, long y)
> {
>     [gWebScrollView setContentOffset:contentOffsetBoundedInValidRange(gWebScrollView, CGPointMake(x, y)) animated:YES];
>diff --git a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>index 7d204e85726a86d3fa8ec2c38b9778c13b706623..5424ba8a8cfbe4ad814c0dabe6117337eedf967c 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>+++ b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>@@ -231,6 +231,9 @@ interface UIScriptController {
> 
>     void resignFirstResponder();
> 
>+    readonly attribute double contentOffsetX;
>+    readonly attribute double contentOffsetY;
>+
>     void scrollToOffset(long x, long y); // Initiate an animated scroll in the UI process.
>     attribute object didEndScrollingCallback;
> 
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>index c9587c42e9f03efa8994d54d39019e7678ec6b78..a5baa06cd533200599f3af56893e375f6e0233c1 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>@@ -336,6 +336,16 @@ JSRetainPtr<JSStringRef> UIScriptController::formInputLabel() const
>     return nullptr;
> }
> 
>+double UIScriptController::contentOffsetX() const
>+{
>+    return 0;
>+}
>+
>+double UIScriptController::contentOffsetY() const
>+{
>+    return 0;
>+}
>+
> void UIScriptController::scrollToOffset(long x, long y)
> {
> }
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>index a6482c9e7940a9e5745fb026406408d0b4e3458e..d3fb1cbb09250a8279dfcb28bbf8c9027f7021cb 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>@@ -113,6 +113,9 @@ public:
>     JSObjectRef contentsOfUserInterfaceItem(JSStringRef) const;
>     void overridePreference(JSStringRef preference, JSStringRef value);
>     
>+    double contentOffsetX() const;
>+    double contentOffsetY() const;
>+
>     void scrollToOffset(long x, long y);
> 
>     void immediateScrollToOffset(long x, long y);
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 7c88e0027f3d750be61567232a881adcdfb4dda8..926177ef2c7729b48a3bc037a41a1b6f2c4382d3 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -1266,6 +1266,8 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std:
>             testOptions.editable = parseBooleanTestHeaderValue(value);
>         else if (key == "enableUndoManagerAPI")
>             testOptions.enableUndoManagerAPI = parseBooleanTestHeaderValue(value);
>+        else if (key == "contentInset.top")
>+            testOptions.contentInsetTop = std::stod(value);
>         pairStart = pairEnd + 1;
>     }
> }
>diff --git a/Tools/WebKitTestRunner/TestOptions.h b/Tools/WebKitTestRunner/TestOptions.h
>index 10cd4e4c34571b0b1b8a02cb6dd479c67fec7340..614ce7b5818597ea9a4813adc7b2e1fc46a46079 100644
>--- a/Tools/WebKitTestRunner/TestOptions.h
>+++ b/Tools/WebKitTestRunner/TestOptions.h
>@@ -69,6 +69,8 @@ struct TestOptions {
>     bool editable { false };
>     bool enableUndoManagerAPI { false };
> 
>+    double contentInsetTop { 0 };
>+
>     float deviceScaleFactor { 1 };
>     Vector<String> overrideLanguages;
>     std::string applicationManifest;
>@@ -113,7 +115,8 @@ struct TestOptions {
>             || shouldIgnoreMetaViewport != options.shouldIgnoreMetaViewport
>             || enableEditableImages != options.enableEditableImages
>             || editable != options.editable
>-            || enableUndoManagerAPI != options.enableUndoManagerAPI)
>+            || enableUndoManagerAPI != options.enableUndoManagerAPI
>+            || contentInsetTop != options.contentInsetTop)
>             return false;
> 
>         if (experimentalFeatures != options.experimentalFeatures)
>diff --git a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>index 4a1080cad49df10dc1b4446705c076c1e0afbb4d..855d2d5bc700340474c4271cc7b7aaa951020043 100644
>--- a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>@@ -132,7 +132,8 @@ void TestController::platformResetStateToConsistentValues(const TestOptions& opt
>         UIScrollView *scrollView = webView.scrollView;
>         [scrollView _removeAllAnimations:YES];
>         [scrollView setZoomScale:1 animated:NO];
>-        [scrollView setContentOffset:CGPointZero];
>+        scrollView.contentInset = UIEdgeInsetsMake(options.contentInsetTop, 0, 0, 0);
>+        scrollView.contentOffset = CGPointMake(0, -options.contentInsetTop);
> 
>         if (webView.interactingWithFormControl)
>             shouldRestoreFirstResponder = [webView resignFirstResponder];
>diff --git a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>index 9e20477b1e868267f49c4a0092b0ad58c3d49186..4aa94290578748cf81bc5355076dfa27a8a81c5d 100644
>--- a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>@@ -487,6 +487,18 @@ static CGPoint contentOffsetBoundedInValidRange(UIScrollView *scrollView, CGPoin
>     return contentOffset;
> }
> 
>+double UIScriptController::contentOffsetX() const
>+{
>+    TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>+    return webView.scrollView.contentOffset.x;
>+}
>+
>+double UIScriptController::contentOffsetY() const
>+{
>+    TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>+    return webView.scrollView.contentOffset.y;
>+}
>+
> void UIScriptController::scrollToOffset(long x, long y)
> {
>     TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index e856254f6b5005505f8a1dd3c3c961e87901d7c1..738272c3fc9e3e95049e6c9a370340d688a578ec 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-01-17  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas
>+        https://bugs.webkit.org/show_bug.cgi?id=193494
>+        <rdar://problem/46859627>
>+
>+        Reviewed by Simon Fraser and Tim Horton.
>+
>+        * editing/selection/ios/autoscroll-with-top-content-inset-expected.txt: Added.
>+        * editing/selection/ios/autoscroll-with-top-content-inset.html: Added.
>+
>+        Add a new test to verify that moving the selection by autoscrolling near the top content inset area does not
>+        cause the scroll view's content offset to jump.
>+
>+        * fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt: Added.
>+        * fast/scrolling/ios/scroll-into-view-with-top-content-inset.html: Added.
>+
>+        Add a new test to verify that programmatically scrolling an element that's already visible into view does not
>+        scroll away the scroll view's content inset.
>+
>+        * resources/ui-helper.js:
>+        (window.UIHelper.contentOffset):
>+        (window.UIHelper):
>+
> 2019-01-16  Alicia Boya GarcÃa  <aboya@igalia.com>
> 
>         Unreviewed GTK and WPE test gardening.
>diff --git a/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-expected.txt b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..3ee5cff85a82615cd35321e10fd0beeb24c74db2
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-expected.txt
>@@ -0,0 +1,13 @@
>+Select me and drag up.
>+
>+Verifies that triggering autoscroll near the top of a web view with a top content inset does not cause the scroll view's content offset to jump to 0. This test requires WebKitTestRunner. To verify manually, load this page in a web view that has a scroll view top content inset, and scroll such that the full top content inset area is visible. Check that starting a text selection loupe gesture near the top of the top of the document and dragging upwards does not thrash the scroll view's content offset.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS finalContentOffset.x is 0
>+PASS verticalMovementDuringDrag < 1 is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset.html b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f4337ef0f41d03294f9a843a6e2eccb25e6efc85
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset.html
>@@ -0,0 +1,51 @@
>+<!DOCTYPE html> 
>+<html>
>+<meta name="viewport" content="width=device-width, initial-scale=1">
>+<head>
>+    <script src="../../../resources/basic-gestures.js"></script>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <script src="../../../resources/js-test.js"></script>
>+    <style>
>+    body {
>+        margin: 0;
>+        box-sizing: border-box;
>+        border: red 1px solid;
>+    }
>+
>+    #text {
>+        font-size: 30px;
>+    }
>+
>+    #console {
>+        overflow: scroll;
>+        height: 100px;
>+    }
>+    </style>
>+    <script>
>+    jsTestIsAsync = true;
>+
>+    async function run()
>+    {
>+        await UIHelper.activateAndWaitForInputSessionAt(110, 40);
>+        originalContentOffset = await UIHelper.contentOffset();
>+        await longPressAndHoldAtPoint(110, 40);
>+        await touchAndDragFromPointToPoint(110, 40, 210, 40);
>+        await liftUpAtPoint(210, 40);
>+        finalContentOffset = await UIHelper.contentOffset();
>+        verticalMovementDuringDrag = Math.abs(finalContentOffset.y - originalContentOffset.y);
>+
>+        shouldBe("finalContentOffset.x", "0");
>+        shouldBeTrue("verticalMovementDuringDrag < 1");
>+        finishJSTest();
>+    }
>+    </script>
>+</head>
>+<body contenteditable onload="run()">
>+    <p id="text"><strong>Select me and drag up.</strong></p>
>+    <p id="description"></p>
>+    <p id="console"></p>
>+</body>
>+<script>
>+    description("Verifies that triggering autoscroll near the top of a web view with a top content inset does not cause the scroll view's content offset to jump to 0. This test requires WebKitTestRunner. To verify manually, load this page in a web view that has a scroll view top content inset, and scroll such that the full top content inset area is visible. Check that starting a text selection loupe gesture near the top of the top of the document and dragging upwards does not thrash the scroll view's content offset.");
>+</script>
>+</html>
>diff --git a/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..f7f479c8583cc6763ade6f7a28c451b90b245c46
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt
>@@ -0,0 +1,13 @@
>+Verifies that Element.scrollIntoView() does not scroll away the top content inset if the element is already visible. This test requires WebKitTestRunner.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS originalContentOffset.x is 0
>+PASS originalContentOffset.y is -100
>+PASS finalContentOffset.x is 0
>+PASS finalContentOffset.y is -100
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset.html b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..fd77ef6f5130b3bba75ab08f6b7ab14e86cd9e39
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset.html
>@@ -0,0 +1,47 @@
>+<!DOCTYPE html> 
>+<html>
>+<meta name="viewport" content="width=device-width, initial-scale=1">
>+<head>
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <script src="../../../resources/js-test.js"></script>
>+    <style>
>+    body {
>+        margin: 0;
>+        border: red 1px solid;
>+    }
>+
>+    #target {
>+        position: absolute;
>+        width: 4px;
>+        height: 4px;
>+        top: 0px;
>+        left: 0px;
>+        background-color: silver;
>+    }
>+    </style>
>+    <script>
>+    jsTestIsAsync = true;
>+
>+    async function run()
>+    {
>+        originalContentOffset = await UIHelper.contentOffset();
>+        target.scrollIntoView({ block: "nearest", inline: "nearest" });
>+        finalContentOffset = await UIHelper.contentOffset();
>+
>+        shouldBe("originalContentOffset.x", "0");
>+        shouldBe("originalContentOffset.y", "-100");
>+        shouldBe("finalContentOffset.x", "0");
>+        shouldBe("finalContentOffset.y", "-100");
>+        finishJSTest();
>+    }
>+    </script>
>+</head>
>+<body contenteditable onload="run()">
>+    <div id="target"></div>
>+    <p id="description"></p>
>+    <p id="console"></p>
>+</body>
>+<script>
>+    description("Verifies that Element.scrollIntoView() does not scroll away the top content inset if the element is already visible. This test requires WebKitTestRunner.");
>+</script>
>+</html>
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 2f569aacc55c45c7025ce216a70345729224bc6e..e65f031d640d321f8aa098c3cf3a3e0e46e47309 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -504,4 +504,16 @@ window.UIHelper = class UIHelper {
>         const escapedIdentifier = identifier.replace(/`/g, "\\`");
>         return new Promise(resolve => testRunner.runUIScript(`uiController.setKeyboardInputModeIdentifier(\`${escapedIdentifier}\`)`, resolve));
>     }
>+
>+    static contentOffset()
>+    {
>+        if (!this.isIOS())
>+            return Promise.resolve();
>+
>+        const uiScript = "JSON.stringify([uiController.contentOffsetX, uiController.contentOffsetY])";
>+        return new Promise(resolve => testRunner.runUIScript(uiScript, result => {
>+            const [offsetX, offsetY] = JSON.parse(result)
>+            resolve({ x: offsetX, y: offsetY });
>+        }));
>+    }
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 193494: 359379 | 359437 | 359441 | 359442