bfcache: Flush before normal-history-cross-BrowsingInstance navigation.

The BackForwardCache relies on pages to live in different
BrowsingInstance. It means the current page and every pages in the
BackForwardCache aren't sharing the same BrowsingInstance.

The problem is that normal history navigation can specify a SiteInstance
to reuse. It will be reused even when |force_browsing_instance_swap| is
true in the RenderFrameHostManagerImpl.

The basic idea of this CL: flush the BFCache before attempting a
cross-BrowsingInstance history navigation.

Bug: 993337, 999846
Change-Id: I26f1726a6ebe35eecb343b7310563e5001fc8f43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1795902Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Lowell Manners <lowell@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695959}

content/browser/back_forward_cache_browsertest.cc

@@ -1899,6 +1899,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest, CachePagesWithBeacon) {
   EXPECT_TRUE(rfh_a->is_in_back_forward_cache());
 }
 
+// Regression test for https://crbug.com/993337.
 IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
                        NavigateToTwoPagesOnSameSite) {
   ASSERT_TRUE(embedded_test_server()->Start());
@@ -1912,18 +1913,21 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTest,
   // 2) Navigate to A2.
   EXPECT_TRUE(NavigateToURL(shell(), url_a2));
   RenderFrameHostImpl* rfh_a2 = current_frame_host();
+  RenderFrameDeletedObserver delete_rfh_a2(current_frame_host());
 
   // 3) Navigate to B1.
   EXPECT_TRUE(NavigateToURL(shell(), url_b1));
   EXPECT_TRUE(rfh_a2->is_in_back_forward_cache());
+  RenderFrameHostImpl* rfh_b1 = current_frame_host();
 
   // 4) Do a history navigation back to A1.
-  // TODO(https://crbug.com/993337): This causes "Check failed: !frame_widget_".
-  RenderProcessHost* process = rfh_a2->GetProcess();
-  RenderProcessHostWatcher crash_observer(
-      process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
   web_contents()->GetController().GoToIndex(0);
-  crash_observer.Wait();
+  EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_TRUE(rfh_b1->is_in_back_forward_cache());
+
+  // As a result, rfh_a2 is deleted. The history navigation tried to restore an
+  // entry using the same BrowsingInstance. They both can't live together.
+  delete_rfh_a2.WaitUntilDeleted();
 }
 
 class GeolocationBackForwardCacheBrowserTest

content/browser/frame_host/navigation_controller_impl.cc

@@ -2536,6 +2536,20 @@ void NavigationControllerImpl::NavigateToExistingPendingEntry(
     return;
   }
 
+  // By design, a page in the BackForwardCache is alone in its BrowsingInstance.
+  // History navigation might try to reuse a specific SiteInstance, already used
+  // by a page in the cache. This must not happen. It would fail creating the
+  // RenderFrame, because only one main document can live there. For this
+  // reason, the BackForwardCache is flushed.
+  // TODO(arthursonzogni): Flushing the entire cache is a bit overkill, this can
+  // be refined to only delete the page (if any) using the same
+  // BrowsingInstance.
+  if (pending_entry_->site_instance()) {
+    SiteInstance* current = root->current_frame_host()->GetSiteInstance();
+    if (!current->IsRelatedSiteInstance(pending_entry_->site_instance()))
+      back_forward_cache_.Flush();
+  }
+
   // If we were navigating to a slow-to-commit page, and the user performs
   // a session history navigation to the last committed page, RenderViewHost
   // will force the throbber to start, but WebKit will essentially ignore the

testing/buildbot/filters/bfcache.content_browsertests.filter

@@ -18,14 +18,6 @@
 # Document expects javascript to run again from the beginning.
 -WebContentsImplBrowserTest.JavaScriptDialogsInMainAndSubframes
 
-# render_view_impl.cc. Check failed: !frame_widget_.
-# https://crbug.com/999846
--NavigationControllerBrowserTest.PageStateAfterForwardInCompetingFrames
--NavigationControllerBrowserTest.PageStateWithIframeAfterForwardInCompetingFrames
--NavigationControllerHistoryInterventionBrowserTest.NoUserActivationSetSkipOnBackForwardCrossSite
--NavigationControllerHistoryInterventionBrowserTest.NoUserActivationSetSkipOnBackForwardCrossSite
--RenderFrameHostManagerTest.BackForwardNotStale
-
 # Javascript dialog is shown from a page in the BackForwardCache.
 # https://crbug.com/999847
 -NavigationControllerBrowserTest.NoDialogsFromSwappedOutFrames