WKPDFView doesn't respect safe area insets
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Aug 2017 17:51:23 +0000 (17:51 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Aug 2017 17:51:23 +0000 (17:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175046
<rdar://problem/33558386>

Reviewed by Wenson Hsieh.

* Platform/spi/ios/UIKitSPI.h:
Add some SPI.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _setHasCustomContentView:loadedMIMEType:]):
Drive-by fix two small custom content view issues:

- Reset _scrollViewBackgroundColor; it is used to early-return from
updating the background color if it hasn't changed, but if you go from a
site with (for example) a white background, to a PDF, to another site
with a white background, we will early-return and not reset the background
color from the grey WKPDFView background.

- When installing a custom content view, scroll to the origin. We miss
the usual mechanism for scrolling to the origin when custom content views
are installed, so do it here.

(-[WKWebView _effectiveObscuredInsetEdgesAffectedBySafeArea]):
Ignore _obscuredInsetEdgesAffectedBySafeArea for custom content views.
This is fairly arbitrary, and we should find a different way to
express different edge sets for different kinds of content (perhaps
bake this into default viewports?), but for now this works.

(-[WKWebView _computedContentInset]):
Use _effectiveObscuredInsetEdgesAffectedBySafeArea instead of the ivar directly.

(-[WKWebView _safeAreaShouldAffectObscuredInsets]):
Force safe areas to not affect obscured insets for custom content views,
so that it's up to every custom content view to take safe areas into
account themselves.

* UIProcess/API/Cocoa/WKWebViewInternal.h:
Expose _computedUnobscuredSafeAreaInset for WKPDFView's use.

* UIProcess/ios/WKPDFView.mm:
(-[WKPDFView web_setMinimumSize:]):
Avoid rebuilding the WKPDFView if the minimum size didn't change.

(-[WKPDFView _offsetForPageNumberIndicator]):
Take unobscured safe area insets into account when insetting
the page number indicator.

(-[WKPDFView web_computedContentInsetDidChange]):
Watch for unobscured safe area inset changes, and apply them to the
layout of the WKPDFView.

(-[WKPDFView _computePageAndDocumentFrames]):
Make it possible to only update the WKPDFView's size and position, without
rebuilding its subviews, if the width did not change. This prevents lots
of flashing of the child UIPDFPageViews when the unobscured safe area
insets change dynamically.

Take the safe area insets into account when determining what width
to lay out to.

(-[WKPDFView _updateDocumentFrame]):
Take the safe area insets into account when positioning the WKPDFView
inside the WKScrollView.

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

Source/WebKit/ChangeLog
Source/WebKit/Platform/spi/ios/UIKitSPI.h
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit/UIProcess/ios/WKPDFView.mm

index 24622cb..2a2ca5c 100644 (file)
@@ -1,3 +1,70 @@
+2017-08-02  Tim Horton  <timothy_horton@apple.com>
+
+        WKPDFView doesn't respect safe area insets
+        https://bugs.webkit.org/show_bug.cgi?id=175046
+        <rdar://problem/33558386>
+
+        Reviewed by Wenson Hsieh.
+
+        * Platform/spi/ios/UIKitSPI.h:
+        Add some SPI.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _setHasCustomContentView:loadedMIMEType:]):
+        Drive-by fix two small custom content view issues:
+
+        - Reset _scrollViewBackgroundColor; it is used to early-return from
+        updating the background color if it hasn't changed, but if you go from a
+        site with (for example) a white background, to a PDF, to another site
+        with a white background, we will early-return and not reset the background
+        color from the grey WKPDFView background.
+
+        - When installing a custom content view, scroll to the origin. We miss
+        the usual mechanism for scrolling to the origin when custom content views
+        are installed, so do it here.
+
+        (-[WKWebView _effectiveObscuredInsetEdgesAffectedBySafeArea]):
+        Ignore _obscuredInsetEdgesAffectedBySafeArea for custom content views.
+        This is fairly arbitrary, and we should find a different way to
+        express different edge sets for different kinds of content (perhaps
+        bake this into default viewports?), but for now this works.
+
+        (-[WKWebView _computedContentInset]):
+        Use _effectiveObscuredInsetEdgesAffectedBySafeArea instead of the ivar directly.
+
+        (-[WKWebView _safeAreaShouldAffectObscuredInsets]):
+        Force safe areas to not affect obscured insets for custom content views,
+        so that it's up to every custom content view to take safe areas into
+        account themselves.
+
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        Expose _computedUnobscuredSafeAreaInset for WKPDFView's use.
+
+        * UIProcess/ios/WKPDFView.mm:
+        (-[WKPDFView web_setMinimumSize:]):
+        Avoid rebuilding the WKPDFView if the minimum size didn't change.
+
+        (-[WKPDFView _offsetForPageNumberIndicator]):
+        Take unobscured safe area insets into account when insetting
+        the page number indicator.
+
+        (-[WKPDFView web_computedContentInsetDidChange]):
+        Watch for unobscured safe area inset changes, and apply them to the
+        layout of the WKPDFView.
+
+        (-[WKPDFView _computePageAndDocumentFrames]):
+        Make it possible to only update the WKPDFView's size and position, without
+        rebuilding its subviews, if the width did not change. This prevents lots
+        of flashing of the child UIPDFPageViews when the unobscured safe area
+        insets change dynamically.
+
+        Take the safe area insets into account when determining what width
+        to lay out to.
+
+        (-[WKPDFView _updateDocumentFrame]):
+        Take the safe area insets into account when positioning the WKPDFView
+        inside the WKScrollView.
+
 2017-08-02  John Wilander  <wilander@apple.com>
 
         ResourceLoadStatisticsClassifierCocoa::singletonPredictionModel() should check the return value of storagePath()
