Title: [233751] trunk/Source/WebKit
Revision
233751
Author
simon.fra...@apple.com
Date
2018-07-11 19:09:12 -0700 (Wed, 11 Jul 2018)

Log Message

[iOS WK2] Address a possible cause of missing tiles
https://bugs.webkit.org/show_bug.cgi?id=187570
rdar://problem/40941118

Reviewed by Tim Horton.

If the web process crashes, it's possible for the user to trigger a scroll before
the process is re-launched. The pre-commit handler will bail early on the _isValid
check without clearing _hasScheduledVisibleRectUpdate, so that remains YES and we
get stuck doing no more visible content rect updates.

Fix by adding -[WKWebView _didRelaunchProcess] (WKContentView got this already), and
in that clear state that could have been set in the UI process while the web process
connection was invalid.

Also add more release logging around process termination, crash and relaunch.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _processDidExit]):
(-[WKWebView _didRelaunchProcess]):
(-[WKWebView _scheduleVisibleContentRectUpdateAfterScrollInView:]):
(-[WKWebView _updateVisibleContentRects]):
* UIProcess/API/Cocoa/WKWebViewInternal.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::reattachToWebProcess):
(WebKit::WebPageProxy::processDidTerminate):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::didClose):
(WebKit::WebProcessProxy::didFinishLaunching):
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didRelaunchProcess):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (233750 => 233751)


--- trunk/Source/WebKit/ChangeLog	2018-07-12 01:35:16 UTC (rev 233750)
+++ trunk/Source/WebKit/ChangeLog	2018-07-12 02:09:12 UTC (rev 233751)
@@ -1,3 +1,37 @@
+2018-07-11  Simon Fraser  <simon.fra...@apple.com>
+
+        [iOS WK2] Address a possible cause of missing tiles
+        https://bugs.webkit.org/show_bug.cgi?id=187570
+        rdar://problem/40941118
+
+        Reviewed by Tim Horton.
+        
+        If the web process crashes, it's possible for the user to trigger a scroll before
+        the process is re-launched. The pre-commit handler will bail early on the _isValid
+        check without clearing _hasScheduledVisibleRectUpdate, so that remains YES and we
+        get stuck doing no more visible content rect updates.
+        
+        Fix by adding -[WKWebView _didRelaunchProcess] (WKContentView got this already), and
+        in that clear state that could have been set in the UI process while the web process
+        connection was invalid.
+        
+        Also add more release logging around process termination, crash and relaunch.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _processDidExit]):
+        (-[WKWebView _didRelaunchProcess]):
+        (-[WKWebView _scheduleVisibleContentRectUpdateAfterScrollInView:]):
+        (-[WKWebView _updateVisibleContentRects]):
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::reattachToWebProcess):
+        (WebKit::WebPageProxy::processDidTerminate):
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::didClose):
+        (WebKit::WebProcessProxy::didFinishLaunching):
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::didRelaunchProcess):
+
 2018-07-11  Wenson Hsieh  <wenson_hs...@apple.com>
 
         Release logging dumps "Cleaning up dragging stateā€¦" every time gesture recognizers are reset in WKContentView

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


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-07-12 01:35:16 UTC (rev 233750)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-07-12 02:09:12 UTC (rev 233751)
@@ -1665,6 +1665,7 @@
 
 - (void)_processDidExit
 {
+    RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _processDidExit]", self);
     [self _hidePasswordView];
     if (!_customContentView && _dynamicViewportUpdateMode != WebKit::DynamicViewportUpdateMode::NotResizing) {
         NSUInteger indexOfResizeAnimationView = [[_scrollView subviews] indexOfObject:_resizeAnimationView.get()];
@@ -1711,6 +1712,13 @@
     _avoidsUnsafeArea = YES;
 }
 
+- (void)_didRelaunchProcess
+{
+    RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _didRelaunchProcess]", self);
+    _hasScheduledVisibleRectUpdate = NO;
+    _visibleContentRectUpdateScheduledFromScrollViewInStableState = YES;
+}
+
 - (void)_didCommitLoadForMainFrame
 {
     _firstPaintAfterCommitLoadTransactionID = downcast<WebKit::RemoteLayerTreeDrawingAreaProxy>(*_page->drawingArea()).nextLayerTreeTransactionID();
@@ -2735,8 +2743,8 @@
     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());
