Attachment 359379 Details for Bug 193494 – Patch

[patch] Patch

bug-193494-20190117082225.patch (text/plain), 35.55 KB, created by Wenson Hsieh on 2019-01-17 08:22:26 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-01-17 08:22:26 PST

Size: 35.55 KB

patch

obsolete

>Subversion Revision: 240102
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 2dd9d50fe636288195f458eb51848519ec65a993..75491dd66f3672da2eb8677bdc2c67765e82d828 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,39 @@
>+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 NOBODY (OOPS!).
>+
>+        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 revealing a rect that's located outside of the content
>+        area, we'll clamp the final scroll position such that only content is visible. For example, when asked to reveal
>+        the rect `(0, -100, 1, 1)`, we adjust the scroll position to be the origin rather than `(0, -100)`.
>+
>+        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 unobscured content insets (i.e. content insets
>+        that are visible). Rather than use the content rects as-is, expand to encompass unobscured 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.
>+
>+        Test: editing/selection/ios/autoscroll-with-top-content-inset.html
>+
>+        * page/Page.h:
>+        (WebCore::Page::unobscuredContentInsets const):
>+        (WebCore::Page::setUnobscuredContentInsets):
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::scrollRectToVisible):
>+
>+        Expand the unobscured content rect to include surrounding unobscured insets.
>+
>+        (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..004fd34af0e67c1c38c39e77aec52d34359e6b91 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 NOBODY (OOPS!).
>+
>+        Adds `unobscuredContentInsets` to `VisibleContentRectUpdateInfo`. This keeps track of the visible inset areas
>+        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::unobscuredContentInsets const):
>+        (WebKit::operator==):
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _computedObscuredInset]):
>+        (-[WKWebView _computedContentInset]):
>+        (-[WKWebView _computedUnobscuredSafeAreaInset]):
>+
>+        We don't care about binary compatibility with iOS 10 and below anymore, so we should change these >= iOS 11
>+        target checks to simply `PLATFORM(IOS)`.
>+
>+        (-[WKWebView _updateVisibleContentRects]):
>+
>+        Compute the unobscured content insets on all sides of the unobscured content rect. These unobscured content
>+        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:unobscuredInsets: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 unobscured content insets.
>+
> 2019-01-16  Youenn Fablet  <youenn@apple.com>
> 
>         Add a new SPI for controlling getUserMedia
>diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h
>index 522be986c1bb14f97a28a8a5ddaad8c29352ee96..995ec79a067882626c9017fd26499ece70781811 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& unobscuredContentInsets() const { return m_unobscuredContentInsets; };
>+    void setUnobscuredContentInsets(const FloatBoxExtent& insets) { m_unobscuredContentInsets = 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_unobscuredContentInsets;
>     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..f21ff41731266d1ed287fd5dbe22d37cdca1a3bb 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -2553,6 +2553,11 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& absoluteRect, bool insid
> #else
>             LayoutRect viewRect = frameView.unobscuredContentRect();
> #endif
>+            // Even though content insets do not represent content, we consider them as part of the visible rect
>+            // to handle the case where the rect to reveal is outside the content, but within the inset area.
>+            auto& unobscuredInsets = page().unobscuredContentInsets();
>+            viewRect.expand({ unobscuredInsets.top(), unobscuredInsets.right(), unobscuredInsets.bottom(), unobscuredInsets.left() });
>+
>             // Move the target rect into "scrollView contents" coordinates.
>             LayoutRect targetRect = absoluteRect;
>             targetRect.move(0, frameView.headerHeight());
>@@ -2591,7 +2596,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..cc2514e5de8e5b88ea2025d562943758876418d6 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_unobscuredContentInsets;
>     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_unobscuredContentInsets))
>+        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("unobscuredContentInsets", info.unobscuredContentInsets());
>     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..6ed79196573a05d23b2f8acb627bb6ab0487c7a6 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& unobscuredContentInsets, 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_unobscuredContentInsets(unobscuredContentInsets)
>         , 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& unobscuredContentInsets() const { return m_unobscuredContentInsets; }
>     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_unobscuredContentInsets;
>     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.unobscuredContentInsets() == b.unobscuredContentInsets()
>         && 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..5eb9013d45a3dc535f5ba687f8a6a1bf1039df65 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,23 @@ - (void)_updateVisibleContentRects
>     CGRect unobscuredRectInContentCoordinates = _frozenUnobscuredContentRect ? _frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()];
>     unobscuredRectInContentCoordinates = CGRectIntersection(unobscuredRectInContentCoordinates, [self _contentBoundsExtendedForRubberbandingWithScale:scaleFactor]);
> 
>+    auto scrollViewInsets = [_scrollView contentInset];
>+    auto scrollViewOriginIncludingInset = UIEdgeInsetsInsetRect([_scrollView bounds], computedContentInsetUnadjustedForKeyboard).origin;
>+    auto maximumVerticalScrollExtentWithoutRevealingBottomContentInset = [_scrollView contentSize].height - CGRectGetHeight([_scrollView bounds]);
>+    auto maximumHorizontalScrollExtentWithoutRevealingRightContentInset = [_scrollView contentSize].width - CGRectGetWidth([_scrollView bounds]);
>+    auto unobscuredContentInsets = UIEdgeInsetsZero;
>+    if (scrollViewInsets.left > 0 && scrollViewOriginIncludingInset.x < 0)
>+        unobscuredContentInsets.left = std::min(-scrollViewOriginIncludingInset.x, scrollViewInsets.left) / scaleFactor;
>+
>+    if (scrollViewInsets.top > 0 && scrollViewOriginIncludingInset.y < 0)
>+        unobscuredContentInsets.top = std::min(-scrollViewOriginIncludingInset.y, scrollViewInsets.top) / scaleFactor;
>+
>+    if (scrollViewInsets.right > 0 && scrollViewOriginIncludingInset.x > maximumHorizontalScrollExtentWithoutRevealingRightContentInset)
>+        unobscuredContentInsets.right = std::min(scrollViewOriginIncludingInset.x - maximumHorizontalScrollExtentWithoutRevealingRightContentInset, scrollViewInsets.right) / scaleFactor;
>+
>+    if (scrollViewInsets.bottom > 0 && scrollViewOriginIncludingInset.y > maximumVerticalScrollExtentWithoutRevealingBottomContentInset)
>+        unobscuredContentInsets.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 +3011,7 @@ - (void)_updateVisibleContentRects
> 
>     [_contentView didUpdateVisibleRect:visibleRectInContentCoordinates
>         unobscuredRect:unobscuredRectInContentCoordinates
>+        unobscuredInsets:unobscuredContentInsets
>         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..463a927bd8f47064953bddf7e967a05f3e39d277 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
>+    unobscuredInsets:(UIEdgeInsets)unobscuredInsets
>     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..202a9704bc3aff74eeaaddecb7fa0b430141090e 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
>+    unobscuredInsets:(UIEdgeInsets)unobscuredInsets
>     unobscuredRectInScrollViewCoordinates:(CGRect)unobscuredRectInScrollViewCoordinates
>     obscuredInsets:(UIEdgeInsets)obscuredInsets
>     unobscuredSafeAreaInsets:(UIEdgeInsets)unobscuredSafeAreaInsets
>@@ -404,11 +410,12 @@ - (void)didUpdateVisibleRect:(CGRect)visibleContentRect
>     WebKit::VisibleContentRectUpdateInfo visibleContentRectUpdateInfo(
>         visibleContentRect,
>         unobscuredContentRect,
>+        floatBoxExtent(unobscuredInsets),
>         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..541bc9960a65fd2d0c37e5206045cf16f25ec708 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->setUnobscuredContentInsets(visibleContentRectUpdateInfo.unobscuredContentInsets());
>     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..36447b9d34cbc4e14eb64391bdb63b159576dd64 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 NOBODY (OOPS!).
>+
>+        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..96ca6f58ff3b40d6c8e5807ed3d21776abed9ec0 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 == "topContentInset")
>+            testOptions.topContentInset = std::stod(value);
>         pairStart = pairEnd + 1;
>     }
> }
>diff --git a/Tools/WebKitTestRunner/TestOptions.h b/Tools/WebKitTestRunner/TestOptions.h
>index 10cd4e4c34571b0b1b8a02cb6dd479c67fec7340..bc1f6b02fb99e772f511b17e094b2fe925d1a501 100644
>--- a/Tools/WebKitTestRunner/TestOptions.h
>+++ b/Tools/WebKitTestRunner/TestOptions.h
>@@ -69,6 +69,8 @@ struct TestOptions {
>     bool editable { false };
>     bool enableUndoManagerAPI { false };
> 
>+    double topContentInset { 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
>+            || topContentInset != options.topContentInset)
>             return false;
> 
>         if (experimentalFeatures != options.experimentalFeatures)
>diff --git a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm
>index 4a1080cad49df10dc1b4446705c076c1e0afbb4d..610c97fd4fe5677d2595b27426e90255bc5e7034 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.topContentInset, 0, 0, 0);
>+        scrollView.contentOffset = CGPointMake(0, -options.topContentInset);
> 
>         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..5ca600d1401f9a242e13c9acda49b5bc257e2555 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,20 @@
>+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 NOBODY (OOPS!).
>+
>+        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.
>+
>+        * editing/selection/ios/autoscroll-with-top-content-inset-expected.txt: Added.
>+        * editing/selection/ios/autoscroll-with-top-content-inset.html: Added.
>+        * 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..2259ca2dcf962ea2faa8d26c71b7e20963861e91
>--- /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/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 });
>+        }));
>+    }
> }

Flags:

simon.fraser: review-

Actions: View | Formatted Diff | Diff

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