Add UMA histogram for the number of SiteInstances per BrowsingInstance.

chrome/browser/site_details.cc

Index: chrome/browser/site_details.cc
diff --git a/chrome/browser/site_details.cc b/chrome/browser/site_details.cc
index b38c389d48a8e4ba1d3d3250f95511803f2d728f..d632932248267d4c8c5f8a13d871aa21ff2c62fe 100644
--- a/chrome/browser/site_details.cc
+++ b/chrome/browser/site_details.cc
@@ -105,11 +105,36 @@ void CollectForScenario(std::map<RenderFrameHost*, GURL>* frame_urls,
   (*frame_urls)[frame] = site;
 }
 
+content::SiteInstance* DeterminePrimarySiteInstance(
+    content::SiteInstance* instance,
+    SiteData* site_data) {
+  // Find the BrowsingInstance this WebContents belongs to by iterating over
+  // the "primary" SiteInstances of each BrowsingInstance we've seen so far.
+  for (auto& already_collected_instance : site_data->instances) {

[sky, 2015/12/03 00:04:05]

nit: already_collected_instance is a bit confusing as a name as it's really a
pair.

[nasko, 2015/12/03 01:36:57]

Yes, it is a pair, but it is the SiteInstance and a set of its related other
SiteInstances. So we are in a sense iterating through all of the known
SiteInstances. I've changed it to existing_site_instance as I can't find a
better name. Any suggestions are welcome.

+    if (instance->IsRelatedSiteInstance(already_collected_instance.first)) {
+      already_collected_instance.second.insert(instance);
+      return already_collected_instance.first;
+    }
+  }
+
+  // Add this as the "primary" SiteInstance of a new BrowsingInstance.
+  std::pair<SiteInstanceMap::iterator, bool> result =
+      site_data->instances.insert(
+          std::make_pair(instance, std::set<SiteInstance*>()));
+
+  // Also include it in the set of all SiteInstances for the BrowsingInstance.
+  result.first->second.insert(instance);

[sky, 2015/12/03 00:04:05]

Unless I'm missing something, isn't what you have above the same as:
site_data->instances[instance].clear();
site_data->instances[instance].insert(instance);

which is a lot easier to understand.

[nasko, 2015/12/03 01:36:57]

Done.

+
+  return instance;
+}
+
 void CollectCurrentSnapshot(SiteData* site_data, RenderFrameHost* frame) {
   if (frame->GetParent()) {
     if (frame->GetSiteInstance() != frame->GetParent()->GetSiteInstance())
       site_data->out_of_process_frames++;
   }
+
+  DeterminePrimarySiteInstance(frame->GetSiteInstance(), site_data);
 }
 
 }  // namespace
@@ -132,21 +157,8 @@ SiteDetails::~SiteDetails() {}
 void SiteDetails::CollectSiteInfo(WebContents* contents,
                                   SiteData* site_data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // Find the BrowsingInstance this WebContents belongs to by iterating over
-  // the "primary" SiteInstances of each BrowsingInstance we've seen so far.
-  SiteInstance* instance = contents->GetSiteInstance();
-  SiteInstance* primary = NULL;
-  for (SiteInstance* already_collected_instance : site_data->instances) {
-    if (instance->IsRelatedSiteInstance(already_collected_instance)) {
-      primary = already_collected_instance;
-      break;
-    }
-  }
-  if (!primary) {
-    // Remember this as the "primary" SiteInstance of a new BrowsingInstance.
-    primary = instance;
-    site_data->instances.push_back(instance);
-  }
+  SiteInstance* primary =
+      DeterminePrimarySiteInstance(contents->GetSiteInstance(), site_data);
 
   // Now keep track of how many sites we have in this BrowsingInstance (and
   // overall), including sites in iframes.
@@ -185,6 +197,10 @@ void SiteDetails::UpdateHistograms(
     }
     num_browsing_instances += i->second.scenarios[ISOLATE_ALL_SITES]
                                   .browsing_instance_site_map.size();
+    for (const auto& site : i->second.instances) {

[sky, 2015/12/03 00:04:05]

Above you call 'site' 'already_collected_instance'. Both are confusing, but be
consistent.

[nasko, 2015/12/03 01:36:57]

Changed to be more consistent.

+      UMA_HISTOGRAM_COUNTS_100("SiteIsolation.SiteInstancesPerBrowsingInstance",
+                               site.second.size());
+    }
     num_oopifs += i->second.out_of_process_frames;
   }