From 37dfd9de7667e9d177edc27e208359cd0020fb26 Mon Sep 17 00:00:00 2001
From: nick <nick@chromium.org>
Date: Mon, 14 Sep 2015 12:54:03 -0700
Subject: [PATCH] Add SiteIsolation.IsolateExtensions metrics and tests.

These new UMA metrics estimate the process-count impact of a policy
wherein extension content and web content never share processes,
even if one contains the other in an iframe.

SiteDetailsBrowserTest: add tests for the new metrics.

WebContentsImpl: Fix a UMA metrics counting error where
<iframe src="about:blank"> was counted as a distinct site.
Now we'll only do that if we see "about:blank" in an
undifferentiated SiteInstance.

BUG=528049

Review URL: https://codereview.chromium.org/1313863006

Cr-Commit-Position: refs/heads/master@{#348690}
---
 chrome/browser/site_details.cc                    | 161 ++++++-----
 chrome/browser/site_details.h                     |  37 ++-
 chrome/browser/site_details_browsertest.cc        | 324 +++++++++++++++++++++-
 content/browser/web_contents/web_contents_impl.cc |   8 +
 tools/metrics/histograms/histograms.xml           |  33 +++
 5 files changed, 487 insertions(+), 76 deletions(-)

diff --git a/chrome/browser/site_details.cc b/chrome/browser/site_details.cc
index 5d9fc073103a..54970b6b6acc 100644
--- a/chrome/browser/site_details.cc
+++ b/chrome/browser/site_details.cc
@@ -7,13 +7,50 @@
 #include "base/metrics/histogram.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
+#include "extensions/common/constants.h"
 
 using content::BrowserThread;
 using content::RenderProcessHost;
 using content::SiteInstance;
 using content::WebContents;
 
-SiteData::SiteData() {}
+namespace {
+
+bool ShouldIsolate(IsolationScenarioType policy, const GURL& site) {
+  switch (policy) {
+    case ISOLATE_ALL_SITES:
+      return true;
+    case ISOLATE_HTTPS_SITES:
+      // Note: For estimation purposes "isolate https sites" is really
+      // implemented as "isolate non-http sites". This means that, for example,
+      // the New Tab Page gets counted as two processes under this policy, and
+      // extensions are isolated as well.
+      return !site.SchemeIs(url::kHttpScheme);
+    case ISOLATE_EXTENSIONS:
+      return site.SchemeIs(extensions::kExtensionScheme);
+  }
+  NOTREACHED();
+  return true;
+}
+
+}  // namespace
+
+IsolationScenario::IsolationScenario() : policy(ISOLATE_ALL_SITES) {}
+
+IsolationScenario::~IsolationScenario() {}
+
+void IsolationScenario::CollectSiteInfoForScenario(SiteInstance* primary,
+                                                   const GURL& site) {
+  const GURL& isolated = ShouldIsolate(policy, site) ? site : GURL("http://");
+  sites.insert(isolated);
+  browsing_instance_site_map[primary->GetId()].insert(isolated);
+}
+
+SiteData::SiteData() {
+  scenarios[ISOLATE_ALL_SITES].policy = ISOLATE_ALL_SITES;
+  scenarios[ISOLATE_HTTPS_SITES].policy = ISOLATE_HTTPS_SITES;
+  scenarios[ISOLATE_EXTENSIONS].policy = ISOLATE_EXTENSIONS;
+}
 
 SiteData::~SiteData() {}
 
@@ -30,9 +67,9 @@ void SiteDetails::CollectSiteInfo(WebContents* contents,
   // the "primary" SiteInstances of each BrowsingInstance we've seen so far.
   SiteInstance* instance = contents->GetSiteInstance();
   SiteInstance* primary = NULL;
-  for (size_t i = 0; i < site_data->instances.size(); ++i) {
-    if (instance->IsRelatedSiteInstance(site_data->instances[i])) {
-      primary = site_data->instances[i];
+  for (SiteInstance* already_collected_instance : site_data->instances) {
+    if (instance->IsRelatedSiteInstance(already_collected_instance)) {
+      primary = already_collected_instance;
       break;
     }
   }
@@ -44,29 +81,19 @@ void SiteDetails::CollectSiteInfo(WebContents* contents,
 
   // Now keep track of how many sites we have in this BrowsingInstance (and
   // overall), including sites in iframes.
-  std::set<GURL> sites_in_tab = contents->GetSitesInTab();
-  for (std::set<GURL>::iterator iter = sites_in_tab.begin();
-       iter != sites_in_tab.end(); ++iter) {
-    // Skip about:blank, since we won't usually give it its own process.
-    // Because about:blank has no host, its site URL will be blank.
-    if (iter->is_empty())
-      continue;
-
-    // Make sure we don't overcount process-per-site sites, like the NTP.
-    if (RenderProcessHost::ShouldUseProcessPerSite(browser_context, *iter) &&
-        site_data->sites.find(*iter) != site_data->sites.end()) {
+  for (const GURL& site : contents->GetSitesInTab()) {
+    // Make sure we don't overcount process-per-site sites, like the NTP or
+    // extensions, by skipping over them if they're already logged for
+    // ISOLATE_ALL_SITES.
+    if (RenderProcessHost::ShouldUseProcessPerSite(browser_context, site) &&
+        site_data->scenarios[ISOLATE_ALL_SITES].sites.find(site) !=
+            site_data->scenarios[ISOLATE_ALL_SITES].sites.end()) {
       continue;
     }
 
-    site_data->sites.insert(*iter);
-    site_data->instance_site_map[primary->GetId()].insert(*iter);
-
-    // Also keep track of how things would look if we only isolated HTTPS sites.
-    // In this model, all HTTP sites are grouped into one "http://" site.  HTTPS
-    // and other schemes (e.g., chrome:) are still isolated.
-    GURL https_site = iter->SchemeIs("http") ? GURL("http://") : *iter;
-    site_data->https_sites.insert(https_site);
-    site_data->instance_https_site_map[primary->GetId()].insert(https_site);
+    for (IsolationScenario& scenario : site_data->scenarios) {
+      scenario.CollectSiteInfoForScenario(primary, site);
+    }
   }
 }
 
@@ -78,39 +105,33 @@ void SiteDetails::UpdateHistograms(
   int process_limit = RenderProcessHost::GetMaxRendererProcessCount();
 
   // Sum the number of sites and SiteInstances in each BrowserContext.
-  int num_sites = 0;
-  int num_https_sites = 0;
+  int num_sites[ISOLATION_SCENARIO_LAST + 1] = {};
+  int num_isolated_site_instances[ISOLATION_SCENARIO_LAST + 1] = {};
   int num_browsing_instances = 0;
-  int num_isolated_site_instances = 0;
-  int num_isolated_https_site_instances = 0;
   for (BrowserContextSiteDataMap::const_iterator i = site_data_map.begin();
        i != site_data_map.end(); ++i) {
-    num_sites += i->second.sites.size();
-    num_https_sites += i->second.https_sites.size();
-    num_browsing_instances += i->second.instance_site_map.size();
-    for (BrowsingInstanceSiteMap::const_iterator iter =
-             i->second.instance_site_map.begin();
-         iter != i->second.instance_site_map.end(); ++iter) {
-      num_isolated_site_instances += iter->second.size();
-    }
-    for (BrowsingInstanceSiteMap::const_iterator iter =
-             i->second.instance_https_site_map.begin();
-         iter != i->second.instance_https_site_map.end(); ++iter) {
-      num_isolated_https_site_instances += iter->second.size();
+    for (const IsolationScenario& scenario : i->second.scenarios) {
+      num_sites[scenario.policy] += scenario.sites.size();
+      for (auto& browsing_instance : scenario.browsing_instance_site_map) {
+        num_isolated_site_instances[scenario.policy] +=
+            browsing_instance.second.size();
+      }
     }
+    num_browsing_instances += i->second.scenarios[ISOLATE_ALL_SITES]
+                                  .browsing_instance_site_map.size();
   }
 
-  // Predict the number of processes needed when isolating all sites and when
-  // isolating only HTTPS sites.
-  int process_count_lower_bound = num_sites;
-  int process_count_upper_bound = num_sites + process_limit - 1;
-  int process_count_estimate = std::min(
-      num_isolated_site_instances, process_count_upper_bound);
-
-  int process_count_https_lower_bound = num_https_sites;
-  int process_count_https_upper_bound = num_https_sites + process_limit - 1;
-  int process_count_https_estimate = std::min(
-      num_isolated_https_site_instances, process_count_https_upper_bound);
+  // Predict the number of processes needed when isolating all sites, when
+  // isolating only HTTPS sites, and when isolating extensions.
+  int process_count_lower_bound[ISOLATION_SCENARIO_LAST + 1];
+  int process_count_upper_bound[ISOLATION_SCENARIO_LAST + 1];
+  int process_count_estimate[ISOLATION_SCENARIO_LAST + 1];
+  for (int policy = 0; policy <= ISOLATION_SCENARIO_LAST; policy++) {
+    process_count_lower_bound[policy] = num_sites[policy];
+    process_count_upper_bound[policy] = num_sites[policy] + process_limit - 1;
+    process_count_estimate[policy] = std::min(
+        num_isolated_site_instances[policy], process_count_upper_bound[policy]);
+  }
 
   // Just renderer process count:
   UMA_HISTOGRAM_COUNTS_100("SiteIsolation.CurrentRendererProcessCount",
@@ -118,30 +139,40 @@ void SiteDetails::UpdateHistograms(
   UMA_HISTOGRAM_COUNTS_100(
       "SiteIsolation.BrowsingInstanceCount",
       num_browsing_instances);
-  UMA_HISTOGRAM_COUNTS_100(
-      "SiteIsolation.IsolateAllSitesProcessCountNoLimit",
-      num_isolated_site_instances);
+  UMA_HISTOGRAM_COUNTS_100("SiteIsolation.IsolateAllSitesProcessCountNoLimit",
+                           num_isolated_site_instances[ISOLATE_ALL_SITES]);
   UMA_HISTOGRAM_COUNTS_100(
       "SiteIsolation.IsolateAllSitesProcessCountLowerBound",
-      process_count_lower_bound);
-  UMA_HISTOGRAM_COUNTS_100(
-      "SiteIsolation.IsolateAllSitesProcessCountEstimate",
-      process_count_estimate);
-  UMA_HISTOGRAM_COUNTS_100(
-      "SiteIsolation.IsolateHttpsSitesProcessCountNoLimit",
-      num_isolated_https_site_instances);
+      process_count_lower_bound[ISOLATE_ALL_SITES]);
+  UMA_HISTOGRAM_COUNTS_100("SiteIsolation.IsolateAllSitesProcessCountEstimate",
+                           process_count_estimate[ISOLATE_ALL_SITES]);
+
+  UMA_HISTOGRAM_COUNTS_100("SiteIsolation.IsolateHttpsSitesProcessCountNoLimit",
+                           num_isolated_site_instances[ISOLATE_HTTPS_SITES]);
   UMA_HISTOGRAM_COUNTS_100(
       "SiteIsolation.IsolateHttpsSitesProcessCountLowerBound",
-      process_count_https_lower_bound);
+      process_count_lower_bound[ISOLATE_HTTPS_SITES]);
   UMA_HISTOGRAM_COUNTS_100(
       "SiteIsolation.IsolateHttpsSitesProcessCountEstimate",
-      process_count_https_estimate);
+      process_count_estimate[ISOLATE_HTTPS_SITES]);
+
+  UMA_HISTOGRAM_COUNTS_100("SiteIsolation.IsolateExtensionsProcessCountNoLimit",
+                           num_isolated_site_instances[ISOLATE_EXTENSIONS]);
+  UMA_HISTOGRAM_COUNTS_100(
+      "SiteIsolation.IsolateExtensionsProcessCountLowerBound",
+      process_count_lower_bound[ISOLATE_EXTENSIONS]);
+  UMA_HISTOGRAM_COUNTS_100(
+      "SiteIsolation.IsolateExtensionsProcessCountEstimate",
+      process_count_estimate[ISOLATE_EXTENSIONS]);
 
   // Total process count:
   UMA_HISTOGRAM_COUNTS_100(
       "SiteIsolation.IsolateAllSitesTotalProcessCountEstimate",
-      process_count_estimate + non_renderer_process_count);
+      process_count_estimate[ISOLATE_ALL_SITES] + non_renderer_process_count);
   UMA_HISTOGRAM_COUNTS_100(
       "SiteIsolation.IsolateHttpsSitesTotalProcessCountEstimate",
-      process_count_https_estimate + non_renderer_process_count);
+      process_count_estimate[ISOLATE_HTTPS_SITES] + non_renderer_process_count);
+  UMA_HISTOGRAM_COUNTS_100(
+      "SiteIsolation.IsolateExtensionsTotalProcessCountEstimate",
+      process_count_estimate[ISOLATE_EXTENSIONS] + non_renderer_process_count);
 }
diff --git a/chrome/browser/site_details.h b/chrome/browser/site_details.h
index 4eb99b97eeba..a8833d2601ac 100644
--- a/chrome/browser/site_details.h
+++ b/chrome/browser/site_details.h
@@ -11,7 +11,32 @@
 #include "content/public/browser/web_contents.h"
 
 // Maps an ID representing each BrowsingInstance to a set of site URLs.
-typedef base::hash_map<int32, std::set<GURL> > BrowsingInstanceSiteMap;
+typedef base::hash_map<int32, std::set<GURL>> BrowsingInstanceSiteMap;
+
+// This enum represents various alternative process model policies that we want
+// to evaluate. We'll estimate the process cost of each scenario.
+enum IsolationScenarioType {
+  ISOLATE_ALL_SITES,
+  ISOLATE_HTTPS_SITES,
+  ISOLATE_EXTENSIONS,
+  ISOLATION_SCENARIO_LAST = ISOLATE_EXTENSIONS
+};
+
+// Contains the state required to estimate the process count under a particular
+// process model. We have one of these per IsolationScenarioType.
+struct IsolationScenario {
+  IsolationScenario();
+  ~IsolationScenario();
+
+  void CollectSiteInfoForScenario(content::SiteInstance* primary,
+                                  const GURL& site);
+  void GetProcessCountEstimate();
+  void GetProcessCountLowerBound();
+
+  IsolationScenarioType policy;
+  std::set<GURL> sites;
+  BrowsingInstanceSiteMap browsing_instance_site_map;
+};
 
 // Information about the sites and SiteInstances in each BrowsingInstance, for
 // use in estimating the number of processes needed for various process models.
@@ -19,14 +44,14 @@ struct SiteData {
   SiteData();
   ~SiteData();
 
-  std::set<GURL> sites;
-  std::set<GURL> https_sites;
+  // One IsolationScenario object per IsolationScenarioType.
+  IsolationScenario scenarios[ISOLATION_SCENARIO_LAST + 1];
+
+  // Global list of all SiteInstances, used for de-duping related instances.
   std::vector<content::SiteInstance*> instances;
-  BrowsingInstanceSiteMap instance_site_map;
-  BrowsingInstanceSiteMap instance_https_site_map;
 };
 
-// Maps a BrowserContext to information about the SiteInstances it contains.
+// Maps a BrowserContext to information about the sites it contains.
 typedef base::hash_map<content::BrowserContext*, SiteData>
     BrowserContextSiteDataMap;
 
diff --git a/chrome/browser/site_details_browsertest.cc b/chrome/browser/site_details_browsertest.cc
index b84be4c35683..c9063769952f 100644
--- a/chrome/browser/site_details_browsertest.cc
+++ b/chrome/browser/site_details_browsertest.cc
@@ -5,22 +5,33 @@
 #include "chrome/browser/site_details.h"
 
 #include "base/bind_helpers.h"
+#include "base/files/file_path.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/test/histogram_tester.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/metrics/metrics_memory_details.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/common/value_builder.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 using base::Bucket;
+using content::WebContents;
+using extensions::DictionaryBuilder;
+using extensions::Extension;
+using extensions::ListBuilder;
+using extensions::TestExtensionDir;
 using testing::ContainerEq;
 using testing::ElementsAre;
 
@@ -57,7 +68,7 @@ class TestMemoryDetails : public MetricsMemoryDetails {
 
 }  // namespace
 
-class SiteDetailsBrowserTest : public InProcessBrowserTest {
+class SiteDetailsBrowserTest : public ExtensionBrowserTest {
  public:
   SiteDetailsBrowserTest() {}
   ~SiteDetailsBrowserTest() override {}
@@ -73,14 +84,55 @@ class SiteDetailsBrowserTest : public InProcessBrowserTest {
     ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   }
 
+  // Create and install an extension that has a couple of web-accessible
+  // resources and, optionally, a background process.
+  const Extension* CreateExtension(const std::string& name,
+                                   bool has_background_process) {
+    scoped_ptr<TestExtensionDir> dir(new TestExtensionDir);
+
+    DictionaryBuilder manifest;
+    manifest.Set("name", name)
+        .Set("version", "1.0")
+        .Set("manifest_version", 2)
+        .Set("web_accessible_resources", ListBuilder()
+                                             .Append("blank_iframe.html")
+                                             .Append("http_iframe.html"));
+
+    if (has_background_process) {
+      manifest.Set("background",
+                   DictionaryBuilder().Set("scripts",
+                                           ListBuilder().Append("script.js")));
+      dir->WriteFile(FILE_PATH_LITERAL("script.js"),
+                     "console.log('" + name + " running');");
+    }
+
+    dir->WriteFile(FILE_PATH_LITERAL("blank_iframe.html"),
+                   "<html><body>" + name +
+                       ", blank iframe: "
+                       "<iframe width=40 height=40></iframe></body></html>");
+    GURL iframe_url = embedded_test_server()->GetURL("w.com", "/title1.html");
+    dir->WriteFile(FILE_PATH_LITERAL("http_iframe.html"),
+                   "<html><body>" + name +
+                       ", http:// iframe: "
+                       "<iframe width=40 height=40 src='" +
+                       iframe_url.spec() + "'></iframe></body></html>");
+    dir->WriteManifest(manifest.ToJSON());
+
+    const Extension* extension = LoadExtension(dir->unpacked_path());
+    EXPECT_TRUE(extension);
+    temp_dirs_.push_back(dir.release());
+    return extension;
+  }
+
  private:
+  ScopedVector<TestExtensionDir> temp_dirs_;
   DISALLOW_COPY_AND_ASSIGN(SiteDetailsBrowserTest);
 };
 
 // Test the accuracy of SiteDetails process estimation, in the presence of
 // multiple iframes, navigation, multiple BrowsingInstances, and multiple tabs
 // in the same BrowsingInstance.
-IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
+IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyIframes) {
   // Page with 14 nested oopifs across 9 sites (a.com through i.com).
   // None of these are https.
   GURL abcdefghi_url = embedded_test_server()->GetURL(
@@ -116,6 +168,15 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
   EXPECT_THAT(details->uma()->GetAllSamples(
                   "SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
               ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(1, 1)));
 
   // Navigate to a different, disjoint set of 7 sites.
   GURL pqrstuv_url = embedded_test_server()->GetURL(
@@ -150,6 +211,15 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
   EXPECT_THAT(details->uma()->GetAllSamples(
                   "SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
               ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(1, 1)));
 
   // Open a second tab (different BrowsingInstance) with 4 sites (a through d).
   GURL abcd_url = embedded_test_server()->GetURL(
@@ -183,6 +253,15 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
   EXPECT_THAT(details->uma()->GetAllSamples(
                   "SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
               ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(2, 1)));
 
   // Open a third tab (different BrowsingInstance) with the same 4 sites.
   AddTabAtIndex(2, abcd_url, ui::PAGE_TRANSITION_TYPED);
@@ -215,11 +294,20 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
   EXPECT_THAT(details->uma()->GetAllSamples(
                   "SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
               ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(3, 1)));
 
-  // Do a window.open() to obtain a fourth tab in the same BrowsingInstance as
-  // the third tab. The new page uses the same four sites "a-d" as third tab,
+  // From the third tab, window.open() a fourth tab in the same
+  // BrowsingInstance, to a page using the same four sites "a-d" as third tab,
   // plus an additional site "e". The estimated process counts should increase
-  // by one (not five) from the previous result, since the new tab can reuse the
+  // by one (not five) from the previous scenario, as the new tab can reuse the
   // four processes already in the BrowsingInstance.
   GURL dcbae_url = embedded_test_server()->GetURL(
       "a.com", "/cross_site_iframe_factory.html?d(c(b(a(e))))");
@@ -260,4 +348,230 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
   EXPECT_THAT(details->uma()->GetAllSamples(
                   "SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
               ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(3, 1)));
+}
+
+IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, IsolateExtensions) {
+  // We start on "about:blank", which should be credited with a process in this
+  // case.
+  scoped_refptr<TestMemoryDetails> details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(1, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(1, 1)));
+
+  // Install one script-injecting extension with background page, and an
+  // extension with web accessible resources.
+  const Extension* extension1 = CreateExtension("Extension One", true);
+  const Extension* extension2 = CreateExtension("Extension Two", false);
+
+  // Open two a.com tabs (with cross site http iframes). IsolateExtensions mode
+  // should have no effect so far, since there are no frames straddling the
+  // extension/web boundary.
+  GURL tab1_url = embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(b,c)");
+  ui_test_utils::NavigateToURL(browser(), tab1_url);
+  WebContents* tab1 = browser()->tab_strip_model()->GetWebContentsAt(0);
+  GURL tab2_url = embedded_test_server()->GetURL(
+      "a.com", "/cross_site_iframe_factory.html?a(d,e)");
+  AddTabAtIndex(1, tab2_url, ui::PAGE_TRANSITION_TYPED);
+  WebContents* tab2 = browser()->tab_strip_model()->GetWebContentsAt(1);
+
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(3, 1)));
+
+  // Test that "one process per extension" applies even when web content has an
+  // extension iframe.
+
+  // Tab1 navigates its first iframe to a resource of extension1. This shouldn't
+  // result in a new extension process (it should share with extension1's
+  // background page).
+  content::NavigateIframeToURL(
+      tab1, "child-0", extension1->GetResourceURL("/blank_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(3, 1)));
+
+  // Tab2 navigates its first iframe to a resource of extension1. This also
+  // shouldn't result in a new extension process (it should share with the
+  // background page and the other iframe).
+  content::NavigateIframeToURL(
+      tab2, "child-0", extension1->GetResourceURL("/blank_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(3, 1)));
+
+  // Tab1 navigates its second iframe to a resource of extension2. This SHOULD
+  // result in a new process since extension2 had no existing process.
+  content::NavigateIframeToURL(
+      tab1, "child-1", extension2->GetResourceURL("/blank_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(4, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(4, 1)));
+
+  // Tab2 navigates its second iframe to a resource of extension2. This should
+  // share the existing extension2 process.
+  content::NavigateIframeToURL(
+      tab2, "child-1", extension2->GetResourceURL("/blank_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(4, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(4, 1)));
+
+  // Install extension3 (identical config to extension2)
+  const Extension* extension3 = CreateExtension("Extension Three", false);
+
+  // Navigate Tab2 to a top-level page from extension3. There are four processes
+  // now: one for tab1's main frame, and one for each of the extensions:
+  // extension1 has a process because it has a background page; extension2 is
+  // used as an iframe in tab1, and extension3 is the top-level frame in tab2.
+  ui_test_utils::NavigateToURL(browser(),
+                               extension3->GetResourceURL("blank_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(4, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(4, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(4, 1)));
+
+  // Navigate tab2 to a different extension3 page containing a web iframe. The
+  // iframe should get its own process. The lower bound number indicates that,
+  // in theory, the iframe could share a process with tab1's main frame.
+  ui_test_utils::NavigateToURL(browser(),
+                               extension3->GetResourceURL("http_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(5, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(4, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(5, 1)));
+
+  // Navigate tab1 to an extension3 page with an extension3 iframe. There should
+  // be three processes estimated by IsolateExtensions: one for extension3, one
+  // for extension1's background page, and one for the web iframe in tab2.
+  browser()->tab_strip_model()->ActivateTabAt(0, true);
+  ui_test_utils::NavigateToURL(browser(),
+                               extension3->GetResourceURL("blank_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(3, 1)));
+
+  // Now navigate tab1 to an extension3 page with a web iframe. This could share
+  // a process with tab2's iframe (the LowerBound number), or it could get its
+  // own process (the Estimate number).
+  ui_test_utils::NavigateToURL(browser(),
+                               extension3->GetResourceURL("http_iframe.html"));
+  details = new TestMemoryDetails();
+  details->StartFetchAndWait();
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.CurrentRendererProcessCount"),
+              ElementsAre(Bucket(2, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+              ElementsAre(Bucket(4, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+              ElementsAre(Bucket(3, 1)));
+  EXPECT_THAT(details->uma()->GetAllSamples(
+                  "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+              ElementsAre(Bucket(4, 1)));
 }
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 99e2fa485014..7d03dc1e3299 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -171,6 +171,14 @@ void NotifyCacheOnIO(
 bool CollectSites(BrowserContext* context,
                   std::set<GURL>* sites,
                   FrameTreeNode* node) {
+  // Record about:blank as a real (process-having) site only if the SiteInstance
+  // is unassigned. Do not otherwise depend on the siteinstance's site URL,
+  // since its value reflects the current process model, and this function
+  // should behave identically across all process models.
+  if (node->current_url() == GURL(url::kAboutBlankURL) &&
+      node->current_frame_host()->GetSiteInstance()->HasSite()) {
+    return true;
+  }
   sites->insert(SiteInstance::GetSiteForURL(context, node->current_url()));
   return true;
 }
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 8a7ca4dce983..2bb468169291 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -43809,6 +43809,39 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="SiteIsolation.IsolateExtensionsProcessCountEstimate">
+  <owner>nick@chromium.org</owner>
+  <summary>
+    The upper bound of the predicted renderer process count if we isolated only
+    Chrome extensions, subject to the process limit.  Recorded once per UMA
+    ping.
+  </summary>
+</histogram>
+
+<histogram name="SiteIsolation.IsolateExtensionsProcessCountLowerBound">
+  <owner>nick@chromium.org</owner>
+  <summary>
+    The lower bound of the predicted renderer process count if we isolated only
+    Chrome extensions, subject to the process limit. Recorded once per UMA ping.
+  </summary>
+</histogram>
+
+<histogram name="SiteIsolation.IsolateExtensionsProcessCountNoLimit">
+  <owner>nick@chromium.org</owner>
+  <summary>
+    The predicted renderer process count if we isolated only Chrome extensions
+    and if there were no process limit.  Recorded once per UMA ping.
+  </summary>
+</histogram>
+
+<histogram name="SiteIsolation.IsolateExtensionsTotalProcessCountEstimate">
+  <owner>nick@chromium.org</owner>
+  <summary>
+    The predicted total process count if we isolated only Chrome extensions,
+    subject to the process limit.  Recorded once per UMA ping.
+  </summary>
+</histogram>
+
 <histogram name="SiteIsolation.IsolateHttpsSitesProcessCountEstimate">
   <owner>creis@chromium.org</owner>
   <summary>
-- 
2.11.4.GIT