Commit 4b93e2d4 authored by Danyao Wang's avatar Danyao Wang Committed by Commit Bot

[Nav Experiment] Create pending back/forward item in decidePolicy.

WKBackForwardList is updated before navigation delegates are called on
back/forward navigations. This creates a window between decidePolicy
and didStartProvisionalNavigation where WKBasedNavigationManager
incorrectly identifies WKBackForwardList.currentItem as last committed,
even though it is really the pending item. Creating the pending item
in decidePolicy delegate fixes this problem.

Bug: 842151
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I3252edd1def6d0d2eb1499602e05058c3b2fd3b1
Reviewed-on: https://chromium-review.googlesource.com/1142142Reviewed-by: default avatarKurt Horimoto <kkhorimoto@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576212}
parent ac758278
...@@ -221,6 +221,17 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) { ...@@ -221,6 +221,17 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) {
// during pre-commit delegate callbacks, and thus must be held until the // during pre-commit delegate callbacks, and thus must be held until the
// navigation commits and the informatino can be used. // navigation commits and the informatino can be used.
@interface CRWWebControllerPendingNavigationInfo : NSObject { @interface CRWWebControllerPendingNavigationInfo : NSObject {
// For back/forward navigation, WKWebView updates its back-forward history
// state before any navigation delegate callbacks. To ensure the correct
// navigation manager states are presented to WebStateObserver,
// WKBasedNavigationManager creates pending item and its navigation context
// in |webView:decidePolicyForNavigationAction| and stores it here to be
// associated with the corresponding WKNavigation when that information is
// available in |webView:didStartProvisionalNavigation|.
// See http://crbug.com/842151 for a bug caused by this scenario.
// TODO(crbug.com/661316): move navigation states management to navigation
// manager.
std::unique_ptr<web::NavigationContextImpl> _pendingBackForwardContext;
} }
// The referrer for the page. // The referrer for the page.
@property(nonatomic, copy) NSString* referrer; @property(nonatomic, copy) NSString* referrer;
...@@ -237,6 +248,18 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) { ...@@ -237,6 +248,18 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) {
@property(nonatomic, assign) BOOL cancelled; @property(nonatomic, assign) BOOL cancelled;
// Whether the navigation was initiated by a user gesture. // Whether the navigation was initiated by a user gesture.
@property(nonatomic, assign) BOOL hasUserGesture; @property(nonatomic, assign) BOOL hasUserGesture;
// Used by |webView:decidePolicyForNavigationAction| during a new back/forward
// navigation to store the navigation context temporarily until it can be
// associated with the WKNavigation in |webView:didStartProvisionalNavigation|.
- (void)setPendingBackForwardContext:
(std::unique_ptr<web::NavigationContextImpl>)context;
// Used by |webView:didStartProvisionalNavigation| to retrieve the navigation
// context created in |webView:decidePolicyForNavigationAction| for back/forward
// navigations.
- (std::unique_ptr<web::NavigationContextImpl>)releasePendingBackForwardContext;
@end @end
@implementation CRWWebControllerPendingNavigationInfo @implementation CRWWebControllerPendingNavigationInfo
...@@ -253,6 +276,16 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) { ...@@ -253,6 +276,16 @@ NSError* WKWebViewErrorWithSource(NSError* error, WKWebViewErrorSource source) {
} }
return self; return self;
} }
- (void)setPendingBackForwardContext:
(std::unique_ptr<web::NavigationContextImpl>)context {
_pendingBackForwardContext = std::move(context);
}
- (std::unique_ptr<web::NavigationContextImpl>)
releasePendingBackForwardContext {
return std::move(_pendingBackForwardContext);
}
@end @end
@interface CRWWebController ()<CRWContextMenuDelegate, @interface CRWWebController ()<CRWContextMenuDelegate,
...@@ -4216,6 +4249,17 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -4216,6 +4249,17 @@ registerLoadRequestForURL:(const GURL&)requestURL
// retrieved state will be pending until |didCommitNavigation| callback. // retrieved state will be pending until |didCommitNavigation| callback.
[self updatePendingNavigationInfoFromNavigationAction:action]; [self updatePendingNavigationInfoFromNavigationAction:action];
if (web::GetWebClient()->IsSlimNavigationManagerEnabled() &&
action.navigationType == WKNavigationTypeBackForward) {
// WKBackForwardList would have already been updated for back/forward
// navigation. Create the pending item here to match.
std::unique_ptr<web::NavigationContextImpl> context = [self
registerLoadRequestForURL:net::GURLWithNSURL(action.request.URL)
sameDocumentNavigation:NO
hasUserGesture:[_pendingNavigationInfo hasUserGesture]];
[_pendingNavigationInfo setPendingBackForwardContext:std::move(context)];
}
if (web::GetWebClient()->IsSlimNavigationManagerEnabled() || if (web::GetWebClient()->IsSlimNavigationManagerEnabled() ||
base::FeatureList::IsEnabled(web::features::kWebErrorPages)) { base::FeatureList::IsEnabled(web::features::kWebErrorPages)) {
// If this is a placeholder navigation, pass through. // If this is a placeholder navigation, pass through.
...@@ -4307,7 +4351,6 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -4307,7 +4351,6 @@ registerLoadRequestForURL:(const GURL&)requestURL
// Loading should not start until webView.loading is changed to YES. // Loading should not start until webView.loading is changed to YES.
_webStateImpl->SetIsLoading(false); _webStateImpl->SetIsLoading(false);
} }
decisionHandler(allowLoad ? WKNavigationActionPolicyAllow decisionHandler(allowLoad ? WKNavigationActionPolicyAllow
: WKNavigationActionPolicyCancel); : WKNavigationActionPolicyCancel);
} }
...@@ -4417,6 +4460,21 @@ registerLoadRequestForURL:(const GURL&)requestURL ...@@ -4417,6 +4460,21 @@ registerLoadRequestForURL:(const GURL&)requestURL
web::NavigationContextImpl* context = web::NavigationContextImpl* context =
[_navigationStates contextForNavigation:navigation]; [_navigationStates contextForNavigation:navigation];
// WKBasedNavigationManager creates context for back/forward navigation in
// decide policy delegate.
if (web::GetWebClient()->IsSlimNavigationManagerEnabled() &&
_pendingNavigationInfo.navigationType == WKNavigationTypeBackForward) {
std::unique_ptr<web::NavigationContextImpl> pendingContext =
[_pendingNavigationInfo releasePendingBackForwardContext];
DCHECK(pendingContext.get());
if (!context) {
[_navigationStates setContext:std::move(pendingContext)
forNavigation:navigation];
context = [_navigationStates contextForNavigation:navigation];
}
}
if (context) { if (context) {
// This is already seen and registered navigation. // This is already seen and registered navigation.
......
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