Reland "Use the same SessionStorageNamespace for prerendering"

This is a reland of eefb8c5

The original CL was reverted because the
PrerenderBackForwardCacheBrowserTest was flaky. The test didn’t check
the behavior of BackForwardCache. It was just running the same tests of
PrerenderBrowserTest.SessionStorageAfterBackNavigation_NoProcessReuse or
SessionStorageAfterBackNavigation_KeepInitialProcess. If the initial
process have been killed before the back navigation, the test failed.

To make the BackForwardCache logic work this CL changed the browser test
as followings:
  - Added enable_same_site flag.
  - Stopped using BroadcastChannel which prevent BFCache.

PS1 is the same as the original CL.


Original change's description:
> Use the same SessionStorageNamespace for prerendering
>
> Currently there is an issue that the Session Storage is not carried over
> to the prerendering page. This is because a new Session Storage
> Namespace is used for the prerendering page.
>
> To fix this issue, this CL changes PrerenderHost::PageHolder to copy the
> Session Storage Namespace from the initiator page to the prerendering
> page.
>
> We don’t want the Session Storage state in the storage service be
> updated by the prerendering page. And we want to synchronize the Session
> Storage state of the prerendering page with the initiator page when the
> prerendering page is activated. So this CL introduces a flag
> |is_session_storage_for_prerendering_| in CachedStorageArea, and make
> CachedStorageArea not to send the changes of the Session Storage state
> to the storage service, and make StorageArea recreate |cached_area_|
> when the prerendering page is activated.
>
> This is the "clone & swap" mechanism for session storage in prerendering
> described in whatwg/storage#119.
>
> This CL still has an issue that when the initial renderer process is
> reused after the back navigation from a prerendered page, the Session
> Storage state is not correctly propagated to the initial renderer
> process. This issue will be fixed in the next CL.
> https://crrev.com/c/2849654
>
> Bug: 1197383
> Change-Id: Ib43386ccf75f8c867bcddb4b77b333ee0d5b5d79
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2850242
> Reviewed-by: Kinuko Yasuda <[email protected]>
> Reviewed-by: Matt Falkenhagen <[email protected]>
> Reviewed-by: Marijn Kruisselbrink <[email protected]>
> Commit-Queue: Tsuyoshi Horo <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#881985}

Bug: 1197383
Change-Id: If83c11d44e37b598111ab1c5ce4a78dfd3757176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2891279
Reviewed-by: Hiroki Nakagawa <[email protected]>
Reviewed-by: Kinuko Yasuda <[email protected]>
Reviewed-by: Fergal Daly <[email protected]>
Reviewed-by: Matt Falkenhagen <[email protected]>
Commit-Queue: Tsuyoshi Horo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#883819}

content/browser/prerender/prerender_browsertest.cc

@@ -18,17 +18,22 @@
 #include "base/test/scoped_feature_list.h"
 #include "base/thread_annotations.h"
 #include "build/build_config.h"
+#include "components/services/storage/public/mojom/storage_service.mojom.h"
+#include "components/services/storage/public/mojom/test_api.test-mojom.h"
 #include "content/browser/file_system_access/file_system_chooser_test_helpers.h"
 #include "content/browser/prerender/prerender_host.h"
 #include "content/browser/prerender/prerender_host_registry.h"
 #include "content/browser/prerender/prerender_metrics.h"
+#include "content/browser/renderer_host/back_forward_cache_impl.h"
 #include "content/browser/renderer_host/frame_tree_node.h"
 #include "content/browser/renderer_host/input/synthetic_tap_gesture.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/storage_partition_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_document_host_user_data.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
@@ -237,12 +242,13 @@ class PrerenderBrowserTest : public ContentBrowserTest {
     return prerender_helper_.get();
   }
 
