Move existing kSitePerProcess checks to a policy-oracle object

content/public/common/site_isolation_policy.h

Index: content/public/common/site_isolation_policy.h
diff --git a/content/public/common/site_isolation_policy.h b/content/public/common/site_isolation_policy.h
new file mode 100644
index 0000000000000000000000000000000000000000..f0357cd96af682a2dc9a92c90eb300ab857ca632
--- /dev/null
+++ b/content/public/common/site_isolation_policy.h
@@ -0,0 +1,79 @@
+// Copyright 2015 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_COMMON_SITE_ISOLATION_POLICY_H_
+#define CONTENT_PUBLIC_COMMON_SITE_ISOLATION_POLICY_H_
+
+#include "base/command_line.h"
+#include "content/common/content_export.h"
+#include "url/gurl.h"
+
+namespace content {
+
+// A centralized place for making policy decisions about out-of-process iframes,
+// site isolation, --site-per-process, and related features.
+//
+// This is currently static because all these modes are controlled by command-
+// line flags.
+class CONTENT_EXPORT SiteIsolationPolicy {

[Charlie Reis, 2015/07/13 22:13:15]

Let's say what threads these are allowed to be called on.

+ public:
+  // Returns true if the current process model might dictate the use of cross-

[Charlie Reis, 2015/07/13 22:13:15]

s/dictate/allow/

(Dictate sounds like they're always necessary.)

+  // process iframes. This should typically used to avoid executing codepaths

[Charlie Reis, 2015/07/13 22:13:15]

s/used/be consulted/

+  // that only matter for cross-process iframes, to protect the default
+  // behavior.
+  //
+  // Note: Since cross process frames will soon be possible by default (e.g. for
+  // <iframe src="http://..."> in an extension process), usage should be limited
+  // to temporary stop-gaps.

[Charlie Reis, 2015/07/13 22:13:15]

I agree with this and we can keep the comment, but it's interesting to note that
this is the second most common call (behind only AreAllSitesIsolatedForTesting).
 Do you get the sense that many of these will be easy to convert after this CL
goes in?

+  //
+  // Instead of calling this method, prefer to examine object state to see
+  // whether a particular frame happens to have a cross-process relationship
+  // with another, or to consult DoesSiteRequireDedicatedProcess() to see if a
+  // particular site merits protection.
+  static bool AreCrossProcessFramesPossible();
+
+  // Returns true if pages loaded from |site| ought to be handled only by a
+  // renderer process isolated from other sites. If --site-per-process
+  // is on the command line, this is true for all sites.
+  //
+  // Eventually, this function will be made to return true for only some schemes
+  // (e.g. extensions) or a whitelist of sites that we should protect for this
+  // user.
+  static bool DoesSiteRequireDedicatedProcess(const GURL& site);

[Charlie Reis, 2015/07/13 22:13:15]

Let's add a bit to the comment about what's expected as input here: is it only
ok to pass site URLs from SiteInstances, or is it ok to pass a full URL with
hostname and path?  Is it expected to be the effective URL (which would be
implied if it's only site URLs)?

This is important if it's possible to call this from the IO thread, etc, where
we can't yet easily determine the effective URL.

+
+  // Returns true if navigation and history code should maintain per-frame
+  // navigation entries. This is an in-progress feature related to site
+  // isolation, so the return value is currently tied to --site-per-process.
+  // TODO(creis, avi): Make this the default, and eliminate this.
+  static bool UseSubframeNavigationEntries();
+
+  // Returns true if <webview> should be implemented in terms of cross-process
+  // iframes. This is an in-progress feature related to site isolation, so the
+  // return value is currently tied to --site-per-process.
+  // TODO(lazyboy, nick): This should probably be a command line flag separate
+  // from full site isolation (--site-per-process).
+  static bool GuestsShouldUseCrossProcessFrames();

[Charlie Reis, 2015/07/13 22:13:15]

Maybe ShouldGuestsUseCrossProcessSubframes or UseCrossProcessFramesForGuests? 
Those seem more consistent with the others, which start with AreX, UseX, DoesX.

+
+  // Appends --site-per-process to the command line, enabling tests to exercise
+  // site isolation and cross-process iframes.
+  //
+  // TODO(nick): In some places this method is called from the top of a test
+  // body. That's not strictly safe (it's setting a command line after it
+  // already may have been read). We should try make that pattern safer, as it
+  // makes browser tests easier to write.
+  static void IsolateAllSitesForTesting(base::CommandLine* command_line);

[Charlie Reis, 2015/07/13 22:13:15]

nit: Let's list this last, since the rest just return bools and this one does an
actual state change.

+
+  // Returns true if all sites are isolated. Typically used to bail from a test
+  // that is incompatible with --site-per-process.
+  static bool AreAllSitesIsolatedForTesting();
+
+ private:
+  SiteIsolationPolicy();  // Not instantiable.
+
+  DISALLOW_COPY_AND_ASSIGN(SiteIsolationPolicy);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_COMMON_SITE_ISOLATION_POLICY_H_