Move SiteIsolationPolicy::ShouldPdfCompositorBeEnabledForOopifs from //content to //components

Currently, there are two SiteIsolationPolicy objects: one in //content
which knows about content-internal things, and one in //components,
which knows about higher-layer modes such as password-triggered site
isolation on Android.  The ShouldPdfCompositorBeEnabledForOopifs() in
content was a bit problematic for two reasons: (1) it's a bit of a
layering violation for content to know about PDF compositor, (2) it
implicitly relied on AreIsolatedOriginsEnabled() being true for the
password-triggered site isolation mode used on Android, which stopped
being true in https://crbug.com/1005895.

This CL moves ShouldPdfCompositorBeEnabledForOopifs() to the
SiteIsolationPolicy in //components, where it's also updated to check
IsIsolationForPasswordSitesEnabled() in addition to the other two site
isolation modes.

Note that this still should exclude (1) low-memory Android devices
that don't use any site isolation (enforced via
IsSiteIsolationDisabled() checks inside SiteIsolationPolicy), and (2)
Android Webview, where OOPIFs aren't supported (hopefully this is true
because AW doesn't go through code paths that check
ShouldPdfCompositorBeEnabledForOopifs()).  Eventually, the goal is to
enable PDF compositor everywhere, so hopefully we can remove
ShouldPdfCompositorBeEnabledForOopifs() altogether at some point.

Bug: 1022917
Change-Id: Ib8d3a7c7b133707d5c6ca88efbbdc0f77635fe1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2500444Reviewed-by: Wei Li <weili@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822049}

components/printing/browser/BUILD.gn

@@ -56,6 +56,7 @@ static_library("browser") {
     "//components/printing/common:mojo_interfaces",
     "//components/services/print_compositor/public/cpp",
     "//components/services/print_compositor/public/mojom",
+    "//components/site_isolation",
     "//components/strings:components_strings_grit",
     "//printing",
     "//printing/common:common",

components/printing/browser/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   "+components/crash/core/common",
   "+components/discardable_memory/service",
   "+components/services/print_compositor/public",
+  "+components/site_isolation",
   "+components/strings/grit",
   "+content/public/browser",
   "+mojo/public",

components/printing/browser/print_manager_utils.cc

@@ -7,7 +7,7 @@
 #include "components/printing/browser/print_composite_client.h"
 #include "components/printing/common/print.mojom.h"
 #include "components/printing/common/print_messages.h"
-#include "content/public/browser/site_isolation_policy.h"
+#include "components/site_isolation/site_isolation_policy.h"
 #include "printing/mojom/print.mojom.h"
 #include "printing/print_settings.h"
 
@@ -37,7 +37,8 @@ void CreateCompositeClientIfNeeded(content::WebContents* web_contents,
   // where OOPIF is used such as isolate-extensions, but should be good for
   // feature testing purpose. Eventually, we will remove this check and use pdf
   // compositor service by default for printing.
-  if (content::SiteIsolationPolicy::ShouldPdfCompositorBeEnabledForOopifs()) {
+  if (site_isolation::SiteIsolationPolicy::
+          ShouldPdfCompositorBeEnabledForOopifs()) {
     PrintCompositeClient::CreateForWebContents(web_contents);
     PrintCompositeClient::FromWebContents(web_contents)
         ->SetUserAgent(user_agent);

components/site_isolation/site_isolation_policy.cc

@@ -130,4 +130,19 @@ void SiteIsolationPolicy::ApplyPersistedIsolatedOrigins(
       "SiteIsolation.SavedUserTriggeredIsolatedOrigins.Size", origins.size());
 }
 
+// static
+bool SiteIsolationPolicy::ShouldPdfCompositorBeEnabledForOopifs() {
+  // We only create pdf compositor client and use pdf compositor service when
+  // one of the site isolation modes that forces OOPIFs is on. This includes
+  // full site isolation on desktop, password-triggered site isolation on
+  // Android for high-memory devices, and/or isolated origins specified via
+  // command line, enterprise policy, or field trials.
+  //
+  // TODO(weili, thestig): Eventually, we should remove this check and use pdf
+  // compositor service by default for printing.
+  return content::SiteIsolationPolicy::UseDedicatedProcessesForAllSites() ||
+         IsIsolationForPasswordSitesEnabled() ||
+         content::SiteIsolationPolicy::AreIsolatedOriginsEnabled();
+}
+
 }  // namespace site_isolation

components/site_isolation/site_isolation_policy.h

@@ -45,6 +45,10 @@ class SiteIsolationPolicy {
   // memory threshold.
   static bool ShouldDisableSiteIsolationDueToMemoryThreshold();
 
+  // Returns true if the PDF compositor should be enabled to allow out-of-
+  // process iframes (OOPIF's) to print properly.
+  static bool ShouldPdfCompositorBeEnabledForOopifs();
+
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(SiteIsolationPolicy);
 };

content/public/browser/site_isolation_policy.cc

@@ -113,17 +113,6 @@ bool SiteIsolationPolicy::IsErrorPageIsolationEnabled(bool in_main_frame) {
   return GetContentClient()->browser()->ShouldIsolateErrorPage(in_main_frame);
 }
 
-// static
-bool SiteIsolationPolicy::ShouldPdfCompositorBeEnabledForOopifs() {
-  // TODO(weili): We only create pdf compositor client and use pdf compositor
-  // service when site-per-process or isolate-origins flag/feature is enabled,
-  // or top-document-isolation feature is enabled. This may not cover all cases
-  // where OOPIF is used such as isolate-extensions, but should be good for
-  // feature testing purpose. Eventually, we will remove this check and use pdf
-  // compositor service by default for printing.
-  return AreIsolatedOriginsEnabled() || UseDedicatedProcessesForAllSites();
-}
-
 // static
 bool SiteIsolationPolicy::AreDynamicIsolatedOriginsEnabled() {
   return !IsSiteIsolationDisabled();

content/public/browser/site_isolation_policy.h

@@ -38,10 +38,6 @@ class CONTENT_EXPORT SiteIsolationPolicy {
   // Returns true if error page isolation is enabled.
   static bool IsErrorPageIsolationEnabled(bool in_main_frame);
 
-  // Returns true if the PDF compositor should be enabled to allow out-of-
-  // process iframes (OOPIF's) to print properly.
-  static bool ShouldPdfCompositorBeEnabledForOopifs();
-
   // Returns true if isolated origins may be added at runtime in response
   // to hints such as users typing in a password or (in the future) an origin
   // opting itself into isolation via a header.