[iOS][web] Update ErrorRetryStateMachine when committed navigation is…

… cancelled

The ErrorRetryStateMachine is responsible for determining how to handle
a failed load -- whether to first load a placeholder URL, creating a new
back / forward item, or to directly load an error view into the current
item. When there's an error on a previously-visited item, the right thing
to do is to directly load an error view into the current item, since loading
a placeholder URL will destroy the forward list.

For navigations that are cancelled after they are committed (e.g., because
the user enters another URL to navigate to before the navigation finishes),
[CRWWebController handleLoadError] earlies-out after calling
|handleCancelledError|, which means it skips updating the
ErrorRetryStateMachine. This leaves the state machine in state |kNewRequest|. As
a result, if a later back / forward navigation to this item fails, a placeholder
will be loaded, destroying the forward list.

This CL makes |handleCancelledError| update the ErrorRetryStateMachine to state
|kNoNavigationError| for navigations that have already committed. This means
that future failed loads of this item will cause the error view to be loaded
into the current item, preserving the forward list.

Bug: 950489
Change-Id: I39dc2d5587525875b8f025ee4f9bd7cb94dd955f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1629000
Commit-Queue: Ali Juma <ajuma@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663570}

ios/web/web_state/ui/crw_web_controller.mm

@@ -4273,11 +4273,10 @@ - (void)handleLoadError:(NSError*)error
 - (void)handleCancelledError:(NSError*)error
                forNavigation:(WKNavigation*)navigation
              provisionalLoad:(BOOL)provisionalLoad {
+  web::NavigationContextImpl* navigationContext =
+      [self.navigationHandler.navigationStates contextForNavigation:navigation];
   if ([self shouldCancelLoadForCancelledError:error
                               provisionalLoad:provisionalLoad]) {
-    web::NavigationContextImpl* navigationContext =
-        [self.navigationHandler.navigationStates
-            contextForNavigation:navigation];
     [self.navigationHandler loadCancelled];
     self.navigationManagerImpl->DiscardNonCommittedItems();
 
@@ -4287,6 +4286,18 @@ - (void)handleCancelledError:(NSError*)error
     if (provisionalLoad) {
       self.webStateImpl->OnNavigationFinished(navigationContext);
     }
+  } else if (!provisionalLoad) {
+    web::NavigationItemImpl* item =
+        web::GetItemWithUniqueID(self.navigationManagerImpl, navigationContext);
+    if (item) {
+      // Since the navigation has already been committed, it will retain its
+      // back / forward item even though the load has been cancelled. Update the
+      // error state machine so that if future loads of this item fail, the same
+      // item will be reused for the error view rather than loading a
+      // placeholder URL into a new navigation item, since the latter would
+      // destroy the forward list.
+      item->error_retry_state_machine().SetNoNavigationError();
+    }
   }
 }
 

ios/web/web_state/ui/crw_web_controller_unittest.mm

@@ -285,6 +285,32 @@ void SetWebViewURL(NSString* url_string) {
   EXPECT_FALSE(GetWebClient()->last_cert_error_overridable());
 }
 
+// Tests that when a committed but not-yet-finished navigation is cancelled,
+// the navigation item's ErrorRetryStateMachine is updated correctly.
+TEST_P(CRWWebControllerTest, CancelCommittedNavigation) {
+  [[[mock_web_view_ stub] andReturnBool:NO] hasOnlySecureContent];
+  [static_cast<WKWebView*>([[mock_web_view_ stub] andReturn:@""]) title];
+
+  WKNavigation* navigation =
+      static_cast<WKNavigation*>([[NSObject alloc] init]);
+  SetWebViewURL(@"http://chromium.test");
+  [navigation_delegate_ webView:mock_web_view_
+      didStartProvisionalNavigation:navigation];
+  [fake_wk_list_ setCurrentURL:@"http://chromium.test"];
+  [navigation_delegate_ webView:mock_web_view_ didCommitNavigation:navigation];
+  NSError* error = [NSError errorWithDomain:NSURLErrorDomain
+                                       code:NSURLErrorCancelled
+                                   userInfo:nil];
+  [navigation_delegate_ webView:mock_web_view_
+              didFailNavigation:navigation
+                      withError:error];
+  NavigationManagerImpl& navigation_manager =
+      web_controller().webStateImpl->GetNavigationManagerImpl();
+  NavigationItemImpl* item = navigation_manager.GetLastCommittedItemImpl();
+  EXPECT_EQ(ErrorRetryState::kNoNavigationError,
+            item->error_retry_state_machine().state());
+}
+
 // Tests that when a placeholder navigation is preempted by another navigation,
 // WebStateObservers get neither a DidStartNavigation nor a DidFinishNavigation
 // call for the corresponding native URL navigation.