From c226ef4e5aaa66edbf29db3239e91bd49bcac2d2 Mon Sep 17 00:00:00 2001 From: Tsuyoshi Horo Date: Tue, 18 May 2021 05:59:54 +0000 Subject: [PATCH] Reland "Use the same SessionStorageNamespace for prerendering" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a reland of eefb8c561ab863863b0541125df363fef040eabb 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 https://github.com/whatwg/storage/issues/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 > Reviewed-by: Matt Falkenhagen > Reviewed-by: Marijn Kruisselbrink > Commit-Queue: Tsuyoshi Horo > 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 Reviewed-by: Kinuko Yasuda Reviewed-by: Fergal Daly Reviewed-by: Matt Falkenhagen Commit-Queue: Tsuyoshi Horo Cr-Commit-Position: refs/heads/master@{#883819} --- .../prerender/prerender_browsertest.cc | 278 +++++++++++++++++- content/browser/prerender/prerender_host.cc | 19 +- .../renderer_host/back_forward_cache_impl.cc | 5 - .../renderer_host/back_forward_cache_impl.h | 5 + .../test/data/prerender/session_storage.html | 40 +++ .../blink/renderer/core/dom/document.cc | 12 + .../blink/renderer/core/dom/document.h | 6 + .../modules/storage/cached_storage_area.cc | 30 +- .../modules/storage/cached_storage_area.h | 13 +- .../storage/cached_storage_area_test.cc | 2 +- .../modules/storage/dom_window_storage.cc | 12 +- .../renderer/modules/storage/storage_area.cc | 20 ++ .../renderer/modules/storage/storage_area.h | 5 +- .../modules/storage/storage_namespace.cc | 14 +- .../modules/storage/storage_namespace.h | 3 + ...-storage-carry-over-to-prerender-page.html | 20 ++ ...n-storage-isolated-while-prerendering.html | 41 +++ ...ion-storage-no-leak-to-initiator-page.html | 35 +++ .../session-storage-swap-after-activate.html | 76 +++++ .../resources/session-storage-utils.js | 76 +++++ .../prerender/session-storage.html | 27 ++ 21 files changed, 711 insertions(+), 28 deletions(-) create mode 100644 content/test/data/prerender/session_storage.html create mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html create mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html create mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html create mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-swap-after-activate.html create mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-utils.js create mode 100644 third_party/blink/web_tests/wpt_internal/prerender/session-storage.html diff --git a/content/browser/prerender/prerender_browsertest.cc b/content/browser/prerender/prerender_browsertest.cc index 4aa694bafd4548..20dba944dc50db 100644 --- a/content/browser/prerender/prerender_browsertest.cc +++ b/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 process_host_watcher = + std::make_unique( + 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(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& service = + StoragePartitionImpl::GetStorageServiceForTesting(); + base::RunLoop loop; + service.set_disconnect_handler(base::BindLambdaForTesting([&] { + loop.Quit(); + service.reset(); + })); + mojo::Remote 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() { diff --git a/content/browser/prerender/prerender_host.cc b/content/browser/prerender/prerender_host.cc index 41d8cbcea9fe23..49a819dcbe41fc 100644 --- a/content/browser/prerender/prerender_host.cc +++ b/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 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(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( diff --git a/content/browser/renderer_host/back_forward_cache_impl.cc b/content/browser/renderer_host/back_forward_cache_impl.cc index 830c9af5144057..d1f61c008197b1 100644 --- a/content/browser/renderer_host/back_forward_cache_impl.cc +++ b/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; diff --git a/content/browser/renderer_host/back_forward_cache_impl.h b/content/browser/renderer_host/back_forward_cache_impl.h index 7e0e77ef455231..6db686cfc70b71 100644 --- a/content/browser/renderer_host/back_forward_cache_impl.h +++ b/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 diff --git a/content/test/data/prerender/session_storage.html b/content/test/data/prerender/session_storage.html new file mode 100644 index 00000000000000..34efa12abf9af8 --- /dev/null +++ b/content/test/data/prerender/session_storage.html @@ -0,0 +1,40 @@ + + + + + + + + diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc index 4e98dd5efce0b7..a20093f0e380e7 100644 --- a/third_party/blink/renderer/core/dom/document.cc +++ b/third_party/blink/renderer/core/dom/document.cc @@ -8185,6 +8185,12 @@ void Document::ActivateForPrerendering() { if (DocumentLoader* loader = Loader()) loader->NotifyPrerenderingDocumentActivated(); + Vector callbacks; + callbacks.swap(will_dispatch_prerenderingchange_callbacks_); + for (auto& callback : callbacks) { + std::move(callback).Run(); + } + // https://jeremyroman.github.io/alternate-loading-modes/#prerendering-browsing-context-activate // Step 8.3.4 "Fire an event named prerenderingchange at doc." DispatchEvent(*Event::Create(event_type_names::kPrerenderingchange)); @@ -8197,6 +8203,12 @@ void Document::ActivateForPrerendering() { frame->DidActivateForPrerendering(); } +void Document::AddWillDispatchPrerenderingchangeCallback( + base::OnceClosure closure) { + DCHECK(is_prerendering_); + will_dispatch_prerenderingchange_callbacks_.push_back(std::move(closure)); +} + void Document::AddPostPrerenderingActivationStep(base::OnceClosure callback) { DCHECK(is_prerendering_); post_prerendering_activation_callbacks_.push_back(std::move(callback)); diff --git a/third_party/blink/renderer/core/dom/document.h b/third_party/blink/renderer/core/dom/document.h index 6eb556a8f15935..083cd83c87a269 100644 --- a/third_party/blink/renderer/core/dom/document.h +++ b/third_party/blink/renderer/core/dom/document.h @@ -1689,6 +1689,8 @@ class CORE_EXPORT Document : public ContainerNode, void ActivateForPrerendering(); + void AddWillDispatchPrerenderingchangeCallback(base::OnceClosure); + void AddPostPrerenderingActivationStep(base::OnceClosure callback); class CORE_EXPORT PaintPreviewScope { @@ -1863,6 +1865,10 @@ class CORE_EXPORT Document : public ContainerNode, // https://github.com/jeremyroman/alternate-loading-modes/blob/main/prerendering-state.md#documentprerendering bool is_prerendering_; + // Callbacks to execute upon activation of a prerendered page, just before the + // prerenderingchange event is dispatched. + Vector will_dispatch_prerenderingchange_callbacks_; + // The callback list for post-prerendering activation step. // https://jeremyroman.github.io/alternate-loading-modes/#document-post-prerendering-activation-steps-list Vector post_prerendering_activation_callbacks_; diff --git a/third_party/blink/renderer/modules/storage/cached_storage_area.cc b/third_party/blink/renderer/modules/storage/cached_storage_area.cc index cf9150051c07e9..4b98ae5eb00fa8 100644 --- a/third_party/blink/renderer/modules/storage/cached_storage_area.cc +++ b/third_party/blink/renderer/modules/storage/cached_storage_area.cc @@ -102,10 +102,13 @@ bool CachedStorageArea::SetItem(const String& key, KURL page_url = source->GetPageUrl(); String source_id = areas_->at(source); String source_string = PackSource(page_url, source_id); - remote_area_->Put(StringToUint8Vector(key, GetKeyFormat()), - StringToUint8Vector(value, value_format), - optional_old_value, source_string, - MakeSuccessCallback(source)); + + if (!is_session_storage_for_prerendering_) { + remote_area_->Put(StringToUint8Vector(key, GetKeyFormat()), + StringToUint8Vector(value, value_format), + optional_old_value, source_string, + MakeSuccessCallback(source)); + } if (!IsSessionStorage()) EnqueuePendingMutation(key, value, old_value, source_string); else if (old_value != value) @@ -127,9 +130,11 @@ void CachedStorageArea::RemoveItem(const String& key, Source* source) { KURL page_url = source->GetPageUrl(); String source_id = areas_->at(source); String source_string = PackSource(page_url, source_id); - remote_area_->Delete(StringToUint8Vector(key, GetKeyFormat()), - optional_old_value, source_string, - MakeSuccessCallback(source)); + if (!is_session_storage_for_prerendering_) { + remote_area_->Delete(StringToUint8Vector(key, GetKeyFormat()), + optional_old_value, source_string, + MakeSuccessCallback(source)); + } if (!IsSessionStorage()) EnqueuePendingMutation(key, String(), old_value, source_string); else @@ -164,8 +169,10 @@ void CachedStorageArea::Clear(Source* source) { KURL page_url = source->GetPageUrl(); String source_id = areas_->at(source); String source_string = PackSource(page_url, source_id); - remote_area_->DeleteAll(source_string, std::move(new_observer), - MakeSuccessCallback(source)); + if (!is_session_storage_for_prerendering_) { + remote_area_->DeleteAll(source_string, std::move(new_observer), + MakeSuccessCallback(source)); + } if (!IsSessionStorage()) EnqueuePendingMutation(String(), String(), String(), source_string); else if (!already_empty) @@ -182,10 +189,12 @@ CachedStorageArea::CachedStorageArea( AreaType type, scoped_refptr origin, scoped_refptr task_runner, - StorageNamespace* storage_namespace) + StorageNamespace* storage_namespace, + bool is_session_storage_for_prerendering) : type_(type), origin_(std::move(origin)), storage_namespace_(storage_namespace), + is_session_storage_for_prerendering_(is_session_storage_for_prerendering), task_runner_(std::move(task_runner)), areas_(MakeGarbageCollected, String>>()) { BindStorageArea(); @@ -218,6 +227,7 @@ void CachedStorageArea::BindStorageArea( void CachedStorageArea::ResetConnection( mojo::PendingRemote new_area) { + DCHECK(!is_session_storage_for_prerendering_); remote_area_.reset(); BindStorageArea(std::move(new_area)); diff --git a/third_party/blink/renderer/modules/storage/cached_storage_area.h b/third_party/blink/renderer/modules/storage/cached_storage_area.h index 9196428d1984ad..68caa52aab9632 100644 --- a/third_party/blink/renderer/modules/storage/cached_storage_area.h +++ b/third_party/blink/renderer/modules/storage/cached_storage_area.h @@ -63,7 +63,8 @@ class MODULES_EXPORT CachedStorageArea CachedStorageArea(AreaType type, scoped_refptr origin, scoped_refptr ipc_runner, - StorageNamespace* storage_namespace); + StorageNamespace* storage_namespace, + bool is_session_storage_for_prerendering); // These correspond to blink::Storage. unsigned GetLength(); @@ -97,6 +98,10 @@ class MODULES_EXPORT CachedStorageArea void ResetConnection( mojo::PendingRemote new_area = {}); + bool is_session_storage_for_prerendering() const { + return is_session_storage_for_prerendering_; + } + void SetRemoteAreaForTesting( mojo::PendingRemote area) { remote_area_.Bind(std::move(area)); @@ -177,6 +182,12 @@ class MODULES_EXPORT CachedStorageArea const AreaType type_; const scoped_refptr origin_; const WeakPersistent storage_namespace_; + // Session storage state for prerendering is initialized by cloning the + // primary session storage state. It is used locally by the prerendering + // context, and does not get propagated back to the primary state (i.e., via + // remote_area_). For more details: + // https://docs.google.com/document/d/1I5Hr8I20-C1GBr4tAXdm0U8a1RDUKHt4n7WcH4fxiSE/edit?usp=sharing + const bool is_session_storage_for_prerendering_; const scoped_refptr task_runner_; std::unique_ptr map_; diff --git a/third_party/blink/renderer/modules/storage/cached_storage_area_test.cc b/third_party/blink/renderer/modules/storage/cached_storage_area_test.cc index b43460747d0042..82997841f7457d 100644 --- a/third_party/blink/renderer/modules/storage/cached_storage_area_test.cc +++ b/third_party/blink/renderer/modules/storage/cached_storage_area_test.cc @@ -38,7 +38,7 @@ class CachedStorageAreaTest : public testing::Test { : CachedStorageArea::AreaType::kLocalStorage; cached_area_ = base::MakeRefCounted( area_type, kOrigin, scheduler::GetSingleThreadTaskRunnerForTesting(), - nullptr); + nullptr, /*is_session_storage_for_prerendering=*/false); cached_area_->SetRemoteAreaForTesting( mock_storage_area_.GetInterfaceRemote()); source_area_ = MakeGarbageCollected(kPageUrl); diff --git a/third_party/blink/renderer/modules/storage/dom_window_storage.cc b/third_party/blink/renderer/modules/storage/dom_window_storage.cc index 6723295660615e..9888455e90752e 100644 --- a/third_party/blink/renderer/modules/storage/dom_window_storage.cc +++ b/third_party/blink/renderer/modules/storage/dom_window_storage.cc @@ -90,10 +90,16 @@ StorageArea* DOMWindowStorage::sessionStorage( StorageNamespace::From(window->GetFrame()->GetPage()); if (!storage_namespace) return nullptr; - auto storage_area = - storage_namespace->GetCachedArea(window->GetSecurityOrigin()); + scoped_refptr cached_storage_area; + if (window->document()->IsPrerendering()) { + cached_storage_area = storage_namespace->CreateCachedAreaForPrerender( + window->GetSecurityOrigin()); + } else { + cached_storage_area = + storage_namespace->GetCachedArea(window->GetSecurityOrigin()); + } session_storage_ = - StorageArea::Create(window, std::move(storage_area), + StorageArea::Create(window, std::move(cached_storage_area), StorageArea::StorageType::kSessionStorage); if (!session_storage_->CanAccessStorage()) { diff --git a/third_party/blink/renderer/modules/storage/storage_area.cc b/third_party/blink/renderer/modules/storage/storage_area.cc index 32373c87d730c6..a40cd639f9584a 100644 --- a/third_party/blink/renderer/modules/storage/storage_area.cc +++ b/third_party/blink/renderer/modules/storage/storage_area.cc @@ -41,6 +41,7 @@ #include "third_party/blink/renderer/modules/storage/storage_namespace.h" #include "third_party/blink/renderer/platform/bindings/exception_state.h" #include "third_party/blink/renderer/platform/weborigin/security_origin.h" +#include "third_party/blink/renderer/platform/wtf/functional.h" #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" namespace blink { @@ -73,6 +74,11 @@ StorageArea::StorageArea(LocalDOMWindow* window, DCHECK(window); DCHECK(cached_area_); cached_area_->RegisterSource(this); + if (cached_area_->is_session_storage_for_prerendering()) { + DomWindow()->document()->AddWillDispatchPrerenderingchangeCallback( + WTF::Bind(&StorageArea::OnDocumentActivatedForPrerendering, + WrapWeakPersistent(this))); + } } unsigned StorageArea::length(ExceptionState& exception_state) const { @@ -246,4 +252,18 @@ blink::WebScopedVirtualTimePauser StorageArea::CreateWebScopedVirtualTimePauser( ->CreateWebScopedVirtualTimePauser(name, duration); } +void StorageArea::OnDocumentActivatedForPrerendering() { + StorageNamespace* storage_namespace = + StorageNamespace::From(DomWindow()->GetFrame()->GetPage()); + if (!storage_namespace) + return; + + // Swap out the session storage state used within prerendering, and replace it + // with the normal session storage state. For more details: + // https://docs.google.com/document/d/1I5Hr8I20-C1GBr4tAXdm0U8a1RDUKHt4n7WcH4fxiSE/edit?usp=sharing + cached_area_ = + storage_namespace->GetCachedArea(DomWindow()->GetSecurityOrigin()); + cached_area_->RegisterSource(this); +} + } // namespace blink diff --git a/third_party/blink/renderer/modules/storage/storage_area.h b/third_party/blink/renderer/modules/storage/storage_area.h index 941e2bdf63d4f5..698c270cf8736a 100644 --- a/third_party/blink/renderer/modules/storage/storage_area.h +++ b/third_party/blink/renderer/modules/storage/storage_area.h @@ -92,7 +92,10 @@ class StorageArea final : public ScriptWrappable, private: void RecordModificationInMetrics(); - const scoped_refptr cached_area_; + + void OnDocumentActivatedForPrerendering(); + + scoped_refptr cached_area_; StorageType storage_type_; const bool should_enqueue_events_; diff --git a/third_party/blink/renderer/modules/storage/storage_namespace.cc b/third_party/blink/renderer/modules/storage/storage_namespace.cc index b10a0f2d9edc73..3827a9afddbcd4 100644 --- a/third_party/blink/renderer/modules/storage/storage_namespace.cc +++ b/third_party/blink/renderer/modules/storage/storage_namespace.cc @@ -100,11 +100,23 @@ scoped_refptr StorageNamespace::GetCachedArea( result = base::MakeRefCounted( IsSessionStorage() ? CachedStorageArea::AreaType::kSessionStorage : CachedStorageArea::AreaType::kLocalStorage, - origin, controller_->TaskRunner(), this); + origin, controller_->TaskRunner(), this, + /*is_session_storage_for_prerendering=*/false); cached_areas_.insert(std::move(origin), result); return result; } +scoped_refptr StorageNamespace::CreateCachedAreaForPrerender( + const SecurityOrigin* origin_ptr) { + DCHECK((IsSessionStorage())); + scoped_refptr origin(origin_ptr); + return base::MakeRefCounted( + IsSessionStorage() ? CachedStorageArea::AreaType::kSessionStorage + : CachedStorageArea::AreaType::kLocalStorage, + origin, controller_->TaskRunner(), this, + /*is_session_storage_for_prerendering=*/true); +} + void StorageNamespace::CloneTo(const String& target) { DCHECK(IsSessionStorage()) << "Cannot clone a local storage namespace."; EnsureConnected(); diff --git a/third_party/blink/renderer/modules/storage/storage_namespace.h b/third_party/blink/renderer/modules/storage/storage_namespace.h index bb62188a8d2d76..ec3658217d6603 100644 --- a/third_party/blink/renderer/modules/storage/storage_namespace.h +++ b/third_party/blink/renderer/modules/storage/storage_namespace.h @@ -84,6 +84,9 @@ class MODULES_EXPORT StorageNamespace final scoped_refptr GetCachedArea(const SecurityOrigin* origin); + scoped_refptr CreateCachedAreaForPrerender( + const SecurityOrigin* origin); + // Only valid to call this if |this| and |target| are session storage // namespaces. void CloneTo(const String& target); diff --git a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html new file mode 100644 index 00000000000000..c3a6fe072145e3 --- /dev/null +++ b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html @@ -0,0 +1,20 @@ + + + + + + diff --git a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html new file mode 100644 index 00000000000000..4fcd5ac1f9f191 --- /dev/null +++ b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html @@ -0,0 +1,41 @@ + + + + + + diff --git a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html new file mode 100644 index 00000000000000..0348439363d0b3 --- /dev/null +++ b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html @@ -0,0 +1,35 @@ + + + + + + diff --git a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-swap-after-activate.html b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-swap-after-activate.html new file mode 100644 index 00000000000000..f81841b1a44f7b --- /dev/null +++ b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-swap-after-activate.html @@ -0,0 +1,76 @@ + + + + + + diff --git a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-utils.js b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-utils.js new file mode 100644 index 00000000000000..d7688607794438 --- /dev/null +++ b/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-utils.js @@ -0,0 +1,76 @@ +// Copyright 2021 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +function getSessionStorageKeys() { + let keys = []; + let txt = ''; + for (let i = 0; i < sessionStorage.length; ++i) { + keys.push(sessionStorage.key(i)); + } + keys.sort(); + keys.forEach((key) => { + if (txt.length) { + txt += ', '; + } + txt += key; + }); + return txt; +} + +function getNextMessage(channel) { + return new Promise(resolve => { + channel.addEventListener('message', e => { + resolve(e.data); + }, {once: true}); + }); +} + +// session_storage_test() is a utility function for running session storage +// related tests that open a initiator page using window.open(). +function session_storage_test(testPath) { + promise_test(async t => { + const testChannel = new BroadcastChannel('test-channel'); + t.add_cleanup(() => { + testChannel.close(); + }); + const gotMessage = getNextMessage(testChannel); + const url = 'resources/' + testPath; + window.open(url, '_blank', 'noopener'); + assert_equals(await gotMessage, 'Done'); + }, testPath); +} + +// RunSessionStorageTest() is a utility function for running session storage +// related tests that requires coordinated code execution on both the initiator +// page and the prerendering page. The passed |func| function will be called +// with the following arguments: +// - isPrerendering: Whether the |func| is called in the prerendering page. +// - url: The URL of the prerendering page. |func| should call +// startPrerendering(url) when |isPrerendering| is false to start the +// prerendering. +// - channel: A Broadcast Channel which can be used to coordinate the code +// execution on the initiator page and the prerendering page. +// - done: A function that should be called when the test completes +// successfully. +async function RunSessionStorageTest(func) { + const url = new URL(document.URL); + url.searchParams.set('prerendering', ''); + const params = new URLSearchParams(location.search); + // The main test page loads the initiator page, then the initiator page will + // prerender itself with the `prerendering` parameter. + const isPrerendering = params.has('prerendering'); + const prerenderChannel = new BroadcastChannel('prerender-channel'); + const testChannel = new BroadcastChannel('test-channel'); + window.addEventListener('unload', () => { + prerenderChannel.close(); + testChannel.close(); + }); + try { + await func(isPrerendering, url.toString(), prerenderChannel, () => { + testChannel.postMessage('Done'); + }) + } catch (e) { + testChannel.postMessage(e.toString()); + } +} diff --git a/third_party/blink/web_tests/wpt_internal/prerender/session-storage.html b/third_party/blink/web_tests/wpt_internal/prerender/session-storage.html new file mode 100644 index 00000000000000..8f6bfc109294d7 --- /dev/null +++ b/third_party/blink/web_tests/wpt_internal/prerender/session-storage.html @@ -0,0 +1,27 @@ + + +Same-origin prerendering can access sessionStorage + + + + + + +