Commit 8b215cc6 authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

ios: Speculative workaround for WebKit dispatchIncomingMessages crash.

Preload appears to trigger an edge-case crash in WebKit when a restore
is triggered and cancelled before it can complete.  This isn't specific
to preload, but is very easy to trigger in preload.  As a speculative
fix, if a preload is in restore, don't destroy it until after restore
is complete. This logic should really belong in WebState itself, so any
attempt to destroy a WebState during restore will trigger this logic.

Even better, this edge case crash should be fixed in WebKit:
    https://bugs.webkit.org/show_bug.cgi?id=217440.

The crash in WebKit appears to be related to IPC throttling.  Session
restore can create a large number of IPC calls, which can then be
throttled.  It seems if the WKWebView is destroyed with this backlog of
IPC calls, sometimes WebKit crashes.

See crbug.com/1032928 for an explanation for how to trigger this crash.

Note the timer should only be called if for some reason session
restoration fails to complete -- thus preventing a WebState leak.

Bug: 1032928
Change-Id: I217918437d5cdc6f610f3331634791699dd678ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461571Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarRohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Auto-Submit: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815667}
parent aa5ee3d3
......@@ -140,6 +140,14 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
private:
__weak id<PreloadCancelling> cancel_handler_ = nil;
};
// Maximum time to let a cancelled webState attempt to finish restore.
static const size_t kMaximumCancelledWebStateDelay = 2;
// Used to enable the workaround for a WebKit crash, see crbug.com/1032928.
const base::Feature kPreloadDelayWebStateReset{
"PreloadDelayWebStateReset", base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace
@interface PreloadController () <CRConnectionTypeObserverBridge,
......@@ -631,8 +639,49 @@ class PreloadJavaScriptDialogPresenter : public web::JavaScriptDialogPresenter {
_webState->RemoveObserver(_webStateObserver.get());
breakpad::StopMonitoringURLsForPreloadWebState(_webState.get());
_webState->SetDelegate(nullptr);
_webState.reset();
// Preload appears to trigger an edge-case crash in WebKit when a restore is
// triggered and cancelled before it can complete. This isn't specific to
// preload, but is very easy to trigger in preload. As a speculative fix, if
// a preload is in restore, don't destroy it until after restore is complete.
// This logic should really belong in WebState itself, so any attempt to
// destroy a WebState during restore will trigger this logic. Even better,
// this edge case crash should be fixed in WebKit:
// https://bugs.webkit.org/show_bug.cgi?id=217440.
// The crash in WebKit appears to be related to IPC throttling. Session
// restore can create a large number of IPC calls, which can then be
// throttled. It seems if the WKWebView is destroyed with this backlog of
// IPC calls, sometimes WebKit crashes.
// See crbug.com/1032928 for an explanation for how to trigger this crash.
// Note the timer should only be called if for some reason session restoration
// fails to complete -- thus preventing a WebState leak.
static bool delayPreloadDestroyWebState =
base::FeatureList::IsEnabled(kPreloadDelayWebStateReset);
if (delayPreloadDestroyWebState &&
_webState->GetNavigationManager()->IsRestoreSessionInProgress()) {
__block std::unique_ptr<web::WebState> webState = std::move(_webState);
__block std::unique_ptr<base::OneShotTimer> resetTimer(
new base::OneShotTimer());
auto reset_block = ^{
if (webState) {
webState.reset();
}
if (resetTimer) {
resetTimer->Stop();
resetTimer.reset();
}
};
resetTimer->Start(
FROM_HERE, base::TimeDelta::FromSeconds(kMaximumCancelledWebStateDelay),
base::BindOnce(reset_block));
webState->GetNavigationManager()->AddRestoreCompletionCallback(
base::BindOnce(^{
dispatch_async(dispatch_get_main_queue(), reset_block);
}));
} else {
_webState.reset();
}
self.prerenderedURL = GURL();
self.startTime = base::TimeTicks();
self.loadCompleted = NO;
......
......@@ -3414,6 +3414,21 @@
]
}
],
"IOSPreloadDelayWebStateReset": [
{
"platforms": [
"ios"
],
"experiments": [
{
"name": "Enabled",
"enable_features": [
"PreloadDelayWebStateReset"
]
}
]
}
],
"IOSRequestDesktopByDefault": [
{
"platforms": [
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment