Move existing kSitePerProcess checks to a policy-oracle object

content/public/browser/site_instance.h

Index: content/public/browser/site_instance.h
diff --git a/content/public/browser/site_instance.h b/content/public/browser/site_instance.h
index a828d29f297859c862345357747e6f0232dc7743..b13d0c5b70300dc8a67082b714809decc6ddf9f3 100644
--- a/content/public/browser/site_instance.h
+++ b/content/public/browser/site_instance.h
@@ -17,13 +17,15 @@ class RenderProcessHost;
 ///////////////////////////////////////////////////////////////////////////////
 // SiteInstance interface.
 //
-// A SiteInstance represents a group of web pages that may be able to
-// synchronously script each other, and thus must live in the same renderer
-// process.
+// A SiteInstance represents a group of web pages that must live in the same
+// renderer process.  Pages able to synchronously script each other will always
+// be placed in the same SiteInstance.  Pages unable to synchronously script
+// each other may also be placed in the same SiteInstance, as determined by the
+// process model.
 //
-// We identify this group using a combination of where the page comes from
-// (the site) and which tabs have references to each other (the instance).
-// Here, a "site" is similar to the page's origin, but it only includes the
+// A page's SiteInstance is determined by a combination of where the page comes
+// from (the site) and which tabs have references to each other (the instance).

[Charlie Reis, 2015/07/08 23:42:59]

s/tabs/frames/

[ncarter (slow), 2015/07/20 17:45:46]

Done.

+// Here, a "site" is similar to the page's origin, but includes only the
 // registered domain name and scheme, not the port or subdomains.  This accounts
 // for the fact that changes to document.domain allow similar origin pages with
 // different ports or subdomains to script each other.  An "instance" includes
@@ -31,29 +33,37 @@ class RenderProcessHost;
 // created (e.g., window.open or targeted links).  We represent instances using
 // the BrowsingInstance class.
 //
-// Process models:
+// Four process models are currently supported:
 //
-// In process-per-site-instance (the current default process model),
-// SiteInstances are created (1) when the user manually creates a new tab
-// (which also creates a new BrowsingInstance), and (2) when the user navigates
-// across site boundaries (which uses the same BrowsingInstance).  If the user
-// navigates within a site, the same SiteInstance is used.
-// (Caveat: we currently allow renderer-initiated cross-site navigations to
-// stay in the same SiteInstance, to preserve compatibility in cases like
-// cross-site iframes that open popups.)
+// PROCESS PER SITE INSTANCE (the current default), SiteInstances are created

[nasko, 2015/07/08 12:52:23]

Why all caps?

[Charlie Reis, 2015/07/08 23:42:59]

I'm ok with either caps or a numbered list.

Please use a colon rather than a comma after the close paren, though.

[ncarter (slow), 2015/07/20 17:45:46]

Done.

+// (1) when the user manually creates a new tab (which also creates a new
+// BrowsingInstance), and (2) when the user navigates across site boundaries
+// (which uses the same BrowsingInstance). If the user navigates within a site,
+// the same SiteInstance is used. Caveat: we currently allow renderer-initiated
+// cross-site navigations to stay in the same SiteInstance, to preserve
+// compatibility in cases like cross-site iframes that open popups.
 //
-// In --process-per-tab, SiteInstances are created when the user manually
-// creates a new tab, but not when navigating across site boundaries (unless
-// a process swap is required for security reasons, such as navigating from
-// a privileged WebUI page to a normal web page).  This corresponds to one
-// process per BrowsingInstance.
+// SITE PER PROCESS (currently experimental) is the most granular process model,

[Charlie Reis, 2015/07/08 23:42:59]

Colon after close paren.

[ncarter (slow), 2015/07/20 17:45:46]

Done.

+// and is made possible by our support for out-of-process iframes.  A subframe

[nasko, 2015/07/08 12:52:23]

nit: s/our support/supporting/

[ncarter (slow), 2015/07/20 17:45:46]

Not done.

+// will be given a different SiteInstance if its site differs from the
+// containing document.  Cross-site navigation of top-level frames or subframes

[nasko, 2015/07/08 12:52:23]

If the wording is "top-level frames or subframes" it seems to encompass the
entire set of possible frames. Can it be dropped?

[nasko, 2015/07/08 12:52:23]

nit: The usage of one or two spaces after a sentence end is inconsistent. I
don't have strong preference for which one should be used, but I do like to see
things being consistent.

[ncarter (slow), 2015/07/20 17:45:46]

Done.

[ncarter (slow), 2015/07/20 17:45:46]

I mentioned "top level frames or subframes" explictly to draw attention to the
fact that not just top-level navs were involved in this process model, as a
contrast to process per site instance.

+// will trigger a change of SiteInstances, even if the navigation is renderer
+// initiated.  In this model, each process can be dedicated to documents from
+// just one site, allowing the same origin policy to be enforced by the IPC

[nasko, 2015/07/08 12:52:23]

We have a sandbox and an IPC channel to communicate outside of it, so "IPC
sandbox" sounds a bit strange. 

[ncarter (slow), 2015/07/08 23:28:38]

What would you say instead? I was actually paraphrasing something you wrote in
email that was equally imprecise, but which I really liked the gist of: "Our
Site Isolation team has been hard at work building the infrastructure for
out-of-process iframes (OOPIFs) so that Chrome's sandbox can finally help
enforce the Same Origin Policy."

It's not clear to me whether the term 'sandbox' includes the abilities that are
granted to a child process via IPC messages, or just the OS level permissions
(other than the ability to do IPC communication) that are taken away from a
process by the OS-level sandbox.

[Charlie Reis, 2015/07/08 23:42:59]

I'd just say sandbox, or renderer sandbox if you want to clarify.  IPC is just
the mechanism code inside the sandbox uses to communicate.  (That's covered more
in http://www.chromium.org/developers/design-documents/sandbox.)

[ncarter (slow), 2015/07/20 17:45:46]

Done.

+// sandbox.
 //
-// In --process-per-site, we consolidate all SiteInstances for a given site into
-// the same process, throughout the entire browser context.  This ensures that
-// only one process will be used for each site.
+// PROCESS PER TAB: SiteInstances are created when the user manually creates a
+// new tab, but not when navigating across site boundaries. (unless a process
+// swap is required for security reasons, such as navigating from a privileged
+// WebUI page to a normal web page).  This corresponds to one process per
+// BrowsingInstance.
+//
+// PROCESS PER SITE: we consolidate all SiteInstances for a given site into the
+// same process, throughout the entire browser context. This ensures that only
+// one process will be used for each site.
 //
 // Each NavigationEntry for a WebContents points to the SiteInstance that
-// rendered it.  Each RenderViewHost also points to the SiteInstance that it is
+// rendered it.  Each RenderFrameHost also points to the SiteInstance that it is
 // associated with.  A SiteInstance keeps track of the number of these
 // references and deletes itself when the count goes to zero.  This means that
 // a SiteInstance is only live as long as it is accessible, either from new
@@ -110,6 +120,11 @@ class CONTENT_EXPORT SiteInstance : public base::RefCounted<SiteInstance> {
   // related SiteInstances in the same BrowsingInstance.
   virtual size_t GetRelatedActiveContentsCount() = 0;
 
+  // Returns true if this SiteInstance is for a site that requires a dedicated
+  // process. This only returns true under the "site per process" process model.
+  // TODO(nick): Does this name suck?
+  virtual bool RequiresDedicatedProcess() = 0;
+
   // Factory method to create a new SiteInstance.  This will create a new
   // new BrowsingInstance, so it should only be used when creating a new tab
   // from scratch (or similar circumstances).  Callers should ensure that