Commit 7a5097d6 authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Chromium LUCI CQ

Prerender: Remove dependencies on RenderFrameHost from PrerenderHost(Registry)

This CL is a preparation for implementing the Omnibox trigger. This
removes dependencies on the initiator RenderFrameHost from
PrerenderHost(Registry) so that the Omnibox trigger that doesn't have a
concept of the initiator frame will be able to trigger prerendering.

Before this CL, PrerenderProcessor passed the identifier of the
initiator frame up to PrerenderHost in order to retrieve BrowserContext.
After this CL, StoragePartitionImpl passes BrowserContext when
instantiating PrerenderHostRegistry, so the registry and hosts no longer
need the initiator frame.

Bug: 1166085
Change-Id: I0da9a95427c10ef65be7f2539311d50c002d68db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639352
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845956}
parent ef624dfe
...@@ -14,10 +14,8 @@ namespace content { ...@@ -14,10 +14,8 @@ namespace content {
PrerenderHost::PrerenderHost( PrerenderHost::PrerenderHost(
blink::mojom::PrerenderAttributesPtr attributes, blink::mojom::PrerenderAttributesPtr attributes,
const GlobalFrameRoutingId& initiator_render_frame_host_id,
const url::Origin& initiator_origin) const url::Origin& initiator_origin)
: attributes_(std::move(attributes)), : attributes_(std::move(attributes)),
initiator_render_frame_host_id_(initiator_render_frame_host_id),
initiator_origin_(initiator_origin) { initiator_origin_(initiator_origin) {
DCHECK(base::FeatureList::IsEnabled(blink::features::kPrerender2)); DCHECK(base::FeatureList::IsEnabled(blink::features::kPrerender2));
} }
...@@ -29,20 +27,9 @@ PrerenderHost::~PrerenderHost() = default; ...@@ -29,20 +27,9 @@ PrerenderHost::~PrerenderHost() = default;
// TODO(https://crbug.com/1132746): Inspect diffs from the current // TODO(https://crbug.com/1132746): Inspect diffs from the current
// no-state-prefetch implementation. See PrerenderContents::StartPrerendering() // no-state-prefetch implementation. See PrerenderContents::StartPrerendering()
// for example. // for example.
void PrerenderHost::StartPrerendering() { void PrerenderHost::StartPrerendering(BrowserContext& browser_context) {
// The initiator render frame host may have already been closed. In that case,
// abort the prerendering request.
auto* initiator_render_frame_host =
RenderFrameHostImpl::FromID(initiator_render_frame_host_id_);
if (!initiator_render_frame_host)
return;
// TODO(https://crbug.com/1138711, https://crbug.com/1138723): Abort if the
// initiator frame is not the main frame (i.e., iframe or pop-up window).
// Create a new WebContents for prerendering. // Create a new WebContents for prerendering.
WebContents::CreateParams web_contents_params( WebContents::CreateParams web_contents_params(&browser_context);
initiator_render_frame_host->GetBrowserContext());
// TODO(https://crbug.com/1132746): Set up other fields of // TODO(https://crbug.com/1132746): Set up other fields of
// `web_contents_params` as well, and add tests for them. // `web_contents_params` as well, and add tests for them.
prerendered_contents_ = WebContents::Create(web_contents_params); prerendered_contents_ = WebContents::Create(web_contents_params);
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
namespace content { namespace content {
class BrowserContext;
class RenderFrameHostImpl; class RenderFrameHostImpl;
class WebContents; class WebContents;
...@@ -31,7 +32,6 @@ class WebContents; ...@@ -31,7 +32,6 @@ class WebContents;
class CONTENT_EXPORT PrerenderHost final : public WebContentsObserver { class CONTENT_EXPORT PrerenderHost final : public WebContentsObserver {
public: public:
PrerenderHost(blink::mojom::PrerenderAttributesPtr attributes, PrerenderHost(blink::mojom::PrerenderAttributesPtr attributes,
const GlobalFrameRoutingId& initiator_render_frame_host_id,
const url::Origin& initiator_origin); const url::Origin& initiator_origin);
~PrerenderHost() override; ~PrerenderHost() override;
...@@ -40,7 +40,7 @@ class CONTENT_EXPORT PrerenderHost final : public WebContentsObserver { ...@@ -40,7 +40,7 @@ class CONTENT_EXPORT PrerenderHost final : public WebContentsObserver {
PrerenderHost(PrerenderHost&&) = delete; PrerenderHost(PrerenderHost&&) = delete;
PrerenderHost& operator=(PrerenderHost&&) = delete; PrerenderHost& operator=(PrerenderHost&&) = delete;
void StartPrerendering(); void StartPrerendering(BrowserContext& browser_context);
// WebContentsObserver implementation: // WebContentsObserver implementation:
void DidFinishNavigation(NavigationHandle* navigation_handle) override; void DidFinishNavigation(NavigationHandle* navigation_handle) override;
...@@ -61,7 +61,6 @@ class CONTENT_EXPORT PrerenderHost final : public WebContentsObserver { ...@@ -61,7 +61,6 @@ class CONTENT_EXPORT PrerenderHost final : public WebContentsObserver {
private: private:
const blink::mojom::PrerenderAttributesPtr attributes_; const blink::mojom::PrerenderAttributesPtr attributes_;
const GlobalFrameRoutingId initiator_render_frame_host_id_;
const url::Origin initiator_origin_; const url::Origin initiator_origin_;
// WebContents for prerendering. // WebContents for prerendering.
......
...@@ -14,7 +14,8 @@ ...@@ -14,7 +14,8 @@
namespace content { namespace content {
PrerenderHostRegistry::PrerenderHostRegistry() { PrerenderHostRegistry::PrerenderHostRegistry(BrowserContext& browser_context)
: browser_context_(browser_context) {
DCHECK(base::FeatureList::IsEnabled(blink::features::kPrerender2)); DCHECK(base::FeatureList::IsEnabled(blink::features::kPrerender2));
} }
...@@ -22,7 +23,6 @@ PrerenderHostRegistry::~PrerenderHostRegistry() = default; ...@@ -22,7 +23,6 @@ PrerenderHostRegistry::~PrerenderHostRegistry() = default;
void PrerenderHostRegistry::CreateAndStartHost( void PrerenderHostRegistry::CreateAndStartHost(
blink::mojom::PrerenderAttributesPtr attributes, blink::mojom::PrerenderAttributesPtr attributes,
const GlobalFrameRoutingId& initiator_render_frame_host_id,
const url::Origin& initiator_origin) { const url::Origin& initiator_origin) {
DCHECK(attributes); DCHECK(attributes);
...@@ -31,9 +31,9 @@ void PrerenderHostRegistry::CreateAndStartHost( ...@@ -31,9 +31,9 @@ void PrerenderHostRegistry::CreateAndStartHost(
if (base::Contains(prerender_host_by_url_, prerendering_url)) if (base::Contains(prerender_host_by_url_, prerendering_url))
return; return;
auto prerender_host = std::make_unique<PrerenderHost>( auto prerender_host =
std::move(attributes), initiator_render_frame_host_id, initiator_origin); std::make_unique<PrerenderHost>(std::move(attributes), initiator_origin);
prerender_host->StartPrerendering(); prerender_host->StartPrerendering(browser_context_);
prerender_host_by_url_[prerendering_url] = std::move(prerender_host); prerender_host_by_url_[prerendering_url] = std::move(prerender_host);
} }
......
...@@ -8,13 +8,13 @@ ...@@ -8,13 +8,13 @@
#include <map> #include <map>
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "content/public/browser/global_routing_id.h"
#include "third_party/blink/public/mojom/prerender/prerender.mojom.h" #include "third_party/blink/public/mojom/prerender/prerender.mojom.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
namespace content { namespace content {
class BrowserContext;
class FrameTreeNode; class FrameTreeNode;
class PrerenderHost; class PrerenderHost;
...@@ -22,13 +22,9 @@ class PrerenderHost; ...@@ -22,13 +22,9 @@ class PrerenderHost;
// PrerenderHostRegistry manages running prerender hosts and provides the host // PrerenderHostRegistry manages running prerender hosts and provides the host
// to navigation code for activating prerendered contents. This is created and // to navigation code for activating prerendered contents. This is created and
// owned by StoragePartitionImpl. // owned by StoragePartitionImpl.
//
// TODO(https://crbug.com/1154501): Once the Prerender2 is migrated to the
// MPArch, it would be more natural to make PrerenderHostRegistry scoped to
// WebContents, that is, WebContents will own this.
class CONTENT_EXPORT PrerenderHostRegistry { class CONTENT_EXPORT PrerenderHostRegistry {
public: public:
PrerenderHostRegistry(); explicit PrerenderHostRegistry(BrowserContext& browser_context);
~PrerenderHostRegistry(); ~PrerenderHostRegistry();
PrerenderHostRegistry(const PrerenderHostRegistry&) = delete; PrerenderHostRegistry(const PrerenderHostRegistry&) = delete;
...@@ -37,10 +33,8 @@ class CONTENT_EXPORT PrerenderHostRegistry { ...@@ -37,10 +33,8 @@ class CONTENT_EXPORT PrerenderHostRegistry {
PrerenderHostRegistry& operator=(PrerenderHostRegistry&&) = delete; PrerenderHostRegistry& operator=(PrerenderHostRegistry&&) = delete;
// Creates and starts a host for `prerendering_url`. // Creates and starts a host for `prerendering_url`.
void CreateAndStartHost( void CreateAndStartHost(blink::mojom::PrerenderAttributesPtr attributes,
blink::mojom::PrerenderAttributesPtr attributes, const url::Origin& initiator_origin);
const GlobalFrameRoutingId& initiator_render_frame_host_id,
const url::Origin& initiator_origin);
// Destroys the host registered for `prerendering_url`. // Destroys the host registered for `prerendering_url`.
void AbandonHost(const GURL& prerendering_url); void AbandonHost(const GURL& prerendering_url);
...@@ -56,6 +50,10 @@ class CONTENT_EXPORT PrerenderHostRegistry { ...@@ -56,6 +50,10 @@ class CONTENT_EXPORT PrerenderHostRegistry {
PrerenderHost* FindHostByUrlForTesting(const GURL& prerendering_url); PrerenderHost* FindHostByUrlForTesting(const GURL& prerendering_url);
private: private:
// This outlives `this` because PrerenderHostRegistry is owned by
// StoragePartitionImpl, which in turn is owned by BrowserContext.
BrowserContext& browser_context_;
// TODO(https://crbug.com/1132746): Expire prerendered contents if they are // TODO(https://crbug.com/1132746): Expire prerendered contents if they are
// not used for a while. // not used for a while.
std::map<GURL, std::unique_ptr<PrerenderHost>> prerender_host_by_url_; std::map<GURL, std::unique_ptr<PrerenderHost>> prerender_host_by_url_;
......
...@@ -66,7 +66,6 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHost) { ...@@ -66,7 +66,6 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHost) {
PrerenderHostRegistry* registry = GetPrerenderHostRegistry(); PrerenderHostRegistry* registry = GetPrerenderHostRegistry();
registry->CreateAndStartHost(std::move(attributes), registry->CreateAndStartHost(std::move(attributes),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
PrerenderHost* prerender_host = PrerenderHost* prerender_host =
registry->FindHostByUrlForTesting(kPrerenderingUrl); registry->FindHostByUrlForTesting(kPrerenderingUrl);
...@@ -96,7 +95,6 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHostForSameURL) { ...@@ -96,7 +95,6 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHostForSameURL) {
PrerenderHostRegistry* registry = GetPrerenderHostRegistry(); PrerenderHostRegistry* registry = GetPrerenderHostRegistry();
registry->CreateAndStartHost(std::move(attributes1), registry->CreateAndStartHost(std::move(attributes1),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
PrerenderHost* prerender_host1 = PrerenderHost* prerender_host1 =
registry->FindHostByUrlForTesting(kPrerenderingUrl); registry->FindHostByUrlForTesting(kPrerenderingUrl);
...@@ -104,7 +102,6 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHostForSameURL) { ...@@ -104,7 +102,6 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHostForSameURL) {
// Start the prerender host for the same URL. This second host should be // Start the prerender host for the same URL. This second host should be
// ignored, and the first host should still be findable. // ignored, and the first host should still be findable.
registry->CreateAndStartHost(std::move(attributes2), registry->CreateAndStartHost(std::move(attributes2),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
EXPECT_EQ(registry->FindHostByUrlForTesting(kPrerenderingUrl), EXPECT_EQ(registry->FindHostByUrlForTesting(kPrerenderingUrl),
prerender_host1); prerender_host1);
...@@ -134,10 +131,8 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHostForDifferentURLs) { ...@@ -134,10 +131,8 @@ TEST_F(PrerenderHostRegistryTest, CreateAndStartHostForDifferentURLs) {
PrerenderHostRegistry* registry = GetPrerenderHostRegistry(); PrerenderHostRegistry* registry = GetPrerenderHostRegistry();
registry->CreateAndStartHost(std::move(attributes1), registry->CreateAndStartHost(std::move(attributes1),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
registry->CreateAndStartHost(std::move(attributes2), registry->CreateAndStartHost(std::move(attributes2),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
PrerenderHost* prerender_host1 = PrerenderHost* prerender_host1 =
registry->FindHostByUrlForTesting(kPrerenderingUrl1); registry->FindHostByUrlForTesting(kPrerenderingUrl1);
...@@ -175,7 +170,6 @@ TEST_F(PrerenderHostRegistryTest, FindHostToActivateBeforeReadyForActivation) { ...@@ -175,7 +170,6 @@ TEST_F(PrerenderHostRegistryTest, FindHostToActivateBeforeReadyForActivation) {
PrerenderHostRegistry* registry = GetPrerenderHostRegistry(); PrerenderHostRegistry* registry = GetPrerenderHostRegistry();
registry->CreateAndStartHost(std::move(attributes), registry->CreateAndStartHost(std::move(attributes),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
PrerenderHost* prerender_host = PrerenderHost* prerender_host =
registry->FindHostByUrlForTesting(kPrerenderingUrl); registry->FindHostByUrlForTesting(kPrerenderingUrl);
...@@ -200,7 +194,6 @@ TEST_F(PrerenderHostRegistryTest, AbandonHost) { ...@@ -200,7 +194,6 @@ TEST_F(PrerenderHostRegistryTest, AbandonHost) {
PrerenderHostRegistry* registry = GetPrerenderHostRegistry(); PrerenderHostRegistry* registry = GetPrerenderHostRegistry();
registry->CreateAndStartHost(std::move(attributes), registry->CreateAndStartHost(std::move(attributes),
render_frame_host->GetGlobalFrameRoutingId(),
render_frame_host->GetLastCommittedOrigin()); render_frame_host->GetLastCommittedOrigin());
EXPECT_NE(registry->FindHostByUrlForTesting(kPrerenderingUrl), nullptr); EXPECT_NE(registry->FindHostByUrlForTesting(kPrerenderingUrl), nullptr);
......
...@@ -48,6 +48,8 @@ class PrerenderHostTest : public RenderViewHostImplTestHarness { ...@@ -48,6 +48,8 @@ class PrerenderHostTest : public RenderViewHostImplTestHarness {
return web_contents; return web_contents;
} }
BrowserContext& browser_context() { return *browser_context_; }
private: private:
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
...@@ -65,11 +67,10 @@ TEST_F(PrerenderHostTest, PrerenderAndActivate) { ...@@ -65,11 +67,10 @@ TEST_F(PrerenderHostTest, PrerenderAndActivate) {
auto attributes = blink::mojom::PrerenderAttributes::New(); auto attributes = blink::mojom::PrerenderAttributes::New();
attributes->url = kPrerenderingUrl; attributes->url = kPrerenderingUrl;
auto prerender_host = std::make_unique<PrerenderHost>( auto prerender_host = std::make_unique<PrerenderHost>(
std::move(attributes), initiator_rfh->GetGlobalFrameRoutingId(), std::move(attributes), initiator_rfh->GetLastCommittedOrigin());
initiator_rfh->GetLastCommittedOrigin());
// Start the prerendering navigation. // Start the prerendering navigation.
prerender_host->StartPrerendering(); prerender_host->StartPrerendering(browser_context());
// Finish the prerendering navigation. Normally we could use // Finish the prerendering navigation. Normally we could use
// EmbeddedTestServer to provide a response, but this test uses // EmbeddedTestServer to provide a response, but this test uses
......
...@@ -57,10 +57,8 @@ void PrerenderProcessor::Start( ...@@ -57,10 +57,8 @@ void PrerenderProcessor::Start(
prerendering_url_ = attributes->url; prerendering_url_ = attributes->url;
GetPrerenderHostRegistry().CreateAndStartHost( GetPrerenderHostRegistry().CreateAndStartHost(std::move(attributes),
std::move(attributes), initiator_origin_);
initiator_render_frame_host_.GetGlobalFrameRoutingId(),
initiator_origin_);
} }
void PrerenderProcessor::Cancel() { void PrerenderProcessor::Cancel() {
......
...@@ -1334,8 +1334,10 @@ void StoragePartitionImpl::Initialize( ...@@ -1334,8 +1334,10 @@ void StoragePartitionImpl::Initialize(
font_access_manager_ = std::make_unique<FontAccessManagerImpl>(); font_access_manager_ = std::make_unique<FontAccessManagerImpl>();
if (base::FeatureList::IsEnabled(blink::features::kPrerender2)) if (base::FeatureList::IsEnabled(blink::features::kPrerender2)) {
prerender_host_registry_ = std::make_unique<PrerenderHostRegistry>(); prerender_host_registry_ =
std::make_unique<PrerenderHostRegistry>(*browser_context_);
}
} }
void StoragePartitionImpl::OnStorageServiceDisconnected() { void StoragePartitionImpl::OnStorageServiceDisconnected() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment