Skip to content

Commit

Permalink
Use the same SessionStorageNamespace for prerendering
Browse files Browse the repository at this point in the history
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 <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#881985}
  • Loading branch information
horo-t authored and Chromium LUCI CQ committed May 12, 2021
1 parent 26d8d5b commit eefb8c5
Show file tree
Hide file tree
Showing 19 changed files with 743 additions and 23 deletions.
288 changes: 286 additions & 2 deletions content/browser/prerender/prerender_browsertest.cc
Expand Up @@ -19,6 +19,8 @@
#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"
Expand All @@ -27,6 +29,7 @@
#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"
Expand Down Expand Up @@ -220,12 +223,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};

Expand Down Expand Up @@ -1736,6 +1740,286 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, LazyLoading) {
EXPECT_EQ(GetRequestCount(kImageUrl), 1);
}

class RenderProcessHostDestroyedWaiter : public RenderProcessHostObserver {
public:
explicit RenderProcessHostDestroyedWaiter(RenderProcessHost* host) {
observation_.Observe(host);
quit_closure_ = run_loop_.QuitClosure();
}

~RenderProcessHostDestroyedWaiter() override = default;

void Wait() { run_loop_.Run(); }

// RenderProcessHostObserver implements:
void RenderProcessHostDestroyed(RenderProcessHost* host) override {
observation_.Reset();
std::move(quit_closure_).Run();
}

private:
base::ScopedObservation<RenderProcessHost, RenderProcessHostObserver>
observation_{this};
base::RunLoop run_loop_;
base::OnceClosure quit_closure_;
};

IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
SessionStorageAfterBackNavigation_NoProcessReuse) {
const GURL kInitialUrl = GetUrl("/prerender/session_storage.html");

// Navigate to an initial page.
ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));

std::unique_ptr<RenderProcessHostDestroyedWaiter>
process_host_destroyed_waiter =
std::make_unique<RenderProcessHostDestroyedWaiter>(
current_frame_host()->GetProcess());

EXPECT_TRUE(ExecJs(current_frame_host(), "startPrerender();"));

content::TestNavigationObserver observer1(shell()->web_contents());
EXPECT_TRUE(ExecJs(current_frame_host(), "activatePrerenderedPage();"));
observer1.Wait();
EXPECT_EQ("initial",
EvalJs(current_frame_host(), "window.sessionKeysInPrerenderchange")
.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_destroyed_waiter->Wait();

// Navigate back to the initial page.
content::TestNavigationObserver observer2(shell()->web_contents());
shell()->GoBackOrForward(-1);
observer2.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");

// 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();

EXPECT_TRUE(ExecJs(current_frame_host(), "startPrerender();"));

content::TestNavigationObserver observer1(shell()->web_contents());
EXPECT_TRUE(ExecJs(current_frame_host(), "activatePrerenderedPage();"));
observer1.Wait();
EXPECT_EQ("initial",
EvalJs(current_frame_host(), "window.sessionKeysInPrerenderchange")
.ExtractString());
EXPECT_EQ(
"activated, initial",
EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());

// Navigate back to the initial page.
content::TestNavigationObserver observer2(shell()->web_contents());
shell()->GoBackOrForward(-1);
observer2.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");

// Navigate to an initial page.
ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
EXPECT_TRUE(ExecJs(current_frame_host(), "startPrerender();"));

content::TestNavigationObserver observer1(shell()->web_contents());
EXPECT_TRUE(ExecJs(current_frame_host(), "activatePrerenderedPage();"));
observer1.Wait();
EXPECT_EQ("initial",
EvalJs(current_frame_host(), "window.sessionKeysInPrerenderchange")
.ExtractString());
EXPECT_EQ(
"activated, initial",
EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());

// Navigate back to the initial page.
content::TestNavigationObserver observer2(shell()->web_contents());
shell()->GoBackOrForward(-1);
observer2.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,
// Set a very long TTL before expiration (longer than the test
// timeout) so tests that are expecting deletion don't pass when
// they shouldn't.
{{"TimeToLiveInBackForwardCacheInSeconds", "3600"}}}},
// 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");

// Navigate to an initial page.
ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));
EXPECT_TRUE(ExecJs(current_frame_host(), "startPrerender();"));

content::TestNavigationObserver observer1(shell()->web_contents());
EXPECT_TRUE(ExecJs(current_frame_host(), "activatePrerenderedPage();"));
observer1.Wait();
EXPECT_EQ("initial",
EvalJs(current_frame_host(), "window.sessionKeysInPrerenderchange")
.ExtractString());
EXPECT_EQ(
"activated, initial",
EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());

#if DCHECK_IS_ON()
// There is a DCHECK failure issue while navigating back from a prerendered
// page (crbug.com/1201914). To avoid this issue, returns here.
// TODO(crbug.com/1201914): Remove this when the DCHECK issue is fixed.
return;
#else // DCHECK_IS_ON()
// Navigate back to the initial page.
content::TestNavigationObserver observer2(shell()->web_contents());
shell()->GoBackOrForward(-1);
observer2.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());
#endif // DCHECK_IS_ON()
}

#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");

// Navigate to an initial page.
ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));

CrashStorageServiceAndWaitForRestart();

EXPECT_TRUE(ExecJs(current_frame_host(), "startPrerender();"));

content::TestNavigationObserver observer(shell()->web_contents());
EXPECT_TRUE(ExecJs(current_frame_host(), "activatePrerenderedPage();"));
observer.Wait();
EXPECT_EQ("initial",
EvalJs(current_frame_host(), "window.sessionKeysInPrerenderchange")
.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");

// Navigate to an initial page.
ASSERT_TRUE(NavigateToURL(shell(), kInitialUrl));

EXPECT_TRUE(ExecJs(current_frame_host(), "startPrerender();"));
CrashStorageServiceAndWaitForRestart();

content::TestNavigationObserver observer(shell()->web_contents());
EXPECT_TRUE(ExecJs(current_frame_host(), "activatePrerenderedPage();"));
observer.Wait();
EXPECT_EQ("initial",
EvalJs(current_frame_host(), "window.sessionKeysInPrerenderchange")
.ExtractString());
EXPECT_EQ(
"activated, initial",
EvalJs(current_frame_host(), "getSessionStorageKeys()").ExtractString());
}
#endif

class PrerenderWithProactiveBrowsingInstanceSwap : public PrerenderBrowserTest {
public:
PrerenderWithProactiveBrowsingInstanceSwap() {
Expand Down
19 changes: 15 additions & 4 deletions content/browser/prerender/prerender_host.cc
Expand Up @@ -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"
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit eefb8c5

Please sign in to comment.