Commit 604482d1 authored by Justin Cohen's avatar Justin Cohen Committed by Commit Bot

[ios] Reload offline pages during fast WKBackForwardList navigations.

Offline pages can leave the WKBackForwardList current item as a placeholder with
no saved content.  In this case, trigger a retry on that navigation with an
updated NavigationItem url and NavigationContext error.

Bug: 875761
Change-Id: Iff5e2688b76da59755726097ac955efefded39d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1573082
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarDanyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652266}
parent 2554bb86
...@@ -186,6 +186,7 @@ source_set("eg_tests") { ...@@ -186,6 +186,7 @@ source_set("eg_tests") {
"//ios/third_party/earl_grey:earl_grey+link", "//ios/third_party/earl_grey:earl_grey+link",
"//ios/third_party/material_components_ios", "//ios/third_party/material_components_ios",
"//ios/web", "//ios/web",
"//ios/web/common:common",
"//ios/web/public", "//ios/web/public",
"//ios/web/public/test", "//ios/web/public/test",
"//ios/web/public/test/http_server", "//ios/web/public/test/http_server",
......
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
#import "ios/chrome/test/earl_grey/chrome_matchers.h" #import "ios/chrome/test/earl_grey/chrome_matchers.h"
#import "ios/chrome/test/earl_grey/chrome_test_case.h" #import "ios/chrome/test/earl_grey/chrome_test_case.h"
#import "ios/third_party/material_components_ios/src/components/Snackbar/src/MaterialSnackbar.h" #import "ios/third_party/material_components_ios/src/components/Snackbar/src/MaterialSnackbar.h"
#import "ios/web/common/features.h"
#import "ios/web/public/navigation_manager.h" #import "ios/web/public/navigation_manager.h"
#import "ios/web/public/reload_type.h" #import "ios/web/public/reload_type.h"
#import "ios/web/public/test/web_view_content_test_util.h" #import "ios/web/public/test/web_view_content_test_util.h"
...@@ -639,6 +640,13 @@ void AssertIsShowingDistillablePage(bool online, const GURL& distillable_url) { ...@@ -639,6 +640,13 @@ void AssertIsShowingDistillablePage(bool online, const GURL& distillable_url) {
web::ReloadType::NORMAL, false); web::ReloadType::NORMAL, false);
AssertIsShowingDistillablePage(false, distillableURL); AssertIsShowingDistillablePage(false, distillableURL);
// TODO(crbug.com/954248) This DCHECK's (but works) with slimnav disabled.
if (base::FeatureList::IsEnabled(web::features::kSlimNavigationManager)) {
[ChromeEarlGrey goBack];
[ChromeEarlGrey goForward];
AssertIsShowingDistillablePage(false, distillableURL);
}
// Start server to reload online error. // Start server to reload online error.
self.serverRespondsWithContent = YES; self.serverRespondsWithContent = YES;
base::test::ios::SpinRunLoopWithMinDelay( base::test::ios::SpinRunLoopWithMinDelay(
...@@ -678,6 +686,10 @@ void AssertIsShowingDistillablePage(bool online, const GURL& distillable_url) { ...@@ -678,6 +686,10 @@ void AssertIsShowingDistillablePage(bool online, const GURL& distillable_url) {
AssertIsShowingDistillablePage(false, distillableURL); AssertIsShowingDistillablePage(false, distillableURL);
[ChromeEarlGrey goBack];
[ChromeEarlGrey goForward];
AssertIsShowingDistillablePage(false, distillableURL);
// Reload should load online page. // Reload should load online page.
chrome_test_util::GetCurrentWebState()->GetNavigationManager()->Reload( chrome_test_util::GetCurrentWebState()->GetNavigationManager()->Reload(
web::ReloadType::NORMAL, false); web::ReloadType::NORMAL, false);
......
...@@ -35,6 +35,9 @@ enum class ErrorRetryState { ...@@ -35,6 +35,9 @@ enum class ErrorRetryState {
// This navigation item failed to load and is in the process of loading a // This navigation item failed to load and is in the process of loading a
// placeholder. // placeholder.
kLoadingPlaceholder, kLoadingPlaceholder,
// This navigation item failed to load and was stored in the WKBackForwardList
// and is now being reloaded.
kRetryPlaceholderNavigation,
// This navigation item has an entry in WKBackForwardList. Ready to present // This navigation item has an entry in WKBackForwardList. Ready to present
// error in native view. // error in native view.
kReadyToDisplayErrorForFailedNavigation, kReadyToDisplayErrorForFailedNavigation,
...@@ -86,6 +89,9 @@ class ErrorRetryStateMachine { ...@@ -86,6 +89,9 @@ class ErrorRetryStateMachine {
// Transitions the state machine to kDisplayingWebErrorForFailedNavigation. // Transitions the state machine to kDisplayingWebErrorForFailedNavigation.
void SetDisplayingWebError(); void SetDisplayingWebError();
// Transitions the state machine to kRetryPlaceholderNavigation.
void SetRetryPlaceholderNavigation();
// Runs state transitions upon a failed provisional navigation. // Runs state transitions upon a failed provisional navigation.
ErrorRetryCommand DidFailProvisionalNavigation(const GURL& web_view_url, ErrorRetryCommand DidFailProvisionalNavigation(const GURL& web_view_url,
const GURL& error_url); const GURL& error_url);
......
...@@ -49,6 +49,11 @@ void ErrorRetryStateMachine::SetDisplayingWebError() { ...@@ -49,6 +49,11 @@ void ErrorRetryStateMachine::SetDisplayingWebError() {
state_ = ErrorRetryState::kDisplayingWebErrorForFailedNavigation; state_ = ErrorRetryState::kDisplayingWebErrorForFailedNavigation;
} }
void ErrorRetryStateMachine::SetRetryPlaceholderNavigation() {
DCHECK(state_ == web::ErrorRetryState::kNoNavigationError);
state_ = ErrorRetryState::kRetryPlaceholderNavigation;
}
ErrorRetryCommand ErrorRetryStateMachine::DidFailProvisionalNavigation( ErrorRetryCommand ErrorRetryStateMachine::DidFailProvisionalNavigation(
const GURL& web_view_url, const GURL& web_view_url,
const GURL& error_url) { const GURL& error_url) {
...@@ -82,6 +87,7 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFailProvisionalNavigation( ...@@ -82,6 +87,7 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFailProvisionalNavigation(
} }
case ErrorRetryState::kLoadingPlaceholder: case ErrorRetryState::kLoadingPlaceholder:
case ErrorRetryState::kRetryPlaceholderNavigation:
case ErrorRetryState::kReadyToDisplayErrorForFailedNavigation: case ErrorRetryState::kReadyToDisplayErrorForFailedNavigation:
case ErrorRetryState::kNavigatingToFailedNavigationItem: case ErrorRetryState::kNavigatingToFailedNavigationItem:
NOTREACHED() << "Unexpected error retry state: " NOTREACHED() << "Unexpected error retry state: "
...@@ -108,6 +114,7 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFailNavigation( ...@@ -108,6 +114,7 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFailNavigation(
return BackForwardOrReloadFailed(web_view_url, error_url); return BackForwardOrReloadFailed(web_view_url, error_url);
case ErrorRetryState::kLoadingPlaceholder: case ErrorRetryState::kLoadingPlaceholder:
case ErrorRetryState::kRetryPlaceholderNavigation:
case ErrorRetryState::kReadyToDisplayErrorForFailedNavigation: case ErrorRetryState::kReadyToDisplayErrorForFailedNavigation:
case ErrorRetryState::kNavigatingToFailedNavigationItem: case ErrorRetryState::kNavigatingToFailedNavigationItem:
NOTREACHED() << "Unexpected error retry state: " NOTREACHED() << "Unexpected error retry state: "
...@@ -126,6 +133,20 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFinishNavigation( ...@@ -126,6 +133,20 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFinishNavigation(
state_ = ErrorRetryState::kReadyToDisplayErrorForFailedNavigation; state_ = ErrorRetryState::kReadyToDisplayErrorForFailedNavigation;
return ErrorRetryCommand::kLoadErrorView; return ErrorRetryCommand::kLoadErrorView;
case ErrorRetryState::kRetryPlaceholderNavigation:
if (wk_navigation_util::IsPlaceholderUrl(web_view_url)) {
// (11) Explicitly keep the state the same so after rewriting to the non
// placeholder url the else block will trigger.
DCHECK_EQ(web_view_url,
wk_navigation_util::CreatePlaceholderUrlForUrl(url_));
state_ = ErrorRetryState::kRetryPlaceholderNavigation;
return ErrorRetryCommand::kRewriteWebViewURL;
} else {
// The url was written by kRewriteWebViewURL in the if block, so on
// this navigation load an error view.
state_ = ErrorRetryState::kReadyToDisplayErrorForFailedNavigation;
return ErrorRetryCommand::kLoadErrorView;
}
case ErrorRetryState::kNewRequest: case ErrorRetryState::kNewRequest:
if (wk_navigation_util::IsRestoreSessionUrl(web_view_url)) { if (wk_navigation_util::IsRestoreSessionUrl(web_view_url)) {
// (8) Initial load of restore_session.html. Don't change state or // (8) Initial load of restore_session.html. Don't change state or
......
...@@ -248,5 +248,34 @@ TEST_F(ErrorRetryStateMachineTest, OfflineAfterColdStart) { ...@@ -248,5 +248,34 @@ TEST_F(ErrorRetryStateMachineTest, OfflineAfterColdStart) {
machine.state()); machine.state());
} }
// Tests retrying a placeholder navigation.
TEST_F(ErrorRetryStateMachineTest, RetryPlaceholderNavigation) {
GURL test_url(kTestUrl);
ErrorRetryStateMachine machine;
machine.SetURL(test_url);
ASSERT_EQ(ErrorRetryState::kNewRequest, machine.state());
// First trigger the cached placeholder load.
const GURL placeholder_url =
wk_navigation_util::CreatePlaceholderUrlForUrl(test_url);
ASSERT_EQ(ErrorRetryCommand::kDoNothing,
machine.DidFinishNavigation(placeholder_url));
ASSERT_EQ(ErrorRetryState::kNoNavigationError, machine.state());
// Then trigger a retry.
machine.SetRetryPlaceholderNavigation();
ASSERT_EQ(ErrorRetryCommand::kRewriteWebViewURL,
machine.DidFinishNavigation(placeholder_url));
ASSERT_EQ(ErrorRetryState::kRetryPlaceholderNavigation, machine.state());
// Lastly trigger the error view.
const GURL target_url =
wk_navigation_util::ExtractUrlFromPlaceholderUrl(placeholder_url);
ASSERT_EQ(ErrorRetryCommand::kLoadErrorView,
machine.DidFinishNavigation(target_url));
ASSERT_EQ(ErrorRetryState::kReadyToDisplayErrorForFailedNavigation,
machine.state());
}
} // namespace } // namespace
} // namespace web } // namespace web
...@@ -4955,6 +4955,23 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*); ...@@ -4955,6 +4955,23 @@ typedef void (^ViewportStateCompletion)(const web::PageViewportState*);
if (web::features::StorePendingItemInContext() && context->GetItem()) { if (web::features::StorePendingItemInContext() && context->GetItem()) {
self.navigationManagerImpl->SetPendingItem(context->ReleaseItem()); self.navigationManagerImpl->SetPendingItem(context->ReleaseItem());
} }
} else if (web::GetWebClient()->IsSlimNavigationManagerEnabled() &&
item->error_retry_state_machine().state() ==
web::ErrorRetryState::kNoNavigationError) {
// Offline pages can leave the WKBackForwardList current item as a
// placeholder with no saved content. In this case, trigger a retry
// on that navigation with an update |item| url and |context| error.
item->SetURL(
ExtractUrlFromPlaceholderUrl(net::GURLWithNSURL(self.webView.URL)));
item->SetVirtualURL(item->GetURL());
context->SetError([NSError
errorWithDomain:NSURLErrorDomain
code:NSURLErrorNetworkConnectionLost
userInfo:@{
NSURLErrorFailingURLStringErrorKey :
base::SysUTF8ToNSString(item->GetURL().spec())
}]);
item->error_retry_state_machine().SetRetryPlaceholderNavigation();
} }
} }
......
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