Commit 5a7671b5 authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

[ios/web] Disconnect the scroll proxy during slimnav restore.

This is a speculative change to mitigate the crashes in https://crbug.com/959499

Bug: 959499
Change-Id: I37e9d088abf8093cf0a74d410d38a42edac0e5ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1604522
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarKurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarDanyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659352}
parent 73dcd9b0
...@@ -35,6 +35,10 @@ extern const base::Feature kWKHTTPSystemCookieStore; ...@@ -35,6 +35,10 @@ extern const base::Feature kWKHTTPSystemCookieStore;
// https://crbug.com/841105. // https://crbug.com/841105.
extern const base::Feature kCrashOnUnexpectedURLChange; extern const base::Feature kCrashOnUnexpectedURLChange;
// Used to disconnect the scroll proxy during slimnav restore. This is a
// speculative change to mitigate the crashes in https://crbug.com/959499.
extern const base::Feature kDisconnectScrollProxyDuringRestore;
// Used to enable the workaround for WKWebView history clobber bug // Used to enable the workaround for WKWebView history clobber bug
// (crbug.com/887497). // (crbug.com/887497).
extern const base::Feature kHistoryClobberWorkaround; extern const base::Feature kHistoryClobberWorkaround;
......
...@@ -30,6 +30,9 @@ const base::Feature kWKHTTPSystemCookieStore{"WKHTTPSystemCookieStore", ...@@ -30,6 +30,9 @@ const base::Feature kWKHTTPSystemCookieStore{"WKHTTPSystemCookieStore",
const base::Feature kCrashOnUnexpectedURLChange{ const base::Feature kCrashOnUnexpectedURLChange{
"CrashOnUnexpectedURLChange", base::FEATURE_ENABLED_BY_DEFAULT}; "CrashOnUnexpectedURLChange", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kDisconnectScrollProxyDuringRestore{
"DisconnectScrollProxyDuringRestore", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kHistoryClobberWorkaround{ const base::Feature kHistoryClobberWorkaround{
"WKWebViewHistoryClobberWorkaround", base::FEATURE_ENABLED_BY_DEFAULT}; "WKWebViewHistoryClobberWorkaround", base::FEATURE_ENABLED_BY_DEFAULT};
......
...@@ -1965,10 +1965,14 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -1965,10 +1965,14 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
// Disable |allowsBackForwardNavigationGestures| during restore. Otherwise, // Disable |allowsBackForwardNavigationGestures| during restore. Otherwise,
// WebKit will trigger a snapshot for each (blank) page, and quickly // WebKit will trigger a snapshot for each (blank) page, and quickly
// overload system memory. // overload system memory. Also disables the scroll proxy during session
// restoration.
if (web::GetWebClient()->IsSlimNavigationManagerEnabled() && if (web::GetWebClient()->IsSlimNavigationManagerEnabled() &&
self.navigationManagerImpl->IsRestoreSessionInProgress()) { self.navigationManagerImpl->IsRestoreSessionInProgress()) {
_webView.allowsBackForwardNavigationGestures = NO; _webView.allowsBackForwardNavigationGestures = NO;
if (base::FeatureList::IsEnabled(
web::features::kDisconnectScrollProxyDuringRestore))
[_containerView disconnectScrollProxy];
} }
WKNavigation* navigation = nil; WKNavigation* navigation = nil;
...@@ -2199,13 +2203,20 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -2199,13 +2203,20 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
if (self.navigationState == web::WKNavigationState::FINISHED) if (self.navigationState == web::WKNavigationState::FINISHED)
return; return;
// Restore allowsBackForwardNavigationGestures once restoration is complete. // Restore allowsBackForwardNavigationGestures and the scroll proxy once
// restoration is complete.
if (web::GetWebClient()->IsSlimNavigationManagerEnabled() && if (web::GetWebClient()->IsSlimNavigationManagerEnabled() &&
!self.navigationManagerImpl->IsRestoreSessionInProgress()) { !self.navigationManagerImpl->IsRestoreSessionInProgress()) {
if (_webView.allowsBackForwardNavigationGestures != if (_webView.allowsBackForwardNavigationGestures !=
_allowsBackForwardNavigationGestures) _allowsBackForwardNavigationGestures) {
_webView.allowsBackForwardNavigationGestures = _webView.allowsBackForwardNavigationGestures =
_allowsBackForwardNavigationGestures; _allowsBackForwardNavigationGestures;
}
if (base::FeatureList::IsEnabled(
web::features::kDisconnectScrollProxyDuringRestore)) {
[_containerView reconnectScrollProxy];
}
} }
BOOL success = !context || !context->GetError(); BOOL success = !context || !context->GetError();
...@@ -3802,6 +3813,13 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -3802,6 +3813,13 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
[[CRWWebViewContentView alloc] initWithWebView:self.webView [[CRWWebViewContentView alloc] initWithWebView:self.webView
scrollView:self.webScrollView]; scrollView:self.webScrollView];
[_containerView displayWebViewContentView:webViewContentView]; [_containerView displayWebViewContentView:webViewContentView];
if (web::GetWebClient()->IsSlimNavigationManagerEnabled() &&
self.navigationManagerImpl->IsRestoreSessionInProgress() &&
base::FeatureList::IsEnabled(
web::features::kDisconnectScrollProxyDuringRestore)) {
[_containerView disconnectScrollProxy];
}
} }
- (void)removeWebView { - (void)removeWebView {
......
...@@ -69,6 +69,11 @@ ...@@ -69,6 +69,11 @@
// Removes all subviews and resets state to default. // Removes all subviews and resets state to default.
- (void)resetContent; - (void)resetContent;
// Disconnects and reconnects the scroll proxy to prevent extra calls to
// WKebView.
- (void)disconnectScrollProxy;
- (void)reconnectScrollProxy;
// Replaces the currently displayed content with |webViewContentView|. // Replaces the currently displayed content with |webViewContentView|.
- (void)displayWebViewContentView:(CRWWebViewContentView*)webViewContentView; - (void)displayWebViewContentView:(CRWWebViewContentView*)webViewContentView;
......
...@@ -211,4 +211,12 @@ ...@@ -211,4 +211,12 @@
self.contentViewProxy.contentView = self.webViewContentView; self.contentViewProxy.contentView = self.webViewContentView;
} }
- (void)disconnectScrollProxy {
[self.contentViewProxy disconnectScrollProxy];
}
- (void)reconnectScrollProxy {
[self.contentViewProxy reconnectScrollProxy];
}
@end @end
...@@ -23,6 +23,11 @@ ...@@ -23,6 +23,11 @@
// Init with a weak reference of web controller, used for passing through calls. // Init with a weak reference of web controller, used for passing through calls.
- (instancetype)initWithWebController:(CRWWebController*)webController; - (instancetype)initWithWebController:(CRWWebController*)webController;
// Disconnects and reconnects the scroll proxy to prevent extra calls to
// WKebView.
- (void)disconnectScrollProxy;
- (void)reconnectScrollProxy;
@end @end
#endif // IOS_WEB_WEB_STATE_UI_CRW_WEB_VIEW_PROXY_IMPL_H_ #endif // IOS_WEB_WEB_STATE_UI_CRW_WEB_VIEW_PROXY_IMPL_H_
...@@ -170,6 +170,14 @@ UIView* GetFirstResponderSubview(UIView* view) { ...@@ -170,6 +170,14 @@ UIView* GetFirstResponderSubview(UIView* view) {
[_contentViewScrollViewProxy setScrollView:contentView.scrollView]; [_contentViewScrollViewProxy setScrollView:contentView.scrollView];
} }
- (void)disconnectScrollProxy {
[_contentViewScrollViewProxy setScrollView:nil];
}
- (void)reconnectScrollProxy {
[_contentViewScrollViewProxy setScrollView:self.contentView.scrollView];
}
- (void)addSubview:(UIView*)view { - (void)addSubview:(UIView*)view {
return [_contentView addSubview:view]; return [_contentView addSubview:view];
} }
......
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