+            RELEASE_LOG_IF_ALLOWED("-[WKWebView %p _scheduleVisibleContentRectUpdateAfterScrollInView]: _hasScheduledVisibleRectUpdate is true but haven't updated visible content rects for %.2fs (last update was %.2fs ago) - valid %d",
+                self, (timeNow - _timeOfRequestForVisibleContentRectUpdate).value(), (timeNow - _timeOfLastVisibleContentRectUpdate).value(), [self _isValid]);
         }
         return;
     }
@@ -2869,7 +2877,7 @@
     
     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());
+        RELEASE_LOG_IF_ALLOWED("%p -[WKWebView _updateVisibleContentRects:] finally ran %.2fs after being scheduled", self, (timeNow - _timeOfRequestForVisibleContentRectUpdate).value());
 
     _timeOfLastVisibleContentRectUpdate = timeNow;
 }

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h (233750 => 233751)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h	2018-07-12 01:35:16 UTC (rev 233750)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h	2018-07-12 02:09:12 UTC (rev 233751)
@@ -77,6 +77,7 @@
 
 #if PLATFORM(IOS)
 - (void)_processDidExit;
+- (void)_didRelaunchProcess;
 
 - (void)_didCommitLoadForMainFrame;
 - (void)_didCommitLayerTree:(const WebKit::RemoteLayerTreeTransaction&)layerTreeTransaction;

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (233750 => 233751)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-07-12 01:35:16 UTC (rev 233750)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-07-12 02:09:12 UTC (rev 233751)
@@ -743,7 +743,7 @@
         m_process->frameCreated(*m_mainFrameID, *m_mainFrame);
     }
 
-    LOG(ProcessSwapping, "(ProcessSwapping) Reattaching WebPageProxy %p to WebProcessProxy %p with pid %i\n", this, m_process.ptr(), m_process->processIdentifier());
+    RELEASE_LOG_IF_ALLOWED(Process, "%p WebPageProxy::reattachToWebProcess\n", this);
 
     ASSERT(m_process->state() != ChildProcessProxy::State::Terminated);
     if (m_process->state() == ChildProcessProxy::State::Running)
@@ -5849,6 +5849,9 @@
 
 void WebPageProxy::processDidTerminate(ProcessTerminationReason reason)
 {
+    if (reason != ProcessTerminationReason::NavigationSwap)
+        RELEASE_LOG_IF_ALLOWED(Process, "%p - WebPageProxy::processDidTerminate (pid %d), reason %d", this, processIdentifier(), reason);
+
     ASSERT(m_isValid);
 
 #if PLATFORM(IOS)

Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (233750 => 233751)


--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2018-07-12 01:35:16 UTC (rev 233750)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp	2018-07-12 02:09:12 UTC (rev 233751)
@@ -661,6 +661,7 @@
 
 void WebProcessProxy::didClose(IPC::Connection&)
 {
+    RELEASE_LOG_IF(m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed(), Process, "%p - WebProcessProxy didClose (web process crash)", this);
     processDidTerminateOrFailedToLaunch();
 }
 
@@ -753,6 +754,7 @@
     ChildProcessProxy::didFinishLaunching(launcher, connectionIdentifier);
 
     if (!IPC::Connection::identifierIsValid(connectionIdentifier)) {
+        RELEASE_LOG_IF(m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed(), Process, "%p - WebProcessProxy didFinishLaunching - invalid connection identifier (web process failed to launch)", this);
         processDidTerminateOrFailedToLaunch();
         return;
     }
@@ -965,6 +967,8 @@
     if (state() == State::Terminated)
         return;
 
+    RELEASE_LOG_IF(m_websiteDataStore->sessionID().isAlwaysOnLoggingAllowed(), Process, "%p - WebProcessProxy::requestTermination - reason %d", this, reason);
+
     ChildProcessProxy::terminate();
 
     if (webConnection())

Modified: trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm (233750 => 233751)


--- trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm	2018-07-12 01:35:16 UTC (rev 233750)
+++ trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm	2018-07-12 02:09:12 UTC (rev 233751)
@@ -211,6 +211,7 @@
 void PageClientImpl::didRelaunchProcess()
 {
     [m_contentView _didRelaunchProcess];
+    [m_webView _didRelaunchProcess];
 }
 
 void PageClientImpl::pageClosed()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to