index e7d5ade..7edb566 100644 (file)
@@ -978,4 +978,6 @@ extern const NSString *UIPreviewDataDDContext;
 extern const NSString *UIPreviewDataAttachmentList;
 extern const NSString *UIPreviewDataAttachmentIndex;
 
+UIEdgeInsets UIEdgeInsetsAdd(UIEdgeInsets lhs, UIEdgeInsets rhs, UIRectEdge);
+
 WTF_EXTERN_C_END
index a2c0ab3..3fe4985 100644 (file)
@@ -1264,6 +1264,11 @@ static CGSize roundScrollViewContentSize(const WebKit::WebPageProxy& page, CGSiz
 
         [_customContentView web_setMinimumSize:self.bounds.size];
         [_customContentView web_setFixedOverlayView:_customContentFixedOverlayView.get()];
+
+        _scrollViewBackgroundColor = WebCore::Color();
+        [_scrollView setContentOffset:[self _adjustedContentOffset:CGPointZero]];
+
+        [self _didChangeAvoidsUnsafeArea:NO];
     } else if (_customContentView) {
         [_customContentView removeFromSuperview];
         _customContentView = nullptr;
@@ -1277,6 +1282,8 @@ static CGSize roundScrollViewContentSize(const WebKit::WebPageProxy& page, CGSiz
 
         [_customContentFixedOverlayView setFrame:self.bounds];
         [self addSubview:_customContentFixedOverlayView.get()];
+
+        [self _didChangeAvoidsUnsafeArea:_page->avoidsUnsafeArea()];
     }
 
     if (self.isFirstResponder) {
@@ -1384,6 +1391,13 @@ static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
     return result;
 }
 
+- (UIRectEdge)_effectiveObscuredInsetEdgesAffectedBySafeArea
+{
+    if (![self usesStandardContentView])
+        return UIRectEdgeAll;
+    return _obscuredInsetEdgesAffectedBySafeArea;
+}
+
 - (UIEdgeInsets)_computedContentInset
 {
     if (_haveSetObscuredInsets)
@@ -1392,17 +1406,8 @@ static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
     UIEdgeInsets insets = [_scrollView contentInset];
 
 #if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
-    if (self._safeAreaShouldAffectObscuredInsets) {
-        UIEdgeInsets systemInsets = [_scrollView _systemContentInset];
-        if (_obscuredInsetEdgesAffectedBySafeArea & UIRectEdgeTop)
-            insets.top += systemInsets.top;
-        if (_obscuredInsetEdgesAffectedBySafeArea & UIRectEdgeBottom)
-            insets.bottom += systemInsets.bottom;
-        if (_obscuredInsetEdgesAffectedBySafeArea & UIRectEdgeLeft)
-            insets.left += systemInsets.left;
-        if (_obscuredInsetEdgesAffectedBySafeArea & UIRectEdgeRight)
-            insets.right += systemInsets.right;
-    }
+    if (self._safeAreaShouldAffectObscuredInsets)
+        UIEdgeInsetsAdd(insets, [_scrollView _systemContentInset], self._effectiveObscuredInsetEdgesAffectedBySafeArea);
 #endif
 
     return insets;
@@ -4416,13 +4421,6 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
     return _page->isShowingNavigationGestureSnapshot();
 }
 
-- (BOOL)_safeAreaShouldAffectObscuredInsets
-{
-    if (!_page)
-        return YES;
-    return _page->avoidsUnsafeArea();
-}
-
 - (_WKLayoutMode)_layoutMode
 {
 #if PLATFORM(MAC)
@@ -4670,6 +4668,15 @@ static inline WebKit::FindOptions toFindOptions(_WKFindOptions wkFindOptions)
     [self _scheduleVisibleContentRectUpdate];
 }
 
+- (BOOL)_safeAreaShouldAffectObscuredInsets
+{
+    if (![self usesStandardContentView])
+        return NO;
+    if (!_page)
+        return YES;
+    return _page->avoidsUnsafeArea();
+}
+
 - (void)_setInterfaceOrientationOverride:(UIInterfaceOrientation)interfaceOrientation
 {
     _overridesInterfaceOrientation = YES;
index c60cc63..4a6ad03 100644 (file)
@@ -143,6 +143,7 @@ struct PrintInfo;
 
 @property (nonatomic, readonly) BOOL _allowsDoubleTapGestures;
 @property (nonatomic, readonly) UIEdgeInsets _computedContentInset;
+@property (nonatomic, readonly) UIEdgeInsets _computedUnobscuredSafeAreaInset;
 #endif
 
 - (WKPageRef)_pageForTesting;
index ec2f331..fd0e739 100644 (file)
@@ -111,6 +111,9 @@ typedef struct {
     RetainPtr<UIWKSelectionAssistant> _webSelectionAssistant;
 
     std::unique_ptr<ApplicationStateTracker> _applicationStateTracker;
+
+    UIEdgeInsets _lastUnobscuredSafeAreaInset;
+    CGFloat _lastLayoutWidth;
 }
 
 - (instancetype)web_initWithFrame:(CGRect)frame webView:(WKWebView *)webView
@@ -205,6 +208,9 @@ static void detachViewForPage(PDFPageInfo& page)
 
 - (void)web_setMinimumSize:(CGSize)size
 {
+    if (CGSizeEqualToSize(size, _minimumSize))
+        return;
+
     _minimumSize = size;
 
     if (_webView._passwordView) {
@@ -297,8 +303,8 @@ static void detachViewForPage(PDFPageInfo& page)
 
 - (CGPoint)_offsetForPageNumberIndicator
 {
-    UIEdgeInsets contentInset = [_webView _computedContentInset];
-    return CGPointMake(contentInset.left, contentInset.top + _overlaidAccessoryViewsInset.height);
+    UIEdgeInsets insets = UIEdgeInsetsAdd(_webView._computedUnobscuredSafeAreaInset, _webView._computedContentInset, UIRectEdgeAll);
+    return CGPointMake(insets.left, insets.top + _overlaidAccessoryViewsInset.height);
 }
 
 - (void)_updatePageNumberIndicator
@@ -324,6 +330,12 @@ static void detachViewForPage(PDFPageInfo& page)
 - (void)web_computedContentInsetDidChange
 {
     [self _updatePageNumberIndicator];
+
+    if (UIEdgeInsetsEqualToEdgeInsets(_webView._computedUnobscuredSafeAreaInset, _lastUnobscuredSafeAreaInset))
+        return;
+
+    [self _computePageAndDocumentFrames];
+    [self _revalidateViews];
 }
 
 - (void)web_setFixedOverlayView:(UIView *)fixedOverlayView
@@ -370,14 +382,27 @@ static void detachViewForPage(PDFPageInfo& page)
 
 - (void)_computePageAndDocumentFrames
 {
+    UIEdgeInsets safeAreaInsets = _webView._computedUnobscuredSafeAreaInset;
+    _lastUnobscuredSafeAreaInset = safeAreaInsets;
+    CGSize minimumSizeRespectingContentInset = CGSizeMake(_minimumSize.width - (safeAreaInsets.left + safeAreaInsets.right), _minimumSize.height - (safeAreaInsets.top + safeAreaInsets.bottom));
+
+    if (!_pages.isEmpty() && _lastLayoutWidth == minimumSizeRespectingContentInset.width) {
+        [self _updateDocumentFrame];
+        return;
+    }
+
     NSUInteger pageCount = [_pdfDocument numberOfPages];
+    if (!pageCount)
+        return;
+    
+    _lastLayoutWidth = minimumSizeRespectingContentInset.width;
     [_pageNumberIndicator setPageCount:pageCount];
     
     [self _clearPages];
 
     _pages.reserveCapacity(pageCount);
 
-    CGRect pageFrame = CGRectMake(0, 0, _minimumSize.width, _minimumSize.height);
+    CGRect pageFrame = CGRectMake(0, 0, minimumSizeRespectingContentInset.width, minimumSizeRespectingContentInset.height);
     for (NSUInteger pageIndex = 0; pageIndex < pageCount; ++pageIndex) {
         UIPDFPage *page = [_pdfDocument pageAtIndex:pageIndex];
         if (!page)
@@ -395,10 +420,21 @@ static void detachViewForPage(PDFPageInfo& page)
         pageFrame.origin.y += pageFrame.size.height - pdfPageMargin;
     }
 
+    [self _updateDocumentFrame];
+}
+
+- (void)_updateDocumentFrame
+{
+    if (_pages.isEmpty())
+        return;
+
+    UIEdgeInsets safeAreaInsets = _webView._computedUnobscuredSafeAreaInset;
     CGFloat scale = _scrollView.zoomScale;
-    CGRect newFrame = [self frame];
-    newFrame.size.width = _minimumSize.width * scale;
-    newFrame.size.height = std::max(pageFrame.origin.y + pdfPageMargin, _minimumSize.height) * scale;
+    CGRect newFrame = CGRectZero;
+    newFrame.origin.x = safeAreaInsets.left;
+    newFrame.origin.y = safeAreaInsets.top;
+    newFrame.size.width = _lastLayoutWidth * scale;
+    newFrame.size.height = std::max((CGRectGetMaxY(_pages.last().frame) + pdfPageMargin) * scale + safeAreaInsets.bottom, _minimumSize.height * scale);
 
     [self setFrame:newFrame];
     [_scrollView setContentSize:newFrame.size];