Commit bb2102fa authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Fix document URL for multiple navigations

This CL fixes which URL is used to set the document URL.
The previous implementation was using the WebView URL. Some times this
is not the same as the URL of the navigation. For example on the
following workflow:
1. Start a navigation to about://newtab
2. Once the navigation1 reach DidStartNavigation, start a navigation
   to another page (e.g. http://example.com).
3. The URL of WebView is updated to http://example.com
4. The provisional navigation to http://example is starting
5. The navigaation1 (to about://newtab) is committed.

At step 5, the documentURL should be the URL of the committed navigation
(i.e. about://newtab), but the URL of the WebView is http://example.com.

Fixed: 1127025
Change-Id: Icc271b87c1877bf005ee4454c863126aacb81127
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418856
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814124}
parent 987f8ca9
...@@ -986,16 +986,32 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -986,16 +986,32 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
} }
} }
// No further code relies an existance of pending item, so this navigation can // The WebView URL can't always be trusted when multiple pending navigations
// be marked as "committed". // are occuring, as a navigation could commit after a new navigation has
[self.navigationStates setState:web::WKNavigationState::COMMITTED // started (and thus the WebView URL will be the URL of the new navigation).
forNavigation:navigation]; // See crbug.com/1127025.
BOOL hasMultiplePendingNavigations =
[self.navigationStates pendingNavigations].count > 1;
// When loading an error page that is a placeholder, the webViewURL should be
// used as it is the actual URL we want to load.
BOOL isErrorPage =
!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) &&
!context->IsPlaceholderNavigation();
BOOL shouldUseContextURL =
context ? !isErrorPage && hasMultiplePendingNavigations : NO;
GURL documentURL = shouldUseContextURL ? context->GetUrl() : webViewURL;
// This is the point where the document's URL has actually changed. // This is the point where the document's URL has actually changed.
[self.delegate navigationHandler:self [self.delegate navigationHandler:self
setDocumentURL:webViewURL setDocumentURL:documentURL
context:context]; context:context];
// No further code relies an existance of pending item, so this navigation can
// be marked as "committed".
[self.navigationStates setState:web::WKNavigationState::COMMITTED
forNavigation:navigation];
if (!committedNavigation && context && if (!committedNavigation && context &&
(base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) || (base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) ||
!context->IsLoadingErrorPage())) { !context->IsLoadingErrorPage())) {
......
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