From a0159268472a7ae35a8573518aacad4e4f758b12 Mon Sep 17 00:00:00 2001 From: Nate Chapin Date: Tue, 18 May 2021 17:11:56 +0000 Subject: [PATCH] Revert "Reland "Use the same SessionStorageNamespace for prerendering"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit c226ef4e5aaa66edbf29db3239e91bd49bcac2d2. Reason for revert: PrerenderSingleProcessBrowserTest.SessionStorageAfterBackNavigation reliably failing on linux-chromeos-chrome since first run with this CL: https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/14428 Original change's description: > Reland "Use the same SessionStorageNamespace for prerendering" > > 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} Bug: 1197383 Change-Id: I8e506a142374fa10c3f9d475bf24d7ba78af93fc No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2903294 Reviewed-by: Gayane Petrosyan Commit-Queue: Gayane Petrosyan Auto-Submit: Nate Chapin Owners-Override: Nate Chapin Cr-Commit-Position: refs/heads/master@{#884008} --- .../prerender/prerender_browsertest.cc | 284 +----------------- 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, 28 insertions(+), 717 deletions(-) delete mode 100644 content/test/data/prerender/session_storage.html delete mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html delete mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html delete mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html delete mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-swap-after-activate.html delete mode 100644 third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-utils.js delete 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 55dd2516fd7da..b1ac022b14f8d 100644 --- a/content/browser/prerender/prerender_browsertest.cc +++ b/content/browser/prerender/prerender_browsertest.cc @@ -18,23 +18,18 @@ #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/common/content_navigation_policy.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" @@ -243,13 +238,12 @@ class PrerenderBrowserTest : public ContentBrowserTest { return prerender_helper_.get(); } - void SetUpCommandLine(base::CommandLine* command_line) override { + private: + void SetUpCommandLine(base::CommandLine* command_line) final { // 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}; @@ -1788,280 +1782,6 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, LazyLoading) { EXPECT_EQ(GetRequestCount(kImageUrl), 1); } -IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, - SessionStorageAfterBackNavigation_NoProcessReuse) { - // When BackForwardCache feature is enabled, this test doesn't work, because - // this test is checking the behavior of a new renderer process which is - // created for a back forward navigation from a prerendered page. - if (IsBackForwardCacheEnabled()) - return; - - 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 49a819dcbe41f..41d8cbcea9fe2 100644 --- a/content/browser/prerender/prerender_host.cc +++ b/content/browser/prerender/prerender_host.cc @@ -16,7 +16,6 @@ #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" @@ -78,20 +77,10 @@ class PrerenderHost::PageHolder : public FrameTree::Delegate, &web_contents, &web_contents, FrameTree::Type::kPrerender)) { - 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)); + frame_tree_->Init( + SiteInstance::Create(web_contents.GetBrowserContext()).get(), + /*renderer_initiated_creation=*/false, + /*main_frame_name=*/""); // 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 5c5d52dc7fde7..795a369b141f8 100644 --- a/content/browser/renderer_host/back_forward_cache_impl.cc +++ b/content/browser/renderer_host/back_forward_cache_impl.cc @@ -40,6 +40,11 @@ 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 0794d62497889..5339c834796ab 100644 --- a/content/browser/renderer_host/back_forward_cache_impl.h +++ b/content/browser/renderer_host/back_forward_cache_impl.h @@ -44,11 +44,6 @@ 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 deleted file mode 100644 index 34efa12abf9af..0000000000000 --- a/content/test/data/prerender/session_storage.html +++ /dev/null @@ -1,40 +0,0 @@ - - - - - - - - diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc index a20093f0e380e..4e98dd5efce0b 100644 --- a/third_party/blink/renderer/core/dom/document.cc +++ b/third_party/blink/renderer/core/dom/document.cc @@ -8185,12 +8185,6 @@ 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)); @@ -8203,12 +8197,6 @@ 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 083cd83c87a26..6eb556a8f1593 100644 --- a/third_party/blink/renderer/core/dom/document.h +++ b/third_party/blink/renderer/core/dom/document.h @@ -1689,8 +1689,6 @@ class CORE_EXPORT Document : public ContainerNode, void ActivateForPrerendering(); - void AddWillDispatchPrerenderingchangeCallback(base::OnceClosure); - void AddPostPrerenderingActivationStep(base::OnceClosure callback); class CORE_EXPORT PaintPreviewScope { @@ -1865,10 +1863,6 @@ 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 4b98ae5eb00fa..cf9150051c07e 100644 --- a/third_party/blink/renderer/modules/storage/cached_storage_area.cc +++ b/third_party/blink/renderer/modules/storage/cached_storage_area.cc @@ -102,13 +102,10 @@ 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); - - if (!is_session_storage_for_prerendering_) { - remote_area_->Put(StringToUint8Vector(key, GetKeyFormat()), - StringToUint8Vector(value, value_format), - optional_old_value, source_string, - MakeSuccessCallback(source)); - } + 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) @@ -130,11 +127,9 @@ 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); - if (!is_session_storage_for_prerendering_) { - remote_area_->Delete(StringToUint8Vector(key, GetKeyFormat()), - optional_old_value, source_string, - MakeSuccessCallback(source)); - } + remote_area_->Delete(StringToUint8Vector(key, GetKeyFormat()), + optional_old_value, source_string, + MakeSuccessCallback(source)); if (!IsSessionStorage()) EnqueuePendingMutation(key, String(), old_value, source_string); else @@ -169,10 +164,8 @@ void CachedStorageArea::Clear(Source* source) { KURL page_url = source->GetPageUrl(); String source_id = areas_->at(source); String source_string = PackSource(page_url, source_id); - if (!is_session_storage_for_prerendering_) { - remote_area_->DeleteAll(source_string, std::move(new_observer), - MakeSuccessCallback(source)); - } + remote_area_->DeleteAll(source_string, std::move(new_observer), + MakeSuccessCallback(source)); if (!IsSessionStorage()) EnqueuePendingMutation(String(), String(), String(), source_string); else if (!already_empty) @@ -189,12 +182,10 @@ CachedStorageArea::CachedStorageArea( AreaType type, scoped_refptr origin, scoped_refptr task_runner, - StorageNamespace* storage_namespace, - bool is_session_storage_for_prerendering) + StorageNamespace* storage_namespace) : 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(); @@ -227,7 +218,6 @@ 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 68caa52aab963..9196428d1984a 100644 --- a/third_party/blink/renderer/modules/storage/cached_storage_area.h +++ b/third_party/blink/renderer/modules/storage/cached_storage_area.h @@ -63,8 +63,7 @@ class MODULES_EXPORT CachedStorageArea CachedStorageArea(AreaType type, scoped_refptr origin, scoped_refptr ipc_runner, - StorageNamespace* storage_namespace, - bool is_session_storage_for_prerendering); + StorageNamespace* storage_namespace); // These correspond to blink::Storage. unsigned GetLength(); @@ -98,10 +97,6 @@ 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)); @@ -182,12 +177,6 @@ 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 82997841f7457..b43460747d004 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, /*is_session_storage_for_prerendering=*/false); + nullptr); 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 9888455e90752..6723295660615 100644 --- a/third_party/blink/renderer/modules/storage/dom_window_storage.cc +++ b/third_party/blink/renderer/modules/storage/dom_window_storage.cc @@ -90,16 +90,10 @@ StorageArea* DOMWindowStorage::sessionStorage( StorageNamespace::From(window->GetFrame()->GetPage()); if (!storage_namespace) return nullptr; - 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()); - } + auto storage_area = + storage_namespace->GetCachedArea(window->GetSecurityOrigin()); session_storage_ = - StorageArea::Create(window, std::move(cached_storage_area), + StorageArea::Create(window, std::move(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 a40cd639f9584..32373c87d730c 100644 --- a/third_party/blink/renderer/modules/storage/storage_area.cc +++ b/third_party/blink/renderer/modules/storage/storage_area.cc @@ -41,7 +41,6 @@ #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 { @@ -74,11 +73,6 @@ 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 { @@ -252,18 +246,4 @@ 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 698c270cf8736..941e2bdf63d4f 100644 --- a/third_party/blink/renderer/modules/storage/storage_area.h +++ b/third_party/blink/renderer/modules/storage/storage_area.h @@ -92,10 +92,7 @@ class StorageArea final : public ScriptWrappable, private: void RecordModificationInMetrics(); - - void OnDocumentActivatedForPrerendering(); - - scoped_refptr cached_area_; + const 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 3827a9afddbcd..b10a0f2d9edc7 100644 --- a/third_party/blink/renderer/modules/storage/storage_namespace.cc +++ b/third_party/blink/renderer/modules/storage/storage_namespace.cc @@ -100,23 +100,11 @@ scoped_refptr StorageNamespace::GetCachedArea( result = base::MakeRefCounted( IsSessionStorage() ? CachedStorageArea::AreaType::kSessionStorage : CachedStorageArea::AreaType::kLocalStorage, - origin, controller_->TaskRunner(), this, - /*is_session_storage_for_prerendering=*/false); + origin, controller_->TaskRunner(), this); 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 ec3658217d660..bb62188a8d2d7 100644 --- a/third_party/blink/renderer/modules/storage/storage_namespace.h +++ b/third_party/blink/renderer/modules/storage/storage_namespace.h @@ -84,9 +84,6 @@ 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 deleted file mode 100644 index c3a6fe072145e..0000000000000 --- a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-carry-over-to-prerender-page.html +++ /dev/null @@ -1,20 +0,0 @@ - - - - - - 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 deleted file mode 100644 index 4fcd5ac1f9f19..0000000000000 --- a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-isolated-while-prerendering.html +++ /dev/null @@ -1,41 +0,0 @@ - - - - - - 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 deleted file mode 100644 index 0348439363d0b..0000000000000 --- a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-no-leak-to-initiator-page.html +++ /dev/null @@ -1,35 +0,0 @@ - - - - - - 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 deleted file mode 100644 index f81841b1a44f7..0000000000000 --- a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-swap-after-activate.html +++ /dev/null @@ -1,76 +0,0 @@ - - - - - - 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 deleted file mode 100644 index d768860779443..0000000000000 --- a/third_party/blink/web_tests/wpt_internal/prerender/resources/session-storage-utils.js +++ /dev/null @@ -1,76 +0,0 @@ -// 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 deleted file mode 100644 index 8f6bfc109294d..0000000000000 --- a/third_party/blink/web_tests/wpt_internal/prerender/session-storage.html +++ /dev/null @@ -1,27 +0,0 @@ - - -Same-origin prerendering can access sessionStorage - - - - - - -