Title: [233561] trunk/Source/WebKit
Revision
233561
Author
[email protected]
Date
2018-07-05 21:56:40 -0700 (Thu, 05 Jul 2018)

Log Message

Address two possible causes of missing tiles in iOS Safari, and add logging to gather more data about other possible causes
https://bugs.webkit.org/show_bug.cgi?id=187376
rdar://problem/40941118

Reviewed by Tim Horton.

We have continual reports of users experiencing missing tiles in MobileSafari, where loading a page
shows the tiles at the top, but we don't render new tiles as the user scrolls down. This is consistent
with failing to dispatch visible content rect updates via -[WKWebView _updateVisibleContentRects].

This patch addresses two possible (but unlikely) causes. First, it resets _currentlyAdjustingScrollViewInsetsForKeyboard
after a web process crash. Second, it catches exceptions thrown by [webView _updateVisibleContentRects]
and resets _hasScheduledVisibleRectUpdate.

This patch also adds release logging that fires if over 1s has elapsed between scheduling
a visible content rect update and trying to re-schedule, and logging for all reasons that
-_updateVisibleContentRects returns early.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _initializeWithConfiguration:]):
(-[WKWebView _processDidExit]):
(-[WKWebView _addUpdateVisibleContentRectPreCommitHandler]):
(-[WKWebView _scheduleVisibleContentRectUpdateAfterScrollInView:]):
(-[WKWebView _updateVisibleContentRects]):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (233560 => 233561)


--- trunk/Source/WebKit/ChangeLog	2018-07-06 03:20:20 UTC (rev 233560)
+++ trunk/Source/WebKit/ChangeLog	2018-07-06 04:56:40 UTC (rev 233561)
@@ -1,3 +1,30 @@
+2018-07-05  Simon Fraser  <[email protected]>
+
+        Address two possible causes of missing tiles in iOS Safari, and add logging to gather more data about other possible causes
+        https://bugs.webkit.org/show_bug.cgi?id=187376
+        rdar://problem/40941118
+
+        Reviewed by Tim Horton.
+        
+        We have continual reports of users experiencing missing tiles in MobileSafari, where loading a page
+        shows the tiles at the top, but we don't render new tiles as the user scrolls down. This is consistent
+        with failing to dispatch visible content rect updates via -[WKWebView _updateVisibleContentRects].
+        
+        This patch addresses two possible (but unlikely) causes. First, it resets _currentlyAdjustingScrollViewInsetsForKeyboard
+        after a web process crash. Second, it catches exceptions thrown by [webView _updateVisibleContentRects]
+        and resets _hasScheduledVisibleRectUpdate.
+        
+        This patch also adds release logging that fires if over 1s has elapsed between scheduling
+        a visible content rect update and trying to re-schedule, and logging for all reasons that
+        -_updateVisibleContentRects returns early.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _initializeWithConfiguration:]):
+        (-[WKWebView _processDidExit]):
+        (-[WKWebView _addUpdateVisibleContentRectPreCommitHandler]):
+        (-[WKWebView _scheduleVisibleContentRectUpdateAfterScrollInView:]):
+        (-[WKWebView _updateVisibleContentRects]):
+
 2018-07-05  Olivia Barnett  <[email protected]>
 
         iPad: Scrolling with hardware keyboard while SELECT popover is visible scrolls the page, detaches popover

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (233560 => 233561)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-07-06 03:20:20 UTC (rev 233560)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-07-06 04:56:40 UTC (rev 233561)
@@ -181,6 +181,7 @@
 
 #if PLATFORM(IOS)
 static const uint32_t firstSDKVersionWithLinkPreviewEnabledByDefault = 0xA0000;
+static const Seconds delayBeforeNoVisibleContentsRectsLogging = 1_s;
 #endif
 
 #if PLATFORM(MAC)
@@ -356,6 +357,10 @@
     Vector<BlockPtr<void ()>> _visibleContentRectUpdateCallbacks;
 
     _WKDragInteractionPolicy _dragInteractionPolicy;
+
+    // For release-logging for <rdar://problem/39281269>.
+    MonotonicTime _timeOfRequestForVisibleContentRectUpdate;
+    MonotonicTime _timeOfLastVisibleContentRectUpdate;
 #endif
 #if PLATFORM(MAC)
     std::unique_ptr<WebKit::WebViewImpl> _impl;
@@ -726,6 +731,9 @@
 
 #if PLATFORM(IOS)
     _dragInteractionPolicy = _WKDragInteractionPolicyDefault;
+
+    _timeOfRequestForVisibleContentRectUpdate = MonotonicTime::now();
+    _timeOfLastVisibleContentRectUpdate = MonotonicTime::now();
 #if PLATFORM(WATCHOS)
     _allowsViewportShrinkToFit = YES;
 #else
@@ -1692,6 +1700,7 @@
     _scrollViewBackgroundColor = WebCore::Color();
     _delayUpdateVisibleContentRects = NO;
     _hadDelayedUpdateVisibleContentRects = NO;
