Commit 821323c7 authored by Robert Ogden's avatar Robert Ogden Committed by Commit Bot

Add Render Process for Site Instance Assignment to content API

This CL adds an enum, SiteInstanceProcessAssignment, which
describes all the ways a SiteInstance may be assigned a render
process. This is exposed in the content API to allow embedders to
record this usage.

See design doc in bug and/or chained CL for more info.

Bug: 1129505
Change-Id: I0654d52b4c0fef3830a877fb1c1476dea4ad9361
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2417100
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809620}
parent aded96ee
......@@ -4473,19 +4473,32 @@ RenderProcessHost* RenderProcessHostImpl::GetProcessHostForSiteInstance(
render_process_host = GetUnusedProcessHostForServiceWorker(site_instance);
}
if (render_process_host) {
site_instance->set_process_assignment(
SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS);
}
// See if the spare RenderProcessHost can be used.
auto& spare_process_manager = SpareRenderProcessHostManager::GetInstance();
bool spare_was_taken = false;
if (!render_process_host) {
render_process_host = spare_process_manager.MaybeTakeSpareRenderProcessHost(
browser_context, site_instance);
spare_was_taken = (render_process_host != nullptr);
if (render_process_host) {
site_instance->set_process_assignment(
SiteInstanceProcessAssignment::USED_SPARE_PROCESS);
spare_was_taken = true;
}
}
// If not (or if none found), see if we should reuse an existing process.
if (!render_process_host && ShouldTryToUseExistingProcessHost(
browser_context, site_info.site_url())) {
render_process_host = GetExistingProcessHost(site_instance);
if (render_process_host) {
site_instance->set_process_assignment(
SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS);
}
}
// If we found a process to reuse, sanity check that it is suitable for
......@@ -4511,6 +4524,9 @@ RenderProcessHost* RenderProcessHostImpl::GetProcessHostForSiteInstance(
// issues as TestBrowserContext initialization is done on the main thread.
render_process_host =
CreateRenderProcessHost(browser_context, site_instance);
site_instance->set_process_assignment(
SiteInstanceProcessAssignment::CREATED_NEW_PROCESS);
}
// It is important to call PrepareForFutureRequests *after* potentially
......
......@@ -81,7 +81,7 @@ TEST_F(RenderProcessHostUnitTest, RendererProcessLimit) {
// Verify that the renderer sharing will happen.
GURL test_url("http://foo.com");
EXPECT_TRUE(RenderProcessHostImpl::ShouldTryToUseExistingProcessHost(
browser_context(), test_url));
browser_context(), test_url));
}
#endif
......@@ -97,7 +97,7 @@ TEST_F(RenderProcessHostUnitTest, NoRendererProcessLimitOnAndroidOrChromeOS) {
// Verify that the renderer sharing still won't happen.
GURL test_url("http://foo.com");
EXPECT_FALSE(RenderProcessHostImpl::ShouldTryToUseExistingProcessHost(
browser_context(), test_url));
browser_context(), test_url));
}
#endif
......@@ -112,6 +112,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseCommittedSite) {
SiteInstanceImpl::CreateReusableInstanceForTesting(browser_context(),
kUrl1);
EXPECT_NE(main_test_rfh()->GetProcess(), site_instance->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
site_instance->GetLastProcessAssignmentOutcome());
// Have the main frame navigate to the first url. Getting a RenderProcessHost
// with the REUSE_PENDING_OR_COMMITTED_SITE policy should now return the
......@@ -120,6 +122,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseCommittedSite) {
site_instance = SiteInstanceImpl::CreateReusableInstanceForTesting(
browser_context(), kUrl1);
EXPECT_EQ(main_test_rfh()->GetProcess(), site_instance->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
site_instance->GetLastProcessAssignmentOutcome());
// Navigate away. Getting a RenderProcessHost with the
// REUSE_PENDING_OR_COMMITTED_SITE policy should again return a new process.
......@@ -127,6 +131,11 @@ TEST_F(RenderProcessHostUnitTest, ReuseCommittedSite) {
site_instance = SiteInstanceImpl::CreateReusableInstanceForTesting(
browser_context(), kUrl1);
EXPECT_NE(main_test_rfh()->GetProcess(), site_instance->GetProcess());
EXPECT_EQ(RenderProcessHostImpl::IsSpareProcessKeptAtAllTimes()
? SiteInstanceProcessAssignment::USED_SPARE_PROCESS
: SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
site_instance->GetLastProcessAssignmentOutcome());
// Now add a subframe that navigates to kUrl1. Getting a RenderProcessHost
// with the REUSE_PENDING_OR_COMMITTED_SITE policy for kUrl1 should now
// return the process of the subframe RFH.
......@@ -146,6 +155,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseCommittedSite) {
site_instance = SiteInstanceImpl::CreateReusableInstanceForTesting(
browser_context(), kUrl1);
EXPECT_EQ(subframe->GetProcess(), site_instance->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
site_instance->GetLastProcessAssignmentOutcome());
}
// Check that only new processes that haven't yet hosted any web content are
......@@ -185,6 +196,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseUnmatchedServiceWorkerProcess) {
scoped_refptr<SiteInstanceImpl> sw_site_instance1 =
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl);
RenderProcessHost* sw_host1 = sw_site_instance1->GetProcess();
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance1->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a service worker with DEFAULT reuse policy
// should not reuse the existing service worker's process. We create this
......@@ -193,12 +206,16 @@ TEST_F(RenderProcessHostUnitTest, ReuseUnmatchedServiceWorkerProcess) {
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl);
RenderProcessHost* sw_host2 = sw_site_instance2->GetProcess();
EXPECT_NE(sw_host1, sw_host2);
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance2->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation to the same site must reuse
// the newest unmatched service worker's process (i.e., sw_host2).
scoped_refptr<SiteInstanceImpl> site_instance1 =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_EQ(sw_host2, site_instance1->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
site_instance1->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation to the same site must reuse
// the newest unmatched service worker's process (i.e., sw_host1). sw_host2
......@@ -207,6 +224,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseUnmatchedServiceWorkerProcess) {
scoped_refptr<SiteInstanceImpl> site_instance2 =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_EQ(sw_host1, site_instance2->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
site_instance2->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation should return a new process
// because there is no unmatched service worker's process.
......@@ -214,6 +233,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseUnmatchedServiceWorkerProcess) {
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_NE(sw_host1, site_instance3->GetProcess());
EXPECT_NE(sw_host2, site_instance3->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
site_instance3->GetLastProcessAssignmentOutcome());
}
class UnsuitableHostContentBrowserClient : public ContentBrowserClient {
......@@ -238,6 +259,8 @@ TEST_F(RenderProcessHostUnitTest,
scoped_refptr<SiteInstanceImpl> sw_site_instance =
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl);
RenderProcessHost* sw_host = sw_site_instance->GetProcess();
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance->GetLastProcessAssignmentOutcome());
// Simulate a situation where |sw_host| won't be considered suitable for
// future navigations to |kUrl|. In https://crbug.com/782349, this happened
......@@ -258,6 +281,8 @@ TEST_F(RenderProcessHostUnitTest,
scoped_refptr<SiteInstanceImpl> site_instance =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_NE(sw_host, site_instance->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance->GetLastProcessAssignmentOutcome());
SetBrowserClientForTesting(regular_client);
}
......@@ -270,6 +295,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseServiceWorkerProcessForServiceWorker) {
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl,
/* can_reuse_process */ true);
RenderProcessHost* sw_host1 = sw_site_instance1->GetProcess();
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance1->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a service worker with DEFAULT reuse policy
// should not reuse the existing service worker's process. This is because
......@@ -280,6 +307,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseServiceWorkerProcessForServiceWorker) {
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl);
RenderProcessHost* sw_host2 = sw_site_instance2->GetProcess();
EXPECT_NE(sw_host1, sw_host2);
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance2->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a service worker of the same site with
// REUSE_PENDING_OR_COMMITTED_SITE reuse policy should reuse the newest
......@@ -289,6 +318,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseServiceWorkerProcessForServiceWorker) {
/* can_reuse_process */ true);
RenderProcessHost* sw_host3 = sw_site_instance3->GetProcess();
EXPECT_EQ(sw_host2, sw_host3);
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
sw_site_instance3->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a service worker of the same site with
// REUSE_PENDING_OR_COMMITTED_SITE reuse policy should reuse the newest
......@@ -300,12 +331,16 @@ TEST_F(RenderProcessHostUnitTest, ReuseServiceWorkerProcessForServiceWorker) {
/* can_reuse_process */ true);
RenderProcessHost* sw_host4 = sw_site_instance4->GetProcess();
EXPECT_EQ(sw_host2, sw_host4);
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
sw_site_instance4->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation to the same site must reuse
// the newest unmatched service worker's process (i.e., sw_host2).
scoped_refptr<SiteInstanceImpl> site_instance1 =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_EQ(sw_host2, site_instance1->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
site_instance1->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation to the same site must reuse
// the newest unmatched service worker's process (i.e., sw_host1). sw_host2
......@@ -314,6 +349,8 @@ TEST_F(RenderProcessHostUnitTest, ReuseServiceWorkerProcessForServiceWorker) {
scoped_refptr<SiteInstanceImpl> site_instance2 =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_EQ(sw_host1, site_instance2->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
site_instance2->GetLastProcessAssignmentOutcome());
}
TEST_F(RenderProcessHostUnitTest,
......@@ -326,6 +363,8 @@ TEST_F(RenderProcessHostUnitTest,
scoped_refptr<SiteInstanceImpl> sw_site_instance1 =
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl);
RenderProcessHost* sw_host1 = sw_site_instance1->GetProcess();
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance1->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a service worker of the same site with
// process-per-site flag should reuse the unmatched service worker's process.
......@@ -333,6 +372,8 @@ TEST_F(RenderProcessHostUnitTest,
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl);
RenderProcessHost* sw_host2 = sw_site_instance2->GetProcess();
EXPECT_EQ(sw_host1, sw_host2);
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
sw_site_instance2->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation to the same site with
// process-per-site flag should reuse the unmatched service worker's process.
......@@ -340,6 +381,8 @@ TEST_F(RenderProcessHostUnitTest,
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
RenderProcessHost* sw_host3 = sw_site_instance3->GetProcess();
EXPECT_EQ(sw_host1, sw_host3);
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
sw_site_instance3->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a navigation to the same site again with
// process-per-site flag should reuse the unmatched service worker's process.
......@@ -347,6 +390,8 @@ TEST_F(RenderProcessHostUnitTest,
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
RenderProcessHost* sw_host4 = sw_site_instance4->GetProcess();
EXPECT_EQ(sw_host1, sw_host4);
EXPECT_EQ(SiteInstanceProcessAssignment::REUSED_EXISTING_PROCESS,
sw_site_instance4->GetLastProcessAssignmentOutcome());
}
TEST_F(RenderProcessHostUnitTest, DoNotReuseOtherSiteServiceWorkerProcess) {
......@@ -357,12 +402,16 @@ TEST_F(RenderProcessHostUnitTest, DoNotReuseOtherSiteServiceWorkerProcess) {
scoped_refptr<SiteInstanceImpl> sw_site_instance1 =
SiteInstanceImpl::CreateForServiceWorker(browser_context(), kUrl1);
RenderProcessHost* sw_host1 = sw_site_instance1->GetProcess();
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance1->GetLastProcessAssignmentOutcome());
// Getting a RenderProcessHost for a service worker of a different site should
// return a new process because there is no reusable process.
scoped_refptr<SiteInstanceImpl> sw_site_instance2 =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl2);
EXPECT_NE(sw_host1, sw_site_instance2->GetProcess());
EXPECT_EQ(SiteInstanceProcessAssignment::CREATED_NEW_PROCESS,
sw_site_instance2->GetLastProcessAssignmentOutcome());
}
// Tests that RenderProcessHost will not consider reusing a process that has
......@@ -866,6 +915,18 @@ TEST_F(RenderProcessHostUnitTest, RendererLockedToSite) {
}
}
// Checks that SiteInstanceProcessAssignment::UNKNOWN is used as the zero-value
// when no renderer process has been assigned to the SiteInstance yet.
TEST_F(RenderProcessHostUnitTest, ProcessAssignmentDefault) {
const GURL kUrl("https://foo.com");
scoped_refptr<SiteInstanceImpl> site_instance =
SiteInstanceImpl::CreateForURL(browser_context(), kUrl);
EXPECT_EQ(SiteInstanceProcessAssignment::UNKNOWN,
site_instance->GetLastProcessAssignmentOutcome());
EXPECT_FALSE(site_instance->HasProcess());
}
class SpareRenderProcessHostUnitTest : public RenderViewHostImplTestHarness {
public:
SpareRenderProcessHostUnitTest() {}
......
......@@ -163,7 +163,8 @@ SiteInstanceImpl::SiteInstanceImpl(BrowsingInstance* browsing_instance)
has_site_(false),
process_reuse_policy_(ProcessReusePolicy::DEFAULT),
is_for_service_worker_(false),
is_guest_(false) {
is_guest_(false),
process_assignment_(SiteInstanceProcessAssignment::UNKNOWN) {
DCHECK(browsing_instance);
}
......@@ -498,8 +499,8 @@ void SiteInstanceImpl::PreventAssociationWithSpareProcess() {
void SiteInstanceImpl::SetSite(const GURL& url) {
// TODO(creis): Consider calling ShouldAssignSiteForURL internally, rather
// than before multiple call sites. See https://crbug.com/949220.
TRACE_EVENT2("navigation", "SiteInstanceImpl::SetSite",
"site id", id_, "url", url.possibly_invalid_spec());
TRACE_EVENT2("navigation", "SiteInstanceImpl::SetSite", "site id", id_, "url",
url.possibly_invalid_spec());
// A SiteInstance's site should not change.
// TODO(creis): When following links or script navigations, we can currently
// render pages from other sites in this SiteInstance. This will eventually
......@@ -580,6 +581,11 @@ void SiteInstanceImpl::ConvertToDefaultOrSetSite(const GURL& url) {
SetSite(url);
}
SiteInstanceProcessAssignment
SiteInstanceImpl::GetLastProcessAssignmentOutcome() {
return process_assignment_;
}
const GURL& SiteInstanceImpl::GetSiteURL() {
return site_info_.site_url();
}
......@@ -607,8 +613,9 @@ scoped_refptr<SiteInstance> SiteInstanceImpl::GetRelatedSiteInstance(
}
bool SiteInstanceImpl::IsRelatedSiteInstance(const SiteInstance* instance) {
return browsing_instance_.get() == static_cast<const SiteInstanceImpl*>(
instance)->browsing_instance_.get();
return browsing_instance_.get() ==
static_cast<const SiteInstanceImpl*>(instance)
->browsing_instance_.get();
}
size_t SiteInstanceImpl::GetRelatedActiveContentsCount() {
......
......@@ -236,6 +236,13 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
bool RequiresDedicatedProcess() override;
bool IsSameSiteWithURL(const GURL& url) override;
bool IsGuest() override;
SiteInstanceProcessAssignment GetLastProcessAssignmentOutcome() override;
// This is called every time a renderer process is assigned to a SiteInstance
// and is used by the content embedder for collecting metrics.
void set_process_assignment(SiteInstanceProcessAssignment assignment) {
process_assignment_ = assignment;
}
// The policy to apply when selecting a RenderProcessHost for the
// SiteInstance. If no suitable RenderProcessHost for the SiteInstance exists
......@@ -451,8 +458,7 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
// hosts.
// Only public so that we can make a consistent process swap decision in
// RenderFrameHostManager.
static GURL GetEffectiveURL(BrowserContext* browser_context,
const GURL& url);
static GURL GetEffectiveURL(BrowserContext* browser_context, const GURL& url);
// Returns true if pages loaded from |site_info| ought to be handled only by a
// renderer process isolated from other sites. If --site-per-process is used,
......@@ -687,6 +693,9 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance,
// TODO(734722): Move this into the SecurityPrincipal once it is available.
bool is_guest_;
// How |this| was last assigned to a renderer process.
SiteInstanceProcessAssignment process_assignment_;
base::ObserverList<Observer, true>::Unchecked observers_;
DISALLOW_COPY_AND_ASSIGN(SiteInstanceImpl);
......
......@@ -341,6 +341,7 @@ source_set("browser_sources") {
"shared_worker_instance.h",
"shared_worker_service.h",
"site_instance.h",
"site_instance_process_assignment.h",
"site_isolation_policy.cc",
"site_isolation_policy.h",
"sms_fetcher.h",
......
......@@ -10,6 +10,7 @@
#include "base/memory/ref_counted.h"
#include "content/common/content_export.h"
#include "content/public/browser/site_instance_process_assignment.h"
#include "url/gurl.h"
namespace content {
......@@ -169,6 +170,12 @@ class CONTENT_EXPORT SiteInstance : public base::RefCounted<SiteInstance> {
// Returns true if this object is used for a <webview> guest.
virtual bool IsGuest() = 0;
// Returns how this SiteInstance was assigned to a renderer process the most
// recent time that such an assignment was done. This allows the content
// embedder to collect metrics on how renderer process starting or reuse
// affects performance.
virtual SiteInstanceProcessAssignment GetLastProcessAssignmentOutcome() = 0;
// Factory method to create a new SiteInstance. This will create a new
// BrowsingInstance, so it should only be used when creating a new tab from
// scratch (or similar circumstances).
......
// Copyright (c) 2020 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.
#ifndef CONTENT_PUBLIC_BROWSER_SITE_INSTANCE_PROCESS_ASSIGNMENT_H_
#define CONTENT_PUBLIC_BROWSER_SITE_INSTANCE_PROCESS_ASSIGNMENT_H_
namespace content {
// This enum describes how a renderer process is assigned to a SiteInstance.
enum class SiteInstanceProcessAssignment {
// No renderer process has been assigned to the SiteInstance yet.
UNKNOWN,
// Reused some pre-existing process.
REUSED_EXISTING_PROCESS,
// Used an existing spare process.
USED_SPARE_PROCESS,
// No renderer could be reused, so a new one was created for the SiteInstance.
CREATED_NEW_PROCESS,
};
} // namespace content
#endif // CONTENT_PUBLIC_BROWSER_SITE_INSTANCE_PROCESS_ASSIGNMENT_H_
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