Breakdown ClientLoadedTime histograms into buckets based on number of clients. Fix RequestTimeThrot…

content/browser/loader/resource_scheduler.cc

Index: content/browser/loader/resource_scheduler.cc
diff --git a/content/browser/loader/resource_scheduler.cc b/content/browser/loader/resource_scheduler.cc
index 64d7e654e61a95d47241026038eefe681ebdbc27..f643dafadd373895eff629504d35f99562f1637e 100644
--- a/content/browser/loader/resource_scheduler.cc
+++ b/content/browser/loader/resource_scheduler.cc
@@ -27,20 +27,40 @@ namespace content {
 
 namespace {
 
+// Post ResourceScheduler histograms of the following forms:
+// If |histogram_suffix| is NULL or the empty string:
+//   ResourceScheduler.base_name.histogram_name
+// Else:
+//   ResourceScheduler.base_name.histogram_name.histogram_suffix
 void PostHistogram(const char* base_name,
-                   const char* suffix,
+                   const char* histogram_name,
+                   const char* histogram_suffix,
                    base::TimeDelta time) {
-  std::string histogram_name =
-      base::StringPrintf("ResourceScheduler.%s.%s", base_name, suffix);
+  std::string histogram =
+      base::StringPrintf("ResourceScheduler.%s.%s", base_name, histogram_name);
+  if (histogram_suffix && histogram_suffix[0] != '\0')
+    histogram = histogram + "." + histogram_suffix;
   base::HistogramBase* histogram_counter = base::Histogram::FactoryTimeGet(
-      histogram_name,
-      base::TimeDelta::FromMilliseconds(1),
-      base::TimeDelta::FromMinutes(5),
-      50,
+      histogram, base::TimeDelta::FromMilliseconds(1),
+      base::TimeDelta::FromMinutes(5), 50,
       base::Histogram::kUmaTargetedHistogramFlag);
   histogram_counter->AddTime(time);
 }
 
+// For use with PostHistogram to specify the correct string for histogram
+// suffixes based on number of Clients.
+const char* GetNumClientsString(size_t num_clients) {
+  if (num_clients == 1)
+    return "1Client";
+  else if (num_clients <= 5)
+    return "Max5Clients";
+  else if (num_clients <= 15)
+    return "Max15Clients";
+  else if (num_clients <= 30)
+    return "Max30Clients";
+  return "Over30Clients";
+}
+
 }  // namespace
 
 static const size_t kCoalescedTimerPeriod = 5000;
@@ -182,9 +202,9 @@ class ResourceScheduler::ScheduledResourceRequest
       controller()->Resume();
       time_was_deferred = time - time_deferred_;
     }
-    PostHistogram("RequestTimeDeferred", client_state, time_was_deferred);
-    PostHistogram(
-        "RequestTimeThrottled", client_state, time - request_->creation_time());
+    PostHistogram("RequestTimeDeferred", client_state, NULL, time_was_deferred);
+    PostHistogram("RequestTimeThrottled", client_state, NULL,
+                  time - request_->creation_time());
     // TODO(aiolos): Remove one of the above histograms after gaining an
     // understanding of the difference between them and which one is more
     // interesting.
@@ -368,15 +388,23 @@ class ResourceScheduler::Client {
       return;
     }
     base::TimeTicks cur_time = base::TimeTicks::Now();
+    const char* num_clients =
+        GetNumClientsString(scheduler_->client_map_.size());
     const char* client_catagory = "Other";
     if (last_active_switch_time_.is_null()) {
       client_catagory = is_active() ? "Active" : "Background";
     } else if (is_active()) {
-      PostHistogram("ClientLoadedTime", "Other.SwitchedToActive",
-                    cur_time - last_active_switch_time_);
+      base::TimeDelta time_since_active = cur_time - last_active_switch_time_;
+      PostHistogram("ClientLoadedTime", "Other.SwitchedToActive", NULL,
+                    time_since_active);
+      PostHistogram("ClientLoadedTime", "Other.SwitchedToActive", num_clients,
+                    time_since_active);
     }
-    PostHistogram("ClientLoadedTime", client_catagory,
-                  cur_time - load_started_time_);
+    base::TimeDelta time_since_load_started = cur_time - load_started_time_;
+    PostHistogram("ClientLoadedTime", client_catagory, NULL,
+                  time_since_load_started);
+    PostHistogram("ClientLoadedTime", client_catagory, num_clients,
+                  time_since_load_started);
     // TODO(aiolos): The above histograms will not take main resource load time
     // into account with PlzNavigate into account. The ResourceScheduler also
     // will load the main resources without a clients with the current logic.
@@ -878,8 +906,6 @@ void ResourceScheduler::OnClientCreated(int child_id,
   Client* client = new Client(this, is_visible, is_audible);
   client_map_[client_id] = client;
 
-  // TODO(aiolos): set Client visibility/audibility when signals are added
-  // this will UNTHROTTLE Clients as needed
   client->UpdateThrottleState();
 }