[bfcache] Delete loader when restarting navigation.

Delete URLLoader (which in this case is CachedNavigationURLLoader) when
restarting a back-forward cache navigation as a non-cached one to
cancel associated tasks and prevent to-be-cancelled navigation from
making further progress.

R=arthursonzogni@chromium.org,rakina@chromium.org
BUG=1122935

Change-Id: Ice6a9ada33e575517bbbe8da07e3034e5c371b6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461328
Commit-Queue: Alexander Timin <altimin@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815362}

content/browser/back_forward_cache_browsertest.cc

@@ -3441,6 +3441,47 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
       FROM_HERE);
 }
 
+// Same test as above, but with eviction happening before URL loader starts a
+// response.
+IN_PROC_BROWSER_TEST_F(
+    BackForwardCacheBrowserTest,
+    ReissuesNavigationIfEvictedDuringNavigation_BeforeResponse) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  GURL url_b(embedded_test_server()->GetURL("b.com", "/title2.html"));
+
+  // 1) Navigate to page A.
+  EXPECT_TRUE(NavigateToURL(shell(), url_a));
+  RenderFrameHostImpl* rfh_a = current_frame_host();
+  RenderFrameDeletedObserver delete_observer_rfh_a(rfh_a);
+
+  // 2) Navigate to page B.
+  EXPECT_TRUE(NavigateToURL(shell(), url_b));
+  RenderFrameHostImpl* rfh_b = current_frame_host();
+  RenderFrameDeletedObserver delete_observer_rfh_b(rfh_b);
+  EXPECT_FALSE(delete_observer_rfh_a.deleted());
+  EXPECT_TRUE(rfh_a->IsInBackForwardCache());
+  EXPECT_NE(rfh_a, rfh_b);
+
+  // 3) Start navigation to page A, and cause the document to be evicted during
+  // the navigation immediately before navigation makes any meaningful progress.
+  web_contents()->GetController().GoBack();
+  EvictByJavaScript(rfh_a);
+
+  // rfh_a should have been deleted, and page A navigated to normally.
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  delete_observer_rfh_a.WaitUntilDeleted();
+  RenderFrameHostImpl* rfh_a2 = current_frame_host();
+  EXPECT_NE(rfh_a2, rfh_b);
+  EXPECT_EQ(rfh_a2->GetLastCommittedURL(), url_a);
+
+  ExpectOutcome(BackForwardCacheMetrics::HistoryNavigationOutcome::kNotRestored,
+                FROM_HERE);
+  ExpectNotRestored(
+      {BackForwardCacheMetrics::NotRestoredReason::kJavaScriptExecution},
+      FROM_HERE);
+}
+
 // Similar to ReissuesNavigationIfEvictedDuringNavigation, except that
 // BackForwardCache::Flush is the source of the eviction.
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,

content/browser/renderer_host/navigation_request.cc

@@ -2267,6 +2267,10 @@ void NavigationRequest::OnResponseStarted(
 
   // Select an appropriate renderer to commit the navigation.
   if (IsServedFromBackForwardCache()) {
+    // If the current navigation is being restarted, it should not try to make
+    // any further progress.
+    DCHECK(!restarting_back_forward_cached_navigation_);
+
     NavigationControllerImpl* controller = GetNavigationController();
     render_frame_host_ = controller->GetBackForwardCache()
                              .GetEntry(nav_entry_id_)
@@ -2275,8 +2279,9 @@ void NavigationRequest::OnResponseStarted(
     // evicted from the BackForwardCache since this navigation started.
     //
     // If the document was evicted, the navigation should have been re-issued
-    // (deleting this NavigationRequest), so we should never reach this point
-    // without the document still present in the BackForwardCache.
+    // (deleting the URL loader and eventually this NavigationRequest), so we
+    // should never reach this point without the document still present in the
+    // BackForwardCache.
     CHECK(render_frame_host_);
   } else if (response_should_be_rendered_) {
     render_frame_host_ =
@@ -4894,6 +4899,9 @@ void NavigationRequest::RestartBackForwardCachedNavigation() {
       FROM_HERE,
       base::BindOnce(&NavigationRequest::RestartBackForwardCachedNavigationImpl,
                      weak_factory_.GetWeakPtr()));
+  // Delete the loader to ensure that it does not try to commit current
+  // navigation before the task above deletes it.
+  loader_.reset();
 }
 
 void NavigationRequest::RestartBackForwardCachedNavigationImpl() {