+    _currentlyAdjustingScrollViewInsetsForKeyboard = NO;
     _lastSentViewLayoutSize = std::nullopt;
     _lastSentMaximumUnobscuredSize = std::nullopt;
     _lastSentDeviceOrientation = std::nullopt;
@@ -2711,9 +2720,16 @@
     auto retainedSelf = retainPtr(self);
     [CATransaction addCommitHandler:[retainedSelf] {
         WKWebView *webView = retainedSelf.get();
-        if (![webView _isValid])
+        if (![webView _isValid]) {
+            RELEASE_LOG_IF(webView._page && webView._page->isAlwaysOnLoggingAllowed(), ViewState, "In CATransaction preCommitHandler, WKWebView %p is invalid", webView);
             return;
-        [webView _updateVisibleContentRects];
+        }
+
+        @try {
+            [webView _updateVisibleContentRects];
+        } @catch (NSException *exception) {
+            RELEASE_LOG_IF(webView._page && webView._page->isAlwaysOnLoggingAllowed(), ViewState, "In CATransaction preCommitHandler, -[WKWebView %p _updateVisibleContentRects] threw an exception", webView);
+        }
         webView->_hasScheduledVisibleRectUpdate = NO;
     } forPhase:kCATransactionPhasePreCommit];
 }
@@ -2720,12 +2736,21 @@
 
 - (void)_scheduleVisibleContentRectUpdateAfterScrollInView:(UIScrollView *)scrollView
 {
+    RELEASE_ASSERT(isMainThread());
+
     _visibleContentRectUpdateScheduledFromScrollViewInStableState = [self _scrollViewIsInStableState:scrollView];
 
-    if (_hasScheduledVisibleRectUpdate)
+    if (_hasScheduledVisibleRectUpdate) {
+        auto timeNow = MonotonicTime::now();
+        if ((timeNow - _timeOfRequestForVisibleContentRectUpdate) > delayBeforeNoVisibleContentsRectsLogging) {
+            RELEASE_LOG_IF_ALLOWED("-[WKWebView %p _scheduleVisibleContentRectUpdateAfterScrollInView]: _hasScheduledVisibleRectUpdate is true but haven't updated visible content rects for %.2fs (last update was %.2fs ago)",
+                self, (timeNow - _timeOfRequestForVisibleContentRectUpdate).value(), (timeNow - _timeOfLastVisibleContentRectUpdate).value());
+        }
         return;
+    }
 
     _hasScheduledVisibleRectUpdate = YES;
+    _timeOfRequestForVisibleContentRectUpdate = MonotonicTime::now();
 
     CATransactionPhase transactionPhase = [CATransaction currentPhase];
     if (transactionPhase == kCATransactionPhaseNull || transactionPhase == kCATransactionPhasePreLayout) {
@@ -2786,11 +2811,13 @@
     if (![self usesStandardContentView]) {
         [_passwordView setFrame:self.bounds];
         [_customContentView web_computedContentInsetDidChange];
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] - usesStandardContentView is NO, bailing", self);
         return;
     }
 
     if (_delayUpdateVisibleContentRects) {
         _hadDelayedUpdateVisibleContentRects = YES;
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] - _delayUpdateVisibleContentRects is YES, bailing", self);
         return;
     }
 
@@ -2797,8 +2824,11 @@
     if (_dynamicViewportUpdateMode != WebKit::DynamicViewportUpdateMode::NotResizing
         || (_needsResetViewStateAfterCommitLoadForMainFrame && ![_contentView sizeChangedSinceLastVisibleContentRectUpdate])
         || [_scrollView isZoomBouncing]
-        || _currentlyAdjustingScrollViewInsetsForKeyboard)
+        || _currentlyAdjustingScrollViewInsetsForKeyboard) {
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] - scroll view state is non-stable, bailing (_dynamicViewportUpdateMode %d, _needsResetViewStateAfterCommitLoadForMainFrame %d, sizeChangedSinceLastVisibleContentRectUpdate %d, [_scrollView isZoomBouncing] %d, _currentlyAdjustingScrollViewInsetsForKeyboard %d)",
+            self, _dynamicViewportUpdateMode, _needsResetViewStateAfterCommitLoadForMainFrame, [_contentView sizeChangedSinceLastVisibleContentRectUpdate], [_scrollView isZoomBouncing], _currentlyAdjustingScrollViewInsetsForKeyboard);
         return;
+    }
 
     CGRect visibleRectInContentCoordinates = [self _visibleContentRect];
 
@@ -2844,6 +2874,12 @@
         auto callback = _visibleContentRectUpdateCallbacks.takeLast();
         callback();
     }
+    
+    auto timeNow = MonotonicTime::now();
+    if ((timeNow - _timeOfRequestForVisibleContentRectUpdate) > delayBeforeNoVisibleContentsRectsLogging)
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] finally ran %.2fs after begin scheduled", self, (timeNow - _timeOfRequestForVisibleContentRectUpdate).value());
+
+    _timeOfLastVisibleContentRectUpdate = timeNow;
 }
 
 - (void)_didStartProvisionalLoadForMainFrame
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to