Title: [293803] trunk
- Revision
- 293803
- Author
- cdu...@apple.com
- Date
- 2022-05-04 16:51:59 -0700 (Wed, 04 May 2022)
Log Message
WebKit should only reload WebViews automatically on crash if the view is visible
https://bugs.webkit.org/show_bug.cgi?id=240079
<rdar://92442052>
Reviewed by Geoffrey Garen and Darin Adler.
If the client app doesn't override WKNavigationDelegate.webViewWebContentProcessDidTerminate,
WebKit would previously reload the web view automatically. However, for some apps with a lot
of non-visible WebViews, this could cause a lot of churn when coming back to the foreground,
after many of these WebContent processes were jetsammed in the background.
To address this, WebKit now delays the automatic web view reload until the view becomes
visible (still reloads right away when the view is visible when the crash occurs).
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:
(TEST):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::launchProcess):
(WebKit::WebPageProxy::activityStateDidChange):
(WebKit::WebPageProxy::viewIsBecomingVisible):
(WebKit::WebPageProxy::dispatchProcessDidTerminate):
* Source/WebKit/UIProcess/WebPageProxy.h:
Canonical link: https://commits.webkit.org/250277@main
Modified Paths
Diff
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (293802 => 293803)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2022-05-04 23:27:43 UTC (rev 293802)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2022-05-04 23:51:59 UTC (rev 293803)
@@ -884,6 +884,7 @@
m_process = processPool.processForRegistrableDomain(m_websiteDataStore.get(), registrableDomain, shouldEnableCaptivePortalMode() ? WebProcessProxy::CaptivePortalMode::Enabled : WebProcessProxy::CaptivePortalMode::Disabled);
m_hasRunningProcess = true;
+ m_shouldReloadDueToCrashWhenVisible = false;
m_isCaptivePortalModeExplicitlySet = m_configuration->isCaptivePortalModeExplicitlySet();
m_process->addExistingWebPage(*this, WebProcessProxy::BeginsUsingDataStore::Yes);
@@ -2165,6 +2166,17 @@
m_potentiallyChangedActivityStateFlags.add(mayHaveChanged);
m_activityStateChangeWantsSynchronousReply = m_activityStateChangeWantsSynchronousReply || replyMode == ActivityStateChangeReplyMode::Synchronous;
+ // We need to do this here instead of inside dispatchActivityStateChange() or viewIsBecomingVisible() because these don't run when the view doesn't
+ // have a running WebProcess. For the same reason, we need to rely on PageClient::isViewVisible() instead of WebPageProxy::isViewVisible().
+ if (m_potentiallyChangedActivityStateFlags & ActivityState::IsVisible && m_shouldReloadDueToCrashWhenVisible && pageClient().isViewVisible()) {
+ RunLoop::main().dispatch([this, weakThis = WeakPtr { *this }] {
+ if (weakThis && std::exchange(m_shouldReloadDueToCrashWhenVisible, false)) {
+ WEBPAGEPROXY_RELEASE_LOG(ViewState, "activityStateDidChange: view is becoming visible after a crash, attempt a reload");
+ tryReloadAfterProcessTermination();
+ }
+ });
+ }
+
if (m_suppressVisibilityUpdates && dispatchMode != ActivityStateChangeDispatchMode::Immediate)
return;
@@ -5303,6 +5315,7 @@
void WebPageProxy::viewIsBecomingVisible()
{
+ WEBPAGEPROXY_RELEASE_LOG(ViewState, "viewIsBecomingVisible:");
m_process->markProcessAsRecentlyUsed();
#if ENABLE(MEDIA_STREAM)
if (m_userMediaPermissionRequestManager)
@@ -7969,8 +7982,15 @@
else
handledByClient = m_navigationClient->processDidTerminate(*this, reason);
- if (!handledByClient && shouldReloadAfterProcessTermination(reason))
- tryReloadAfterProcessTermination();
+ if (!handledByClient && shouldReloadAfterProcessTermination(reason)) {
+ // We delay the view reload until it becomes visible.
+ if (isViewVisible())
+ tryReloadAfterProcessTermination();
+ else {
+ WEBPAGEPROXY_RELEASE_LOG_ERROR(Loading, "dispatchProcessDidTerminate: Not eagerly reloading the view because it is not currently visible");
+ m_shouldReloadDueToCrashWhenVisible = true;
+ }
+ }
}
void WebPageProxy::tryReloadAfterProcessTermination()
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (293802 => 293803)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2022-05-04 23:27:43 UTC (rev 293802)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2022-05-04 23:51:59 UTC (rev 293803)
@@ -3014,6 +3014,7 @@
bool m_isShowingNavigationGestureSnapshot { false };
bool m_mainFramePluginHandlesPageScaleGesture { false };
+ bool m_shouldReloadDueToCrashWhenVisible { false };
unsigned m_pageCount { 0 };
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm (293802 => 293803)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm 2022-05-04 23:27:43 UTC (rev 293802)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm 2022-05-04 23:51:59 UTC (rev 293803)
@@ -179,9 +179,10 @@
EXPECT_TRUE(receivedScriptMessage);
}
-TEST(WKNavigation, AutomaticViewReloadAfterWebProcessCrash)
+TEST(WKNavigation, AutomaticVisibleViewReloadAfterWebProcessCrash)
{
- RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]);
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100) configuration:configuration.get() addToWindow:YES]);
auto delegate = adoptNS([[BasicNavigationDelegateWithoutCrashHandler alloc] init]);
[webView setNavigationDelegate:delegate.get()];
@@ -213,6 +214,43 @@
EXPECT_FALSE(startedLoad);
}
+TEST(WKNavigation, AutomaticHiddenViewDelayedReloadAfterWebProcessCrash)
+{
+ auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100) configuration:configuration.get() addToWindow:YES]);
+
+ auto delegate = adoptNS([[BasicNavigationDelegateWithoutCrashHandler alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ // Make sure the view is not visible.
+ [webView removeFromSuperview];
+
+ startedLoad = false;
+ finishedLoad = false;
+
+ [webView loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"rich-and-plain-text" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+ TestWebKitAPI::Util::run(&finishedLoad);
+
+ startedLoad = false;
+ finishedLoad = false;
+
+ // Simulate crash.
+ [webView _killWebContentProcess];
+
+ TestWebKitAPI::Util::sleep(0.5);
+
+ // WebKit should not have attempted a reload since the view is not visible.
+ EXPECT_FALSE(startedLoad);
+ EXPECT_FALSE(finishedLoad);
+
+ // Make the view visible.
+ [webView addToTestWindow];
+ [webView focus];
+
+ // WebKit should have triggered a reload when the view became visible.
+ TestWebKitAPI::Util::run(&finishedLoad);
+}
+
TEST(WKNavigation, ProcessCrashDuringCallback)
{
auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