- private:
-  void SetUpCommandLine(base::CommandLine* command_line) final {
+  void SetUpCommandLine(base::CommandLine* command_line) override {
     // Useful for testing CSP:prefetch-src
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
   }
+
+ private:
   net::test_server::EmbeddedTestServer ssl_server_{
       net::test_server::EmbeddedTestServer::TYPE_HTTPS};
 
@@ -1781,6 +1787,274 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, LazyLoading) {
   EXPECT_EQ(GetRequestCount(kImageUrl), 1);
 }
 
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       SessionStorageAfterBackNavigation_NoProcessReuse) {
+  const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");
+  const GURL kPrerenderingUrl =
+      GetUrl("/prerender/session_storage.html?prerendering=");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  std::unique_ptr<RenderProcessHostWatcher> process_host_watcher =
+      std::make_unique<RenderProcessHostWatcher>(
+          current_frame_host()->GetProcess(),
+          RenderProcessHostWatcher::WATCH_FOR_HOST_DESTRUCTION);
+
+  AddPrerender(kPrerenderingUrl);
+  NavigatePrimaryPage(kPrerenderingUrl);
+
+  EXPECT_EQ("initial", EvalJs(current_frame_host(),
+                              "window.sessionKeysInPrerenderingchange")
+                           .ExtractString());
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+
+  // Make sure that the initial renderer process is destroyed. So that the
+  // initial renderer process will not be reused after the back forward
+  // navigation below.
+  process_host_watcher->Wait();
+
+  // Navigate back to the initial page.
+  content::TestNavigationObserver observer(shell()->web_contents());
+  shell()->GoBackOrForward(-1);
+  observer.Wait();
+  EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(), kInitialUrl);
+
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
+                       SessionStorageAfterBackNavigation_KeepInitialProcess) {
+  const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");
+  const GURL kPrerenderingUrl =
+      GetUrl("/prerender/session_storage.html?prerendering=");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  RenderProcessHostImpl* initial_process_host =
+      static_cast<RenderProcessHostImpl*>(current_frame_host()->GetProcess());
+  // Increment the keep alive ref count of the renderer process to keep it alive
+  // so it is reused on the back navigation below. The test checks that the
+  // session storage state changed in the activated page is correctly propagated
+  // after a back navigation that uses an existing renderer process. (Note: This
+  // is not working correctly now.)
+  initial_process_host->IncrementKeepAliveRefCount();
+
+  AddPrerender(kPrerenderingUrl);
+  NavigatePrimaryPage(kPrerenderingUrl);
+
+  EXPECT_EQ("initial", EvalJs(current_frame_host(),
+                              "window.sessionKeysInPrerenderingchange")
+                           .ExtractString());
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+
+  // Navigate back to the initial page.
+  content::TestNavigationObserver observer(shell()->web_contents());
+  shell()->GoBackOrForward(-1);
+  observer.Wait();
+  EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(), kInitialUrl);
+
+  // There is a known issue that when the initial renderer process is reused
+  // after the back navigation, the session storage state changed in the
+  // activated is not correctly propagated to the initial renderer process.
+  // TODO(crbug.com/1197383): Fix this issue.
+  EXPECT_EQ(
+      // This should be "activated, initial".
+      "initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+}
+
+class PrerenderSingleProcessBrowserTest : public PrerenderBrowserTest {
+ public:
+  PrerenderSingleProcessBrowserTest() = default;
+
+  void SetUpCommandLine(base::CommandLine* cmd_line) override {
+    PrerenderBrowserTest::SetUpCommandLine(cmd_line);
+    cmd_line->AppendSwitch("single-process");
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(PrerenderSingleProcessBrowserTest,
+                       SessionStorageAfterBackNavigation) {
+  const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");
+  const GURL kPrerenderingUrl =
+      GetUrl("/prerender/session_storage.html?prerendering=");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  AddPrerender(kPrerenderingUrl);
+  NavigatePrimaryPage(kPrerenderingUrl);
+
+  EXPECT_EQ("initial", EvalJs(current_frame_host(),
+                              "window.sessionKeysInPrerenderingchange")
+                           .ExtractString());
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+
+  // Navigate back to the initial page.
+  content::TestNavigationObserver observer(shell()->web_contents());
+  shell()->GoBackOrForward(-1);
+  observer.Wait();
+  EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(), kInitialUrl);
+
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+}
+
+class PrerenderBackForwardCacheBrowserTest : public PrerenderBrowserTest {
+ public:
+  PrerenderBackForwardCacheBrowserTest() {
+    feature_list_.InitWithFeaturesAndParameters(
+        {{features::kBackForwardCache, {{"enable_same_site", "true"}}},
+         {kBackForwardCacheNoTimeEviction, {}}},
+        // Allow BackForwardCache for all devices regardless of their memory.
+        {features::kBackForwardCacheMemoryControls});
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(PrerenderBackForwardCacheBrowserTest,
+                       SessionStorageAfterBackNavigation) {
+  const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");
+  const GURL kPrerenderingUrl =
+      GetUrl("/prerender/session_storage.html?prerendering=");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  RenderFrameDeletedObserver deleted_observer(
+      shell()->web_contents()->GetMainFrame());
+
+  AddPrerender(kPrerenderingUrl);
+  NavigatePrimaryPage(kPrerenderingUrl);
+
+  EXPECT_EQ("initial", EvalJs(current_frame_host(),
+                              "window.sessionKeysInPrerenderingchange")
+                           .ExtractString());
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+
+  // Navigate back to the initial page.
+  shell()->GoBackOrForward(-1);
+  WaitForLoadStop(shell()->web_contents());
+
+  // Expect the navigation to be served from the back-forward cache to verify
+  // the test is testing what is intended.
+  ASSERT_EQ(shell()->web_contents()->GetMainFrame(),
+            deleted_observer.render_frame_host());
+
+  // There is a known issue that when the initial renderer process is reused
+  // after the back navigation, the session storage state changed in the
+  // activated is not correctly propagated to the initial renderer process.
+  // TODO(crbug.com/1197383): Fix this issue.
+  EXPECT_EQ(
+      // This should be "activated, initial".
+      "initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+}
+
+#if !defined(OS_ANDROID)
+// StorageServiceOutOfProcess is not implemented on Android.
+
+class PrerenderRestartStorageServiceBrowserTest : public PrerenderBrowserTest {
+ public:
+  PrerenderRestartStorageServiceBrowserTest() {
+    // These tests only make sense when the service is running
+    // out-of-process.
+    feature_list_.InitAndEnableFeature(features::kStorageServiceOutOfProcess);
+  }
+
+ protected:
+  void CrashStorageServiceAndWaitForRestart() {
+    mojo::Remote<storage::mojom::StorageService>& service =
+        StoragePartitionImpl::GetStorageServiceForTesting();
+    base::RunLoop loop;
+    service.set_disconnect_handler(base::BindLambdaForTesting([&] {
+      loop.Quit();
+      service.reset();
+    }));
+    mojo::Remote<storage::mojom::TestApi> test_api;
+    StoragePartitionImpl::GetStorageServiceForTesting()->BindTestApi(
+        test_api.BindNewPipeAndPassReceiver().PassPipe());
+    test_api->CrashNow();
+    loop.Run();
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+IN_PROC_BROWSER_TEST_F(PrerenderRestartStorageServiceBrowserTest,
+                       RestartStorageServiceBeforePrerendering) {
+  const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");
+  const GURL kPrerenderingUrl =
+      GetUrl("/prerender/session_storage.html?prerendering=");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  CrashStorageServiceAndWaitForRestart();
+
+  EXPECT_EQ(
+      "initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+
+  AddPrerender(kPrerenderingUrl);
+  NavigatePrimaryPage(kPrerenderingUrl);
+
+  EXPECT_EQ("initial", EvalJs(current_frame_host(),
+                              "window.sessionKeysInPrerenderingchange")
+                           .ExtractString());
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+}
+
+IN_PROC_BROWSER_TEST_F(PrerenderRestartStorageServiceBrowserTest,
+                       RestartStorageServiceWhilePrerendering) {
+  const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");
+  const GURL kPrerenderingUrl =
+      GetUrl("/prerender/session_storage.html?prerendering=");
+
+  // Navigate to an initial page.
+  ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
+
+  const int host_id = AddPrerender(kPrerenderingUrl);
+
+  CrashStorageServiceAndWaitForRestart();
+
+  EXPECT_EQ(
+      "initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+  EXPECT_EQ(
+      "initial, prerendering",
+      EvalJs(GetPrerenderedMainFrameHost(host_id), "getSessionStorageKeys()")
+          .ExtractString());
+
+  NavigatePrimaryPage(kPrerenderingUrl);
+
+  EXPECT_EQ("initial", EvalJs(current_frame_host(),
+                              "window.sessionKeysInPrerenderingchange")
+                           .ExtractString());
+  EXPECT_EQ(
+      "activated, initial",
+      EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
+}
+#endif
+
 class PrerenderWithProactiveBrowsingInstanceSwap : public PrerenderBrowserTest {
  public:
   PrerenderWithProactiveBrowsingInstanceSwap() {

content/browser/prerender/prerender_host.cc

@@ -16,6 +16,7 @@
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/renderer_host/render_frame_proxy_host.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/site_instance_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/back_forward_cache.h"
 #include "content/public/browser/navigation_controller.h"
@@ -77,10 +78,20 @@ class PrerenderHost::PageHolder : public FrameTree::Delegate,
                                         &web_contents,
                                         &web_contents,
                                         FrameTree::Type::kPrerender)) {
-    frame_tree_->Init(
-        SiteInstance::Create(web_contents.GetBrowserContext()).get(),
-        /*renderer_initiated_creation=*/false,
-        /*main_frame_name=*/"");
+    scoped_refptr<SiteInstance> site_instance =
+        SiteInstance::Create(web_contents.GetBrowserContext());
+    frame_tree_->Init(site_instance.get(),
+                      /*renderer_initiated_creation=*/false,
+                      /*main_frame_name=*/"");
+
+    const auto& site_info =
+        static_cast<SiteInstanceImpl*>(site_instance.get())->GetSiteInfo();
+    // Use the same SessionStorageNamespace as the primary page for the
+    // prerendering page.
+    frame_tree_->controller().SetSessionStorageNamespace(
+        site_info.GetStoragePartitionId(site_instance->GetBrowserContext()),
+        web_contents_.GetFrameTree()->controller().GetSessionStorageNamespace(
+            site_info));
 
     // TODO(https://crbug.com/1199679): This should be moved to FrameTree::Init
     web_contents_.NotifySwappedFromRenderManager(

content/browser/renderer_host/back_forward_cache_impl.cc

@@ -39,11 +39,6 @@ namespace {
 
 using blink::scheduler::WebSchedulerTrackedFeature;
 
-// Removes the time limit for cached content. This is used on bots to identify
-// accidentally passing tests.
-const base::Feature kBackForwardCacheNoTimeEviction{
-    "BackForwardCacheNoTimeEviction", base::FEATURE_DISABLED_BY_DEFAULT};
-
 // The default number of entries the BackForwardCache can hold per tab.
 static constexpr size_t kDefaultBackForwardCacheSize = 1;
 

content/browser/renderer_host/back_forward_cache_impl.h

@@ -44,6 +44,11 @@ constexpr base::Feature kRecordBackForwardCacheMetricsWithoutEnabling{
     "RecordBackForwardCacheMetricsWithoutEnabling",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Removes the time limit for cached content. This is used on bots to identify
+// accidentally passing tests.
+constexpr base::Feature kBackForwardCacheNoTimeEviction{
+    "BackForwardCacheNoTimeEviction", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // BackForwardCache:
 //
 // After the user navigates away from a document, the old one goes into the