Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(10)

Side by Side Diff: content/browser/site_instance_impl.cc

Issue 2921063003: Fix process reuse for dedicated processes when over process limit. (Closed)
Patch Set: Rebase Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « content/browser/site_instance_impl.h ('k') | content/browser/site_per_process_browsertest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/browser/site_instance_impl.h" 5 #include "content/browser/site_instance_impl.h"
6 6
7 #include "base/macros.h" 7 #include "base/macros.h"
8 #include "base/memory/ptr_util.h" 8 #include "base/memory/ptr_util.h"
9 #include "content/browser/browsing_instance.h" 9 #include "content/browser/browsing_instance.h"
10 #include "content/browser/child_process_security_policy_impl.h" 10 #include "content/browser/child_process_security_policy_impl.h"
(...skipping 106 matching lines...) Expand 10 before | Expand all | Expand 10 after
117 has_site_) { 117 has_site_) {
118 RenderProcessHostImpl::RegisterProcessHostForSite(browser_context, 118 RenderProcessHostImpl::RegisterProcessHostForSite(browser_context,
119 process_, site_); 119 process_, site_);
120 } 120 }
121 121
122 TRACE_EVENT2("navigation", "SiteInstanceImpl::GetProcess", 122 TRACE_EVENT2("navigation", "SiteInstanceImpl::GetProcess",
123 "site id", id_, "process id", process_->GetID()); 123 "site id", id_, "process id", process_->GetID());
124 GetContentClient()->browser()->SiteInstanceGotProcess(this); 124 GetContentClient()->browser()->SiteInstanceGotProcess(this);
125 125
126 if (has_site_) 126 if (has_site_)
127 LockToOrigin(); 127 LockToOriginIfNeeded();
128 } 128 }
129 DCHECK(process_); 129 DCHECK(process_);
130 130
131 return process_; 131 return process_;
132 } 132 }
133 133
134 void SiteInstanceImpl::SetSite(const GURL& url) { 134 void SiteInstanceImpl::SetSite(const GURL& url) {
135 TRACE_EVENT2("navigation", "SiteInstanceImpl::SetSite", 135 TRACE_EVENT2("navigation", "SiteInstanceImpl::SetSite",
136 "site id", id_, "url", url.possibly_invalid_spec()); 136 "site id", id_, "url", url.possibly_invalid_spec());
137 // A SiteInstance's site should not change. 137 // A SiteInstance's site should not change.
(...skipping 16 matching lines...) Expand all
154 browsing_instance_->RegisterSiteInstance(this); 154 browsing_instance_->RegisterSiteInstance(this);
155 155
156 // Update the process reuse policy based on the site. 156 // Update the process reuse policy based on the site.
157 bool should_use_process_per_site = 157 bool should_use_process_per_site =
158 RenderProcessHost::ShouldUseProcessPerSite(browser_context, site_); 158 RenderProcessHost::ShouldUseProcessPerSite(browser_context, site_);
159 if (should_use_process_per_site) { 159 if (should_use_process_per_site) {
160 process_reuse_policy_ = ProcessReusePolicy::PROCESS_PER_SITE; 160 process_reuse_policy_ = ProcessReusePolicy::PROCESS_PER_SITE;
161 } 161 }
162 162
163 if (process_) { 163 if (process_) {
164 LockToOrigin(); 164 LockToOriginIfNeeded();
165 165
166 // Ensure the process is registered for this site if necessary. 166 // Ensure the process is registered for this site if necessary.
167 if (should_use_process_per_site) { 167 if (should_use_process_per_site) {
168 RenderProcessHostImpl::RegisterProcessHostForSite( 168 RenderProcessHostImpl::RegisterProcessHostForSite(
169 browser_context, process_, site_); 169 browser_context, process_, site_);
170 } 170 }
171 } 171 }
172 } 172 }
173 173
174 const GURL& SiteInstanceImpl::GetSiteURL() const { 174 const GURL& SiteInstanceImpl::GetSiteURL() const {
(...skipping 29 matching lines...) Expand all
204 // We want to use such a process in the IsSuitableHost check, so we 204 // We want to use such a process in the IsSuitableHost check, so we
205 // may end up assigning process_ in the GetProcess() call below. 205 // may end up assigning process_ in the GetProcess() call below.
206 if (!HasProcess()) 206 if (!HasProcess())
207 return false; 207 return false;
208 208
209 // If the URL to navigate to can be associated with any site instance, 209 // If the URL to navigate to can be associated with any site instance,
210 // we want to keep it in the same process. 210 // we want to keep it in the same process.
211 if (IsRendererDebugURL(url)) 211 if (IsRendererDebugURL(url))
212 return false; 212 return false;
213 213
214 // Any process can host an about:blank URL. This check avoids a process
215 // transfer for browser-initiated navigations to about:blank in a dedicated
216 // process; without it, IsSuitableHost would consider this process unsuitable
217 // for about:blank when it compares origin locks. Renderer-initiated
218 // navigations will handle about:blank navigations elsewhere and leave them
219 // in the source SiteInstance, along with about:srcdoc and data:.
220 if (url == url::kAboutBlankURL)
221 return false;
222
214 // If the site URL is an extension (e.g., for hosted apps or WebUI) but the 223 // If the site URL is an extension (e.g., for hosted apps or WebUI) but the
215 // process is not (or vice versa), make sure we notice and fix it. 224 // process is not (or vice versa), make sure we notice and fix it.
216 GURL site_url = GetSiteForURL(browsing_instance_->browser_context(), url); 225 GURL site_url = GetSiteForURL(browsing_instance_->browser_context(), url);
217 return !RenderProcessHostImpl::IsSuitableHost( 226 return !RenderProcessHostImpl::IsSuitableHost(
218 GetProcess(), browsing_instance_->browser_context(), site_url); 227 GetProcess(), browsing_instance_->browser_context(), site_url);
219 } 228 }
220 229
221 scoped_refptr<SiteInstanceImpl> 230 scoped_refptr<SiteInstanceImpl>
222 SiteInstanceImpl::GetDefaultSubframeSiteInstance() { 231 SiteInstanceImpl::GetDefaultSubframeSiteInstance() {
223 return browsing_instance_->GetDefaultSubframeSiteInstance(); 232 return browsing_instance_->GetDefaultSubframeSiteInstance();
(...skipping 171 matching lines...) Expand 10 before | Expand all | Expand 10 after
395 // (blob and filesystem) work properly. 404 // (blob and filesystem) work properly.
396 if (GetContentClient()->IsSupplementarySiteIsolationModeEnabled() && 405 if (GetContentClient()->IsSupplementarySiteIsolationModeEnabled() &&
397 GetContentClient()->browser()->DoesSiteRequireDedicatedProcess( 406 GetContentClient()->browser()->DoesSiteRequireDedicatedProcess(
398 browser_context, site_url)) { 407 browser_context, site_url)) {
399 return true; 408 return true;
400 } 409 }
401 410
402 return false; 411 return false;
403 } 412 }
404 413
414 // static
415 bool SiteInstanceImpl::ShouldLockToOrigin(BrowserContext* browser_context,
416 GURL site_url) {
417 if (!DoesSiteRequireDedicatedProcess(browser_context, site_url))
418 return false;
419
420 // Guest processes cannot be locked to their site because guests always have
421 // a fixed SiteInstance. The site of GURLs a guest loads doesn't match that
422 // SiteInstance. So we skip locking the guest process to the site.
423 // TODO(ncarter): Remove this exclusion once we can make origin lock per
424 // RenderFrame routing id.
425 if (site_url.SchemeIs(content::kGuestScheme))
426 return false;
427
428 // TODO(creis, nick) https://crbug.com/510588 Chrome UI pages use the same
429 // site (chrome://chrome), so they can't be locked because the site being
430 // loaded doesn't match the SiteInstance.
431 if (site_url.SchemeIs(content::kChromeUIScheme))
432 return false;
433
434 // TODO(creis, nick): Until we can handle sites with effective URLs at the
435 // call sites of ChildProcessSecurityPolicy::CanAccessDataForOrigin, we
436 // must give the embedder a chance to exempt some sites to avoid process
437 // kills.
438 if (!GetContentClient()->browser()->ShouldLockToOrigin(browser_context,
439 site_url)) {
440 return false;
441 }
442
443 return true;
444 }
445
405 void SiteInstanceImpl::RenderProcessHostDestroyed(RenderProcessHost* host) { 446 void SiteInstanceImpl::RenderProcessHostDestroyed(RenderProcessHost* host) {
406 DCHECK_EQ(process_, host); 447 DCHECK_EQ(process_, host);
407 process_->RemoveObserver(this); 448 process_->RemoveObserver(this);
408 process_ = nullptr; 449 process_ = nullptr;
409 } 450 }
410 451
411 void SiteInstanceImpl::RenderProcessWillExit(RenderProcessHost* host) { 452 void SiteInstanceImpl::RenderProcessWillExit(RenderProcessHost* host) {
412 // TODO(nick): http://crbug.com/575400 - RenderProcessWillExit might not serve 453 // TODO(nick): http://crbug.com/575400 - RenderProcessWillExit might not serve
413 // any purpose here. 454 // any purpose here.
414 for (auto& observer : observers_) 455 for (auto& observer : observers_)
415 observer.RenderProcessGone(this); 456 observer.RenderProcessGone(this);
416 } 457 }
417 458
418 void SiteInstanceImpl::RenderProcessExited(RenderProcessHost* host, 459 void SiteInstanceImpl::RenderProcessExited(RenderProcessHost* host,
419 base::TerminationStatus status, 460 base::TerminationStatus status,
420 int exit_code) { 461 int exit_code) {
421 for (auto& observer : observers_) 462 for (auto& observer : observers_)
422 observer.RenderProcessGone(this); 463 observer.RenderProcessGone(this);
423 } 464 }
424 465
425 void SiteInstanceImpl::LockToOrigin() { 466 void SiteInstanceImpl::LockToOriginIfNeeded() {
467 DCHECK(HasSite());
468
469 // From now on, this process should be considered "tainted" for future
470 // process reuse decisions:
471 // (1) If |site_| required a dedicated process, this SiteInstance's process
472 // can only host URLs for the same site.
473 // (2) Even if |site_| does not require a dedicated process, this
474 // SiteInstance's process still cannot be reused to host other sites
475 // requiring dedicated sites in the future.
476 // We can get here either when we commit a URL into a SiteInstance that does
477 // not yet have a site, or when we create a process for a SiteInstance with a
478 // preassigned site.
479 process_->SetIsUsed();
480
426 // TODO(nick): When all sites are isolated, this operation provides strong 481 // TODO(nick): When all sites are isolated, this operation provides strong
427 // protection. If only some sites are isolated, we need additional logic to 482 // protection. If only some sites are isolated, we need additional logic to
428 // prevent the non-isolated sites from requesting resources for isolated 483 // prevent the non-isolated sites from requesting resources for isolated
429 // sites. https://crbug.com/509125 484 // sites. https://crbug.com/509125
430 if (RequiresDedicatedProcess()) { 485 if (ShouldLockToOrigin(GetBrowserContext(), site_)) {
431 // Guest processes cannot be locked to its site because guests always have
432 // a fixed SiteInstance. The site of GURLs a guest loads doesn't match that
433 // SiteInstance. So we skip locking the guest process to the site.
434 // TODO(ncarter): Remove this exclusion once we can make origin lock per
435 // RenderFrame routing id.
436 if (site_.SchemeIs(content::kGuestScheme))
437 return;
438
439 // TODO(creis, nick) https://crbug.com/510588 Chrome UI pages use the same
440 // site (chrome://chrome), so they can't be locked because the site being
441 // loaded doesn't match the SiteInstance.
442 if (site_.SchemeIs(content::kChromeUIScheme))
443 return;
444
445 // TODO(creis, nick): Until we can handle sites with effective URLs at the
446 // call sites of ChildProcessSecurityPolicy::CanAccessDataForOrigin, we
447 // must give the embedder a chance to exempt some sites to avoid process
448 // kills.
449 if (!GetContentClient()->browser()->ShouldLockToOrigin(
450 browsing_instance_->browser_context(), site_))
451 return;
452
453 ChildProcessSecurityPolicyImpl* policy = 486 ChildProcessSecurityPolicyImpl* policy =
454 ChildProcessSecurityPolicyImpl::GetInstance(); 487 ChildProcessSecurityPolicyImpl::GetInstance();
455 policy->LockToOrigin(process_->GetID(), site_); 488 policy->LockToOrigin(process_->GetID(), site_);
456 } 489 }
457 } 490 }
458 491
459 } // namespace content 492 } // namespace content
OLDNEW
« no previous file with comments | « content/browser/site_instance_impl.h ('k') | content/browser/site_per_process_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698