Support site isolation for specific origins rather than sites |
||||
Issue descriptionSome OOPIF modes that we're pursuing require us to support process isolation for specific origins rather than sites. For example, we would like to be able to specify that https://isolated.foo.com is an isolated origin which should be placed in a different process from https://www.foo.com or https://foo.com. When making process model decisions for such origins, our SiteInstance logic (such as GetSiteForURL) will need to use the full scheme+host+port tuple rather than just the eTLD+1. This bug will track progress for this work.
,
Apr 28 2017
,
May 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3b9ad10d35e7cb293f559d262bf417d7aa099f4d commit 3b9ad10d35e7cb293f559d262bf417d7aa099f4d Author: alexmos <alexmos@chromium.org> Date: Fri May 26 23:41:08 2017 Introduce support for origins that require process isolation. This CL changes logic in SiteInstance to support isolating specific origins based on the full scheme+host+port tuple rather than just scheme and eTLD+1. The global list of such origins is maintained by SiteInstanceImpl and currently not exposed outside of content. This CL also adds a command-line option to add isolated origins. Sample usage: --isolate-origins=https://isolated.foo.com,https://bar.com Design doc: https://goo.gl/99ynqK BUG= 713444 Review-Url: https://codereview.chromium.org/2831683002 Cr-Commit-Position: refs/heads/master@{#475191} [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/chrome/browser/ui/extensions/hosted_app_browsertest.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/browser_main_loop.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/child_process_security_policy_impl.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/child_process_security_policy_impl.h [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/child_process_security_policy_unittest.cc [add] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/isolated_origin_browsertest.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/site_instance_impl.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/site_instance_impl.h [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/browser/site_instance_impl_unittest.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/common/site_isolation_policy.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/common/site_isolation_policy.h [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/public/common/content_switches.cc [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/public/common/content_switches.h [modify] https://crrev.com/3b9ad10d35e7cb293f559d262bf417d7aa099f4d/content/test/BUILD.gn
,
Jun 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b7555fc8a5b8eddc838c28595199eb663e2587a commit 7b7555fc8a5b8eddc838c28595199eb663e2587a Author: alexmos <alexmos@chromium.org> Date: Thu Jun 01 22:58:32 2017 Consolidate isolated origin subframes into existing processes. Previously, isolated origins subframes from unrelated tabs would always get their own process. This results in too many tabs for scenarios like sign-in, where there's a sign-in subframe on every Google tab. To address this, set the process reuse policy to allow isolated origin subframes to reuse existing processes for that origin. BUG= 713444 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2920473005 Cr-Commit-Position: refs/heads/master@{#476467} [modify] https://crrev.com/7b7555fc8a5b8eddc838c28595199eb663e2587a/content/browser/frame_host/render_frame_host_manager.cc [modify] https://crrev.com/7b7555fc8a5b8eddc838c28595199eb663e2587a/content/browser/isolated_origin_browsertest.cc
,
Jun 12 2017
,
Jun 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/13fe1960e0819a5b5e642de1b5b845875601f769 commit 13fe1960e0819a5b5e642de1b5b845875601f769 Author: alexmos <alexmos@chromium.org> Date: Wed Jun 28 04:25:12 2017 Fix process reuse for dedicated processes when over process limit. Previously, process reuse was broken for --isolate-origins, where an isolated origin was allowed to reuse a process containing content from other non-isolated sites when over process limit. It was also overly restrictive for --site-per-process, where a site could never reuse an existing process when over process limit, even if the process was dedicated to the same site. This CL fixes these cases by modifying RenderProcessHostImpl::IsSuitableHost() to consult ChildProcessSecurityPolicy and check whether the process is locked to a compatible site. For --site-per-process, it removes the "return false" in ShouldTryToUseExistingProcessHost(), which allows using IsSuitableHost(). One tricky case was caused by lazily-initialied SiteInstances: when a new WebContents is created with a new SiteInstance that has no site, that SiteInstance's process should be allowed to host a site that requires a dedicated process until something commits in it and the SiteInstance sets its site. Looking purely at origin locks isn't sufficient here, as we can't tell apart the case where the process is brand new and hasn't yet hosted any content vs. process that has committed URLs that don't require dedicated processes -- both end up with empty origin locks. To track this state, a new flag is added to RenderProcessHost. Another tricky case is when a page navigates itself to about:blank. This ought to be allowed to stay in-process, even for dedicated processes. This required adding an extra check to HasWrongProcessForURL() so it allows about:blank to stay in-process before it consults IsSuitableHost(), which would otherwise prevent this. The changes in this CL also revealed a bug in RenderProcessHostImpl::GetProcessHostForSite, which passed in a full URL instead of site URL to IsSuitableHost. BUG= 513036 , 713444 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/2921063003 Cr-Commit-Position: refs/heads/master@{#482879} [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/child_process_security_policy_impl.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/child_process_security_policy_impl.h [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/isolated_origin_browsertest.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/renderer_host/render_process_host_impl.h [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/renderer_host/render_process_host_unittest.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/site_instance_impl.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/site_instance_impl.h [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/site_per_process_browsertest.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/browser/web_contents/web_contents_impl_unittest.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/public/browser/render_process_host.h [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/public/test/mock_render_process_host.cc [modify] https://crrev.com/13fe1960e0819a5b5e642de1b5b845875601f769/content/public/test/mock_render_process_host.h
,
Jul 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4bc26323b196f3b8957e85fc1eb48c08456fc84d commit 4bc26323b196f3b8957e85fc1eb48c08456fc84d Author: alexmos <alexmos@chromium.org> Date: Sat Jul 01 00:57:14 2017 Keep subdomains of an isolated origin in the isolated origin's SiteInstance. Previously, if bar.foo.com was an isolated origin, subdomains like subdomain.bar.foo.com would end up in a different SiteInstance (using "foo.com" for its site URL) than the isolated origin, which was confusing/undesirable. There was also confusion with subdomains if an etld+1 (e.g., isolated.com) was marked as an isolated origin: we would try to assign a different SiteInstance to foo.isolated.com than isolated.com, yet the site URL would still resolve to "isolated.com". This CL changes this behavior to keep subdomains in the isolated origin's SiteInstance. It also adds conflict resolution, which allows adding multiple isolated origins with a common domain (e.g., c.b.a.com and a.com), where the most specific isolated origin is used when picking the SiteInstance site URL for a particular URL (e.g., b.a.com would use a.com, but d.c.b.a.com would use c.b.a.com). For more discussion about this, see the isolated origin design doc: https://goo.gl/99ynqK BUG= 713444 Review-Url: https://codereview.chromium.org/2891443002 Cr-Commit-Position: refs/heads/master@{#483881} [modify] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/BUILD.gn [modify] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/child_process_security_policy_impl.cc [modify] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/child_process_security_policy_impl.h [modify] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/isolated_origin_browsertest.cc [add] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/isolated_origin_util.cc [add] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/isolated_origin_util.h [modify] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/site_instance_impl.cc [modify] https://crrev.com/4bc26323b196f3b8957e85fc1eb48c08456fc84d/content/browser/site_instance_impl_unittest.cc
,
Jul 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d252c196594fd41bf5bd4e78a63ed344640992c4 commit d252c196594fd41bf5bd4e78a63ed344640992c4 Author: Alex Moshchuk <alexmos@chromium.org> Date: Mon Jul 17 22:03:48 2017 Set up an experiment for requiring a dedicated process for sign-in. This CL introduces a new base::Feature to control whether the sign-in origin (https://accounts.google.com) requires a dedicated process, to be used for a Finch experiment. As part of this, during initialization content/ will call out to a new ContentBrowserClient method to get a whitelist of origins requiring a dedicated process. These origins will be process-isolated in all profiles for the lifetime of the browser. Bug: 713444 , 739418 Change-Id: I308cd1eb209394a9990c5eb5793a95a78d76485a Reviewed-on: https://chromium-review.googlesource.com/569519 Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Reviewed-by: Nick Carter <nick@chromium.org> Cr-Commit-Position: refs/heads/master@{#487263} [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/chrome/browser/chrome_navigation_browsertest.cc [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/content/browser/browser_main_loop.cc [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/content/public/browser/content_browser_client.cc [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/content/public/browser/content_browser_client.h [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/content/public/common/content_features.cc [modify] https://crrev.com/d252c196594fd41bf5bd4e78a63ed344640992c4/content/public/common/content_features.h
,
Aug 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cf5a45d77f299616e5bfd086348bd6b35386f05 commit 9cf5a45d77f299616e5bfd086348bd6b35386f05 Author: Alex Moshchuk <alexmos@chromium.org> Date: Mon Aug 14 18:50:57 2017 Add sanity checks for LockToOrigin. These checks ensure that: 1. When determining a process for a SiteInstance, we never return an existing process with a wrong StoragePartition. 2. We never attempt to lock a process to an origin if the process has already been locked to a different origin. 3. When locking a process to an origin, the process has not previously been used to host any other web sites. This is a followup to https://chromium-review.googlesource.com/c/602707 These checks revealed a few unit tests that were broken for --site-per-process: - tests like TabsApiUnitTest.QueryWithHostPermission ran in --single-process mode, but that wasn't propagating to the decision made in ShouldLockToOrigin, because it was set via SetRunRendererInProcess, while ShouldLockToOrigin checks for the command-line flag. This has been fixed by checking run_renderer_in_process() from ShouldLockToOrigin instead. - two tests in render_process_host_unittest.cc were directly committing cross-site navigations into the same RenderFrameHost, which is invalid with --site-per-process. The tests were fixed to utilize pending/speculative RFHs if available. Bug: 751916, 751920, 713444 Change-Id: I0e754e65731c7871adc872b8dce42253842480c6 Reviewed-on: https://chromium-review.googlesource.com/604481 Commit-Queue: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Cr-Commit-Position: refs/heads/master@{#494114} [modify] https://crrev.com/9cf5a45d77f299616e5bfd086348bd6b35386f05/content/browser/renderer_host/render_process_host_impl.cc [modify] https://crrev.com/9cf5a45d77f299616e5bfd086348bd6b35386f05/content/browser/renderer_host/render_process_host_unittest.cc [modify] https://crrev.com/9cf5a45d77f299616e5bfd086348bd6b35386f05/content/browser/site_instance_impl.cc [modify] https://crrev.com/9cf5a45d77f299616e5bfd086348bd6b35386f05/content/browser/site_instance_impl.h
,
Sep 12 2017
The core support for isolated origins is complete. I'll mark this as fixed, and we can file new bugs for any new issues. Note that the launch bug for using this for sign-in is issue 739418. |
||||
►
Sign in to add a comment |
||||
Comment 1 by ew...@chromium.org
, Apr 28 2017