Fix going forward to error page if kWebErrorPages is enabled.

Added LoadingErrorPage flag to NavigationContext to mark
LoadHTMLString: navigations which feed Error page to
WKWebView. These navigations will be ignored in
WKWebViewNavigationDelegate callbacks.

GoBackFromErrorPage and testGoBackFromErrorPage tests were
updated to cover this test case.

Bug: 725241
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I1a38685d96997328c4be2a250dcedb5837e76d77
Reviewed-on: https://chromium-review.googlesource.com/1093515Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567281}

ios/chrome/browser/web/error_page_egtest.mm

@@ -129,7 +129,7 @@ NSString* GetNSErrorMessage() {
 // Sucessfully loads the page, then loads the URL which fails to load, then
 // sucessfully goes back to the first page.
 // TODO(crbug.com/840489): Remove this test.
-- (void)testGoBackFromErrorPage {
+- (void)testGoBackFromErrorPageAndForwardToErrorPage {
   // First page loads sucessfully.
   [ChromeEarlGrey loadURL:self.testServer->GetURL("/echo")];
   [ChromeEarlGrey waitForWebViewContainingText:"Echo"];
@@ -145,6 +145,14 @@ NSString* GetNSErrorMessage() {
   // Going back should sucessfully load the first page.
   [ChromeEarlGrey goBack];
   [ChromeEarlGrey waitForWebViewContainingText:"Echo"];
+
+  // Going forward fails the load.
+  [ChromeEarlGrey goForward];
+  if (base::FeatureList::IsEnabled(web::features::kWebErrorPages)) {
+    [ChromeEarlGrey waitForWebViewContainingText:GetErrorMessage()];
+  } else {
+    [ChromeEarlGrey waitForStaticHTMLViewContainingText:GetNSErrorMessage()];
+  }
 }
 
 // Loads the URL which redirects to unresponsive server.

ios/web/navigation/error_retry_state_machine.h

@@ -78,12 +78,6 @@ class ErrorRetryStateMachine {
   // Returns the current error retry state.
   ErrorRetryState state() const;
 
-  // Resets the state to kNewRequest. This is needed for legacy navigation
-  // manager when using features::kWebErrorPages.
-  // TODO(crbug.com/738020): Remove this state after legacy navigation manager
-  // is deprecated.
-  void ResetState();
-
   // Transitions the state machine to kDisplayingNativeErrorForFailedNavigation.
   void SetDisplayingNativeError();
 

ios/web/navigation/error_retry_state_machine.mm

@@ -28,10 +28,6 @@ ErrorRetryState ErrorRetryStateMachine::state() const {
   return state_;
 }
 
-void ErrorRetryStateMachine::ResetState() {
-  state_ = ErrorRetryState::kNewRequest;
-}
-
 void ErrorRetryStateMachine::SetDisplayingNativeError() {
   DCHECK_EQ(ErrorRetryState::kReadyToDisplayErrorForFailedNavigation, state_);
   state_ = ErrorRetryState::kDisplayingNativeErrorForFailedNavigation;
@@ -120,6 +116,14 @@ ErrorRetryCommand ErrorRetryStateMachine::DidFinishNavigation(
       break;
 
     case ErrorRetryState::kReadyToDisplayErrorForFailedNavigation:
+      if (web_view_url ==
+          wk_navigation_util::CreatePlaceholderUrlForUrl(url_)) {
+        // (4) Back/forward to or reload of placeholder URL. Rewrite WebView URL
+        // to prepare for retry.
+        state_ = ErrorRetryState::kNavigatingToFailedNavigationItem;
+        return ErrorRetryCommand::kRewriteWebViewURL;
+      }
+
       // (3) Finished loading error in web view.
       DCHECK_EQ(web_view_url, url_);
       state_ = ErrorRetryState::kDisplayingWebErrorForFailedNavigation;

ios/web/navigation/error_retry_state_machine_unittest.mm

@@ -81,9 +81,6 @@ TEST_F(ErrorRetryStateMachineTest, OfflineThenReload) {
     ASSERT_EQ(ErrorRetryState::kReadyToDisplayErrorForFailedNavigation,
               clone.state());
   }
-
-  machine.ResetState();
-  ASSERT_EQ(ErrorRetryState::kNewRequest, machine.state());
 }
 
 // Tests state transitions for displaying error in web view.

ios/web/web_state/error_page_inttest.mm

@@ -137,7 +137,7 @@ TEST_F(ErrorPageTest, GoForwardAfterServerIsDownAndReload) {
 
 // Sucessfully loads the page, then loads the URL which fails to load, then
 // sucessfully goes back to the first page.
-TEST_F(ErrorPageTest, GoBackFromErrorPage) {
+TEST_F(ErrorPageTest, GoBackFromErrorPageAndForwardToErrorPage) {
   // First page loads sucessfully.
   test::LoadUrl(web_state(), server_.GetURL("/echo"));
   ASSERT_TRUE(test::WaitForWebViewContainingText(web_state(), "Echo"));
@@ -150,6 +150,11 @@ TEST_F(ErrorPageTest, GoBackFromErrorPage) {
   // Going back should sucessfully load the first page.
   web_state()->GetNavigationManager()->GoBack();
   ASSERT_TRUE(test::WaitForWebViewContainingText(web_state(), "Echo"));
+
+  // Going forward fails the load.
+  web_state()->GetNavigationManager()->GoForward();
+  ASSERT_TRUE(test::WaitForWebViewContainingText(
+      web_state(), "domain: NSURLErrorDomain code: -1005 post: 0 otr: 0"));
 }
 
 // Loads the URL which redirects to unresponsive server.

ios/web/web_state/navigation_context_impl.h

@@ -68,6 +68,11 @@ class NavigationContextImpl : public NavigationContext {
   void SetWKNavigationType(WKNavigationType wk_navigation_type);
   WKNavigationType GetWKNavigationType() const;
 
+  // true if this navigation context is a loadHTMLString: navigation used to
+  // load Error page into web view.
+  bool IsLoadingErrorPage() const;
+  void SetLoadingErrorPage(bool is_loading_error_page);
+
  private:
   NavigationContextImpl(WebState* web_state,
                         const GURL& url,
@@ -89,6 +94,7 @@ class NavigationContextImpl : public NavigationContext {
   bool is_renderer_initiated_ = false;
   int navigation_item_unique_id_ = -1;
   WKNavigationType wk_navigation_type_ = WKNavigationTypeOther;
+  bool is_loading_error_page_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationContextImpl);
 };

ios/web/web_state/navigation_context_impl.mm

@@ -148,6 +148,14 @@ WKNavigationType NavigationContextImpl::GetWKNavigationType() const {
   return wk_navigation_type_;
 }
 
+bool NavigationContextImpl::IsLoadingErrorPage() const {
+  return is_loading_error_page_;
+}
+
+void NavigationContextImpl::SetLoadingErrorPage(bool is_loading_error_page) {
+  is_loading_error_page_ = is_loading_error_page;
+}
+
 NavigationContextImpl::NavigationContextImpl(WebState* web_state,
                                              const GURL& url,
                                              bool has_user_gesture,

ios/web/web_state/ui/crw_web_controller.mm

@@ -1749,7 +1749,18 @@ registerLoadRequestForURL:(const GURL&)requestURL
         error, context->IsPost(),
         _webStateImpl->GetBrowserState()->IsOffTheRecord(), &errorHTML);
 
-    [_webView loadHTMLString:errorHTML baseURL:net::NSURLWithGURL(currentURL)];
+    WKNavigation* navigation =
+        [_webView loadHTMLString:errorHTML
+                         baseURL:net::NSURLWithGURL(currentURL)];
+
+    auto loadHTMLContext = web::NavigationContextImpl::CreateNavigationContext(
+        _webStateImpl, GURL::EmptyGURL(),
+        /*has_user_gesture=*/false, ui::PAGE_TRANSITION_FIRST,
+        /*is_renderer_initiated=*/false);
+    loadHTMLContext->SetLoadingErrorPage(true);
+
+    [_navigationStates setContext:std::move(loadHTMLContext)
+                    forNavigation:navigation];
   }
 
   // If |context| has placeholder URL, this is the second part of a native error
@@ -4423,6 +4434,12 @@ registerLoadRequestForURL:(const GURL&)requestURL
   if (context) {
     // This is already seen and registered navigation.
 
+    if (context->IsLoadingErrorPage()) {
+      // This is loadHTMLString: navigation to display error page in web view.
+      _loadPhase = web::LOAD_REQUESTED;
+      return;
+    }
+
     if (context->GetUrl() != webViewURL) {
       // Update last seen URL because it may be changed by WKWebView (f.e. by
       // performing characters escaping).
@@ -4783,6 +4800,10 @@ registerLoadRequestForURL:(const GURL&)requestURL
 
   web::NavigationContextImpl* context =
       [_navigationStates contextForNavigation:navigation];
+  if (context && context->IsLoadingErrorPage()) {
+    return;
+  }
+
   if (context && context->GetUrl() == currentWKItemURL) {
     // If webView.backForwardList.currentItem.URL matches |context|, then this
     // is a known edge case where |webView.URL| is wrong.
@@ -5452,9 +5473,6 @@ registerLoadRequestForURL:(const GURL&)requestURL
     [_navigationStates setContext:std::move(navigationContext)
                     forNavigation:navigation];
     [self reportBackForwardNavigationTypeForFastNavigation:NO];
-    if (self.currentNavItem) {
-      self.currentNavItem->error_retry_state_machine().ResetState();
-    }
   };
 
   // When navigating via WKBackForwardListItem to pages created or updated by
@@ -5509,9 +5527,6 @@ registerLoadRequestForURL:(const GURL&)requestURL
                   forNavigation:navigation];
     [_navigationStates setContext:std::move(navigationContext)
                     forNavigation:navigation];
-    if (self.currentNavItem) {
-      self.currentNavItem->error_retry_state_machine().ResetState();
-    }
   };
 
   // If the request is not a form submission or resubmission, or the user