Changeset 233561 in webkit

Timestamp:

Jul 5, 2018, 9:56:40 PM (7 years ago)

Author:

Simon Fraser

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]):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/Cocoa/WKWebView.mm (modified) (10 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-07-05  Simon Fraser  <simon.fraser@apple.com>
+
+        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  <obarnett@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -182 +182 @@
 #if PLATFORM(IOS)
 static const uint32_t firstSDKVersionWithLinkPreviewEnabledByDefault = 0xA0000;
+static const Seconds delayBeforeNoVisibleContentsRectsLogging = 1_s;
 #endif
 
@@ -357 +358 @@
 
     _WKDragInteractionPolicy _dragInteractionPolicy;
+
+    // For release-logging for <rdar://problem/39281269>.
+    MonotonicTime _timeOfRequestForVisibleContentRectUpdate;
+    MonotonicTime _timeOfLastVisibleContentRectUpdate;
 #endif
 #if PLATFORM(MAC)
@@ -727 +732 @@
 #if PLATFORM(IOS)
     _dragInteractionPolicy = _WKDragInteractionPolicyDefault;
+
+    _timeOfRequestForVisibleContentRectUpdate = MonotonicTime::now();
+    _timeOfLastVisibleContentRectUpdate = MonotonicTime::now();
 #if PLATFORM(WATCHOS)
     _allowsViewportShrinkToFit = YES;
@@ -1693 +1701 @@
     _delayUpdateVisibleContentRects = NO;
     _hadDelayedUpdateVisibleContentRects = NO;
+    _currentlyAdjustingScrollViewInsetsForKeyboard = NO;
     _lastSentViewLayoutSize = std::nullopt;
     _lastSentMaximumUnobscuredSize = std::nullopt;
@@ -2712 +2721 @@
     [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];
@@ -2721 +2737 @@
 - (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];
@@ -2787 +2812 @@
         [_passwordView setFrame:self.bounds];
         [_customContentView web_computedContentInsetDidChange];
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] - usesStandardContentView is NO, bailing", self);
         return;
     }
@@ -2792 +2818 @@
     if (_delayUpdateVisibleContentRects) {
         _hadDelayedUpdateVisibleContentRects = YES;
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] - _delayUpdateVisibleContentRects is YES, bailing", self);
         return;
     }
@@ -2798 +2825 @@
         || (_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];
@@ -2845 +2875 @@
         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;
 }