Commit 277b2bbe authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Fix error page URL

This CL makes sure that the correct URL (i.e. the one of the page that
created the error) is used instead of the error page file URL.

To do this, this CL uses IsLoadingErrorPage with the new error page
workflow. It allows to have the URL of the loaded page (not the URL of
the error page) and having the navigation marked as being a navigation
to an error page. Then the callbacks can act on it.

The main difference with the non-JS workflow is the back-forward
navigation. Because the URL of the page is the URL of the file page
that is displaying the error, we need to mark the navigation as a
error page navigation.
Then the navigation to the real page starts, and it is a new context
so it is no longer an error page navigation.

Bug: 991608
Change-Id: I4362d1cc50262ea86db1e211bfaec619f008625e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2449391Reviewed-by: default avatarAli Juma <ajuma@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815177}
parent 7e53fc74
...@@ -338,8 +338,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -338,8 +338,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
(!context->IsRendererInitiated() || (!context->IsRendererInitiated() ||
(context->GetPageTransition() & ui::PAGE_TRANSITION_FORWARD_BACK))) { (context->GetPageTransition() & ui::PAGE_TRANSITION_FORWARD_BACK))) {
transition = context->GetPageTransition(); transition = context->GetPageTransition();
if (!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) && if (context->IsLoadingErrorPage()) {
context->IsLoadingErrorPage()) {
// loadHTMLString: navigation which loads error page into WKWebView. // loadHTMLString: navigation which loads error page into WKWebView.
decisionHandler(WKNavigationActionPolicyAllow); decisionHandler(WKNavigationActionPolicyAllow);
return; return;
...@@ -625,18 +624,19 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -625,18 +624,19 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
if (context) { if (context) {
// This is already seen and registered navigation. // This is already seen and registered navigation.
if ((!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) && if (context->IsLoadingErrorPage()) {
context->IsLoadingErrorPage()) ||
(base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) &&
[ErrorPageHelper isErrorPageFileURL:context->GetUrl()])) {
// This is loadHTMLString: navigation to display error page in web view. // This is loadHTMLString: navigation to display error page in web view.
self.navigationState = web::WKNavigationState::REQUESTED; self.navigationState = web::WKNavigationState::REQUESTED;
return; return;
} }
if ((base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) || BOOL isErrorPageNavigation =
!context->IsPlaceholderNavigation()) && (base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) &&
!IsWKInternalUrl(webViewURL)) { [ErrorPageHelper isErrorPageFileURL:webViewURL]) ||
(!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) &&
context->IsPlaceholderNavigation());
if (!isErrorPageNavigation && !IsWKInternalUrl(webViewURL)) {
web::NavigationItem* item = web::NavigationItem* item =
web::GetItemWithUniqueID(self.navigationManagerImpl, context); web::GetItemWithUniqueID(self.navigationManagerImpl, context);
if (item) { if (item) {
...@@ -997,14 +997,23 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -997,14 +997,23 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
// See crbug.com/1127025. // See crbug.com/1127025.
BOOL hasMultiplePendingNavigations = BOOL hasMultiplePendingNavigations =
[self.navigationStates pendingNavigations].count > 1; [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. // When loading an error page, the context has the correct URL whereas the
// webView has the file URL.
BOOL isErrorPage = BOOL isErrorPage =
base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) &&
context && context->IsLoadingErrorPage();
// When loading an error page that is a placeholder (legacy), the webViewURL
// should be used as it is the actual URL we want to load.
BOOL isLegacyErrorPage =
!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) && !base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) &&
!context->IsPlaceholderNavigation(); context && !context->IsPlaceholderNavigation();
BOOL shouldUseContextURL = BOOL shouldUseContextURL =
context ? !isErrorPage && hasMultiplePendingNavigations : NO; context
? isErrorPage || (!isLegacyErrorPage && hasMultiplePendingNavigations)
: NO;
GURL documentURL = shouldUseContextURL ? context->GetUrl() : webViewURL; 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.
...@@ -1017,9 +1026,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -1017,9 +1026,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
[self.navigationStates setState:web::WKNavigationState::COMMITTED [self.navigationStates setState:web::WKNavigationState::COMMITTED
forNavigation:navigation]; forNavigation:navigation];
if (!committedNavigation && context && if (!committedNavigation && context && !context->IsLoadingErrorPage()) {
(base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) ||
!context->IsLoadingErrorPage())) {
self.webStateImpl->OnNavigationFinished(context); self.webStateImpl->OnNavigationFinished(context);
} }
...@@ -1966,6 +1973,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -1966,6 +1973,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
std::unique_ptr<web::NavigationContextImpl> originalContext = std::unique_ptr<web::NavigationContextImpl> originalContext =
[self.navigationStates removeNavigation:navigation]; [self.navigationStates removeNavigation:navigation];
originalContext->SetLoadingErrorPage(true);
[self.navigationStates setContext:std::move(originalContext) [self.navigationStates setContext:std::move(originalContext)
forNavigation:errorNavigation]; forNavigation:errorNavigation];
// Return as the context was moved. // Return as the context was moved.
...@@ -2038,6 +2046,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -2038,6 +2046,7 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
context->SetNavigationItemUniqueID(item->GetUniqueID()); context->SetNavigationItemUniqueID(item->GetUniqueID());
context->SetItem(std::move(item)); context->SetItem(std::move(item));
context->SetError(error); context->SetError(error);
context->SetLoadingErrorPage(true);
self.webStateImpl->OnNavigationStarted(context.get()); self.webStateImpl->OnNavigationStarted(context.get());
......
...@@ -149,12 +149,10 @@ WKNavigationType NavigationContextImpl::GetWKNavigationType() const { ...@@ -149,12 +149,10 @@ WKNavigationType NavigationContextImpl::GetWKNavigationType() const {
} }
bool NavigationContextImpl::IsLoadingErrorPage() const { bool NavigationContextImpl::IsLoadingErrorPage() const {
DCHECK(!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage));
return is_loading_error_page_; return is_loading_error_page_;
} }
void NavigationContextImpl::SetLoadingErrorPage(bool is_loading_error_page) { void NavigationContextImpl::SetLoadingErrorPage(bool is_loading_error_page) {
DCHECK(!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage));
is_loading_error_page_ = is_loading_error_page; is_loading_error_page_ = is_loading_error_page;
} }
......
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#import "ios/web/navigation/crw_web_view_navigation_observer_delegate.h" #import "ios/web/navigation/crw_web_view_navigation_observer_delegate.h"
#import "ios/web/navigation/crw_wk_navigation_handler.h" #import "ios/web/navigation/crw_wk_navigation_handler.h"
#import "ios/web/navigation/crw_wk_navigation_states.h" #import "ios/web/navigation/crw_wk_navigation_states.h"
#import "ios/web/navigation/error_page_helper.h"
#import "ios/web/navigation/navigation_context_impl.h" #import "ios/web/navigation/navigation_context_impl.h"
#import "ios/web/navigation/wk_back_forward_list_item_holder.h" #import "ios/web/navigation/wk_back_forward_list_item_holder.h"
#import "ios/web/navigation/wk_navigation_util.h" #import "ios/web/navigation/wk_navigation_util.h"
...@@ -822,6 +823,10 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -822,6 +823,10 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
self.navigationHandler.navigationState = web::WKNavigationState::REQUESTED; self.navigationHandler.navigationState = web::WKNavigationState::REQUESTED;
} }
if ([ErrorPageHelper isErrorPageFileURL:URL]) {
context->SetLoadingErrorPage(true);
}
web::WKBackForwardListItemHolder* holder = web::WKBackForwardListItemHolder* holder =
web::WKBackForwardListItemHolder::FromNavigationItem(item); web::WKBackForwardListItemHolder::FromNavigationItem(item);
holder->set_navigation_type(WKNavigationTypeBackForward); holder->set_navigation_type(WKNavigationTypeBackForward);
...@@ -914,11 +919,9 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -914,11 +919,9 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
_documentURL = newURL; _documentURL = newURL;
_userInteractionState.SetUserInteractionRegisteredSinceLastUrlChange(false); _userInteractionState.SetUserInteractionRegisteredSinceLastUrlChange(false);
} }
if (context && !context->IsLoadingHtmlString() && if (context && !context->IsLoadingErrorPage() &&
(base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) || !context->IsLoadingHtmlString() && !IsWKInternalUrl(newURL) &&
!context->IsLoadingErrorPage()) && !newURL.SchemeIs(url::kAboutScheme) && self.webView) {
!IsWKInternalUrl(newURL) && !newURL.SchemeIs(url::kAboutScheme) &&
self.webView) {
// On iOS13, WebKit started changing the URL visible webView.URL when // On iOS13, WebKit started changing the URL visible webView.URL when
// opening a new tab and then writing to it, e.g. // opening a new tab and then writing to it, e.g.
// window.open('javascript:document.write(1)'). This URL is never commited, // window.open('javascript:document.write(1)'). This URL is never commited,
......
...@@ -408,8 +408,7 @@ enum class BackForwardNavigationType { ...@@ -408,8 +408,7 @@ enum class BackForwardNavigationType {
if (IsRestoreSessionUrl(net::GURLWithNSURL(self.webView.URL))) if (IsRestoreSessionUrl(net::GURLWithNSURL(self.webView.URL)))
return; return;
if (!base::FeatureList::IsEnabled(web::features::kUseJSForErrorPage) && if (context && context->IsLoadingErrorPage())
context && context->IsLoadingErrorPage())
return; return;
if (!loadSuccess) { if (!loadSuccess) {
......
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