Attachment 373216 Details for Bug 199353 – Patch

[patch] Patch

bug-199353-20190701020935.patch (text/plain), 12.09 KB, created by Andy Estes on 2019-07-01 02:09:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Estes

Created: 2019-07-01 02:09:35 PDT

Size: 12.09 KB

patch

obsolete

>Subversion Revision: 246723
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 9abbd2a00665c489b8d60d4afbd3e4e75cf4b1fc..ccb9ab4a81946326c4a7d914a8b4c8d5614515c1 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-07-01  Andy Estes  <aestes@apple.com>
>+
>+        [iOS] Do not disable scrolling when content has been added to WKScrollView's margins
>+        https://bugs.webkit.org/show_bug.cgi?id=199353
>+        <rdar://problem/50822146>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _didCommitLayerTree:]):
>+        * UIProcess/ios/WKScrollView.h:
>+        * UIProcess/ios/WKScrollView.mm:
>+        (-[WKScrollView _currentTopLeftRubberbandAmount]):
>+        (-[WKScrollView _hasContentInMargins]):
>+        (-[WKScrollView layoutSubviews]):
>+
> 2019-06-23  Simon Fraser  <simon.fraser@apple.com>
> 
>         Add OverflowScrollProxyNodes to the scrolling tree
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index bbeaef4865e1da30bdf1ac443adee07108f86397..83a6ee1292cf8e5112aa3d5a89c1fd0d93450ff6 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -2020,7 +2020,8 @@ - (void)_didCommitLayerTree:(const WebKit::RemoteLayerTreeTransaction&)layerTree
> #if ENABLE(ASYNC_SCROLLING)
>     bool hasDockedInputView = !CGRectIsEmpty(_inputViewBounds);
>     bool isZoomed = layerTreeTransaction.pageScaleFactor() > layerTreeTransaction.initialScaleFactor();
>-    [_scrollView _setScrollEnabledInternal:_page->scrollingCoordinatorProxy()->hasScrollableMainFrame() || hasDockedInputView || isZoomed];
>+    bool hasContentInScrollViewMargins = [_scrollView _hasContentInMargins];
>+    [_scrollView _setScrollEnabledInternal:_page->scrollingCoordinatorProxy()->hasScrollableMainFrame() || hasDockedInputView || isZoomed || hasContentInScrollViewMargins];
> #endif
>     if (!layerTreeTransaction.scaleWasSetByUIProcess() && ![_scrollView isZooming] && ![_scrollView isZoomBouncing] && ![_scrollView _isAnimatingZoom] && [_scrollView zoomScale] != layerTreeTransaction.pageScaleFactor()) {
>         LOG_WITH_STREAM(VisibleRects, stream << " updating scroll view with pageScaleFactor " << layerTreeTransaction.pageScaleFactor());
>diff --git a/Source/WebKit/UIProcess/ios/WKScrollView.h b/Source/WebKit/UIProcess/ios/WKScrollView.h
>index 409086404f55b3d63a5fd5739bbdab9d5988bf90..1965125087f0a4c500d89bd6318aaa98bb848758 100644
>--- a/Source/WebKit/UIProcess/ios/WKScrollView.h
>+++ b/Source/WebKit/UIProcess/ios/WKScrollView.h
>@@ -42,6 +42,8 @@
> - (void)_setContentInsetAdjustmentBehaviorInternal:(UIScrollViewContentInsetAdjustmentBehavior)insetAdjustmentBehavior;
> #endif
> 
>+@property (nonatomic, readonly) BOOL _hasContentInMargins;
>+
> @end
> 
> #endif // PLATFORM(IOS_FAMILY)
>diff --git a/Source/WebKit/UIProcess/ios/WKScrollView.mm b/Source/WebKit/UIProcess/ios/WKScrollView.mm
>index 67c63b5fdcfea30f492311daafb7e132dae803fd..b8b250bee580974d1b259ad148e3a975d3afb887 100644
>--- a/Source/WebKit/UIProcess/ios/WKScrollView.mm
>+++ b/Source/WebKit/UIProcess/ios/WKScrollView.mm
>@@ -134,6 +134,7 @@ @implementation WKScrollView {
>     BOOL _scrollEnabledInternal;
>     BOOL _zoomEnabledByClient;
>     BOOL _zoomEnabledInternal;
>+    BOOL _hasContentInMargins;
> }
> 
> - (id)initWithFrame:(CGRect)frame
>@@ -302,7 +303,7 @@ - (CGSize)_currentTopLeftRubberbandAmount
> 
>     if (contentOffset.y < -edgeInsets.top)
>         rubberbandAmount.height = std::min<CGFloat>(contentOffset.y + edgeInsets.top, 0);
>-    
>+
>     return rubberbandAmount;
> }
> 
>@@ -397,6 +398,27 @@ - (void)_updateZoomability
>     [super setZoomEnabled:(_zoomEnabledByClient && _zoomEnabledInternal)];
> }
> 
>+- (BOOL)_hasContentInMargins
>+{
>+    return _hasContentInMargins;
>+}
>+
>+- (void)layoutSubviews
>+{
>+    [super layoutSubviews];
>+
>+    UIEdgeInsets contentInset = self.contentInset;
>+    UIEdgeInsets marginOutset = { -contentInset.top, -contentInset.left, -contentInset.bottom, -contentInset.right };
>+    CGRect contentRect = { CGPointMake(0, 0), self.contentSize };
>+    CGRect scrollableRect = UIEdgeInsetsInsetRect(contentRect, marginOutset);
>+
>+    CGRect boundingRect = CGRectNull;
>+    for (UIView *view in self.subviews)
>+        boundingRect = CGRectUnion(boundingRect, CGRectIntersection(scrollableRect, view.frame));
>+
>+    _hasContentInMargins = !CGRectIsEmpty(boundingRect) && !CGRectContainsRect(contentRect, boundingRect);
>+}
>+
> #if PLATFORM(WATCHOS)
> 
> - (void)addGestureRecognizer:(UIGestureRecognizer *)gestureRecognizer
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 9b1f60b6ab2eeceb791058a8a3d7526bb057d8a3..3caa7c466a9f733bd3ee6c65f67efc68c1ce3427 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-07-01  Andy Estes  <aestes@apple.com>
>+
>+        [iOS] Do not disable scrolling when content has been added to WKScrollView's margins
>+        https://bugs.webkit.org/show_bug.cgi?id=199353
>+        <rdar://problem/50822146>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:
>+        (-[ContentSizeObserver initWithScrollView:changeHandler:]):
>+        (-[ContentSizeObserver observeValueForKeyPath:ofObject:change:context:]):
>+        (-[ContentSizeObserver dealloc]):
>+        (TestWebKitAPI::TEST):
>+
> 2019-06-22  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Expand tests coverage (328 new tests -> 2652).
>diff --git a/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm b/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm
>index 8e9f41bc0c3d59949afe44f3e3577bc199b94f5a..e5e81ef2cde831a037c9f9fc0264c1cd5b8db69d 100644
>--- a/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm
>+++ b/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm
>@@ -71,6 +71,44 @@ - (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView
> }
> @end
> 
>+static NSString * const contentSizeKeyPath = @"contentSize";
>+
>+@interface ContentSizeObserver : NSObject
>+- (instancetype)init NS_UNAVAILABLE;
>+- (instancetype)initWithScrollView:(UIScrollView *)scrollView changeHandler:(void(^)())changeHandler;
>+@end
>+
>+@implementation ContentSizeObserver {
>+    void (^_changeHandler)();
>+    UIScrollView *_scrollView;
>+}
>+
>+- (instancetype)initWithScrollView:(UIScrollView *)scrollView changeHandler:(void(^)())changeHandler
>+{
>+    if (!(self = [super init]))
>+        return nil;
>+
>+    _changeHandler = [changeHandler copy];
>+    _scrollView = [scrollView retain];
>+    [_scrollView addObserver:self forKeyPath:contentSizeKeyPath options:NSKeyValueObservingOptionInitial context:nullptr];
>+    return self;
>+}
>+
>+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey, id> *)change context:(void*)context
>+{
>+    _changeHandler();
>+}
>+
>+- (void)dealloc
>+{
>+    [_scrollView removeObserver:self forKeyPath:contentSizeKeyPath context:nullptr];
>+    [_changeHandler release];
>+    [_scrollView release];
>+    [super dealloc];
>+}
>+
>+@end
>+
> namespace TestWebKitAPI {
> 
> static const CGFloat viewHeight = 500;
>@@ -206,6 +244,99 @@ TEST(ScrollViewInsetTests, RestoreInitialContentOffsetAfterCrashWithAsyncPolicyD
>     EXPECT_EQ(-400, initialContentOffset.y);
> }
> 
>+static NSString * const overflowHiddenDocumentMarkup = @"<!DOCTYPE html><meta name='viewport' content='width=device-width, initial-scale=1'><body style='width: 100%; height: 5000px; overflow: hidden;'>";
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithContentInset)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewOverlapping)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, 0, 320, 100)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewOutsideScrollableArea)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, -101, 320, 50)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewPartiallyOutsideScrollableArea)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, -99, 320, 50)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewAbove)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, -50, 320, 50)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewBelow)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, 0, 320, 50)]);
>+    auto observer = adoptNS([[ContentSizeObserver alloc] initWithScrollView:[webView scrollView] changeHandler:^{
>+        CGRect frame = [otherView frame];
>+        frame.origin.y = [webView scrollView].contentSize.height;
>+        [otherView setFrame:frame];
>+    }]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewBefore)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(-50, 0, 50, viewHeight)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(0, 50, 0, 50);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewAfter)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(320, 0, 50, viewHeight)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(0, 50, 0, 50);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
> } // namespace TestWebKitAPI
> 
> #endif // PLATFORM(IOS_FAMILY)

Actions: View | Formatted Diff | Diff

Attachments on bug 199353: 373216 | 373290