Record offline histograms with suffix via Histogram::FactoryGet

components/offline_pages/offline_page_model.cc

Index: components/offline_pages/offline_page_model.cc
diff --git a/components/offline_pages/offline_page_model.cc b/components/offline_pages/offline_page_model.cc
index af39f37a8fd0d318eb0679a3ab940a786be1617e..709c742d1eb2654b56ef18fda121f0de79037bfa 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model.cc
@@ -19,6 +19,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "components/offline_pages/archive_manager.h"
+#include "components/offline_pages/client_namespace_constants.h"
 #include "components/offline_pages/client_policy_controller.h"
 #include "components/offline_pages/offline_page_item.h"
 #include "components/offline_pages/offline_page_storage_manager.h"
@@ -74,18 +75,6 @@ SavePageResult ToSavePageResult(ArchiverResult archiver_result) {
   return result;
 }
 
-std::string AddHistogramSuffix(const ClientId& client_id,
-                               const char* histogram_name) {
-  if (client_id.name_space.empty()) {
-    NOTREACHED();
-    return histogram_name;
-  }
-  std::string adjusted_histogram_name(histogram_name);
-  adjusted_histogram_name += ".";
-  adjusted_histogram_name += client_id.name_space;
-  return adjusted_histogram_name;
-}
-
 void ReportStorageHistogramsAfterSave(
     const ArchiveManager::StorageStats& storage_stats) {
   const int kMB = 1024 * 1024;
@@ -122,6 +111,131 @@ void ReportStorageHistogramsAfterDelete(
   }
 }
 
+void ReportPageHistogramAfterSave(const ClientId& client_id,
+                                  SavePageResult result) {
+  if (client_id.name_space == kBookmarkNamespace) {
+    UMA_HISTOGRAM_ENUMERATION("OfflinePages.SavePageResult.bookmark",

[dewittj, 2016/05/31 22:38:45]

Please open a bug to move this to the diagnostics service when it is available.

+                              static_cast<int>(result),
+                              static_cast<int>(SavePageResult::RESULT_COUNT));
+  } else if (client_id.name_space == kLastNNamespace) {
+    UMA_HISTOGRAM_ENUMERATION("OfflinePages.SavePageResult.last_n",
+                              static_cast<int>(result),
+                              static_cast<int>(SavePageResult::RESULT_COUNT));
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("OfflinePages.SavePageResult",
+                              static_cast<int>(result),
+                              static_cast<int>(SavePageResult::RESULT_COUNT));
+  }
+}
+
+void ReportPageHistogramsAfterDelete(
+    const std::map<int64_t, OfflinePageItem>& offline_pages,
+    const std::vector<int64_t>& deleted_offline_ids) {
+  base::Time now = base::Time::Now();
+  int64_t total_size = 0;
+  for (int64_t offline_id : deleted_offline_ids) {
+    auto iter = offline_pages.find(offline_id);
+    if (iter == offline_pages.end())
+      continue;
+    total_size += iter->second.file_size;
+    ClientId client_id = iter->second.client_id;
+
+    int lifetime_minutes = (now - iter->second.creation_time).InMinutes();
+    int since_last_access_minutes =
+        (now - iter->second.last_access_time).InMinutes();
+    int last_access_to_creation_minutes =
+        (iter->second.last_access_time - iter->second.creation_time).
+            InMinutes();
+    int max_minutes = base::TimeDelta::FromDays(365).InMinutes();
+    int page_size_in_kb = iter->second.file_size / 1024;
+    int account_count = iter->second.access_count;
+
+    if (client_id.name_space == kBookmarkNamespace) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.PageLifetime.bookmark",
+          lifetime_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.DeletePage.TimeSinceLastOpen.bookmark",
+          since_last_access_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.DeletePage.LastOpenToCreated.bookmark",
+          last_access_to_creation_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_MEMORY_KB("OfflinePages.DeletePage.PageSize.bookmark",
+          page_size_in_kb);
+      UMA_HISTOGRAM_COUNTS("OfflinePages.DeletePage.AccessCount.bookmark",
+          account_count);
+    } else if (client_id.name_space == kBookmarkNamespace) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.PageLifetime.last_n",
+          lifetime_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.DeletePage.TimeSinceLastOpen.last_n",
+          since_last_access_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.DeletePage.LastOpenToCreated.last_n",
+          last_access_to_creation_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_MEMORY_KB("OfflinePages.DeletePage.PageSize.last_n",
+          page_size_in_kb);
+      UMA_HISTOGRAM_COUNTS("OfflinePages.DeletePage.AccessCount.last_n",
+          account_count);
+    } else {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.PageLifetime",
+          lifetime_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.DeletePage.TimeSinceLastOpen",
+          since_last_access_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.DeletePage.LastOpenToCreated",
+          last_access_to_creation_minutes, 1, max_minutes, 100);
+      UMA_HISTOGRAM_MEMORY_KB("OfflinePages.DeletePage.PageSize",
+          page_size_in_kb);
+      UMA_HISTOGRAM_COUNTS("OfflinePages.DeletePage.AccessCount",
+          account_count);
+    }
+  }
+
+  if (deleted_offline_ids.size() > 1) {
+    UMA_HISTOGRAM_COUNTS("OfflinePages.BatchDelete.Count",
+                         static_cast<int32_t>(deleted_offline_ids.size()));
+    UMA_HISTOGRAM_MEMORY_KB(
+        "OfflinePages.BatchDelete.TotalPageSize", total_size / 1024);
+  }
+}
+
+void ReportPageHistogramsAfterAccess(const OfflinePageItem& offline_page_item) {
+  int minutes =
+      (base::Time::Now() - offline_page_item.last_access_time).InMinutes();
+  int max = kMaxOpenedPageHistogramBucket.InMinutes();
+  if (offline_page_item.client_id.name_space == kBookmarkNamespace) {
+    // When the access account is still zero, the page is opened for the first
+    // time since its creation.
+    if (offline_page_item.access_count == 0) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.FirstOpenSinceCreated.bookmark", minutes, 1, max, 50);
+    } else {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.OpenSinceLastOpen.bookmark", minutes, 1, max, 50);
+    }
+  } else if (offline_page_item.client_id.name_space == kLastNNamespace) {
+    if (offline_page_item.access_count == 0) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.FirstOpenSinceCreated.last_n", minutes, 1, max, 50);
+    } else {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.OpenSinceLastOpen.last_n", minutes, 1, max, 50);
+    }
+  } else {
+    if (offline_page_item.access_count == 0) {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.FirstOpenSinceCreated", minutes, 1, max, 50);
+    } else {
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          "OfflinePages.OpenSinceLastOpen", minutes, 1, max, 50);
+    }
+  }
+}
+
 }  // namespace
 
 // static
@@ -201,22 +315,9 @@ void OfflinePageModel::MarkPageAccessed(int64_t offline_id) {
   // be updated upon the successful store operation.
   OfflinePageItem offline_page_item = iter->second;
 
-  base::Time now = base::Time::Now();
-  base::TimeDelta time_since_last_accessed =
-      now - offline_page_item.last_access_time;
-
-  // When the access account is still zero, the page is opened for the first
-  // time since its creation.
-  UMA_HISTOGRAM_CUSTOM_COUNTS(
-      AddHistogramSuffix(
-          offline_page_item.client_id,
-          (offline_page_item.access_count == 0) ?
-              "OfflinePages.FirstOpenSinceCreated" :
-              "OfflinePages.OpenSinceLastOpen").c_str(),
-      time_since_last_accessed.InMinutes(), 1,
-      kMaxOpenedPageHistogramBucket.InMinutes(), 50);
-
-  offline_page_item.last_access_time = now;
+  ReportPageHistogramsAfterAccess(offline_page_item);
+
+  offline_page_item.last_access_time = base::Time::Now();
   offline_page_item.access_count++;
 
   store_->AddOrUpdateOfflinePage(
@@ -602,14 +703,25 @@ void OfflinePageModel::OnAddOfflinePageDone(OfflinePageArchiver* archiver,
   if (success) {
     offline_pages_[offline_page.offline_id] = offline_page;
     result = SavePageResult::SUCCESS;
-    UMA_HISTOGRAM_TIMES(
-        AddHistogramSuffix(
-            offline_page.client_id, "OfflinePages.SavePageTime").c_str(),
-        base::Time::Now() - offline_page.creation_time);
-    // 50 buckets capped between 1Kb and 10Mb.
-    UMA_HISTOGRAM_COUNTS_10000(AddHistogramSuffix(
-        offline_page.client_id, "OfflinePages.PageSize").c_str(),
-        offline_page.file_size / 1024);
+    if (offline_page.client_id.name_space == kBookmarkNamespace) {
+      UMA_HISTOGRAM_TIMES("OfflinePages.SavePageTime.bookmark",
+                          base::Time::Now() - offline_page.creation_time);
+      // 50 buckets capped between 1Kb and 10Mb.
+      UMA_HISTOGRAM_COUNTS_10000("OfflinePages.PageSize.bookmark",
+                                 offline_page.file_size / 1024);
+    } else if (offline_page.client_id.name_space == kLastNNamespace) {
+      UMA_HISTOGRAM_TIMES("OfflinePages.SavePageTime.last_n",
+                          base::Time::Now() - offline_page.creation_time);
+      // 50 buckets capped between 1Kb and 10Mb.
+      UMA_HISTOGRAM_COUNTS_10000("OfflinePages.PageSize.last_n",
+                                 offline_page.file_size / 1024);
+    } else {
+      UMA_HISTOGRAM_TIMES("OfflinePages.SavePageTime",
+                          base::Time::Now() - offline_page.creation_time);
+      // 50 buckets capped between 1Kb and 10Mb.
+      UMA_HISTOGRAM_COUNTS_10000("OfflinePages.PageSize",
+                                 offline_page.file_size / 1024);
+    }
   } else {
     result = SavePageResult::STORE_FAILURE;
   }
@@ -672,10 +784,7 @@ void OfflinePageModel::InformSavePageDone(const SavePageCallback& callback,
                                           SavePageResult result,
                                           const ClientId& client_id,
                                           int64_t offline_id) {
-  UMA_HISTOGRAM_ENUMERATION(
-      AddHistogramSuffix(client_id, "OfflinePages.SavePageResult").c_str(),
-      static_cast<int>(result),
-      static_cast<int>(SavePageResult::RESULT_COUNT));
+  ReportPageHistogramAfterSave(client_id, result);
   archive_manager_->GetStorageStats(
       base::Bind(&ReportStorageHistogramsAfterSave));
   callback.Run(result, offline_id);
@@ -705,56 +814,20 @@ void OfflinePageModel::OnRemoveOfflinePagesDone(
     const std::vector<int64_t>& offline_ids,
     const DeletePageCallback& callback,
     bool success) {
+  ReportPageHistogramsAfterDelete(offline_pages_, offline_ids);
+
   // Delete the offline page from the in memory cache regardless of success in
   // store.
-  base::Time now = base::Time::Now();
-  int64_t total_size = 0;
   for (int64_t offline_id : offline_ids) {
     auto iter = offline_pages_.find(offline_id);
     if (iter == offline_pages_.end())
       continue;
-    total_size += iter->second.file_size;
-    ClientId client_id = iter->second.client_id;
-    UMA_HISTOGRAM_CUSTOM_COUNTS(
-        AddHistogramSuffix(client_id, "OfflinePages.PageLifetime").c_str(),
-        (now - iter->second.creation_time).InMinutes(),
-        1,
-        base::TimeDelta::FromDays(365).InMinutes(),
-        100);
-    UMA_HISTOGRAM_CUSTOM_COUNTS(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.TimeSinceLastOpen").c_str(),
-        (now - iter->second.last_access_time).InMinutes(),
-        1,
-        base::TimeDelta::FromDays(365).InMinutes(),
-        100);
-    UMA_HISTOGRAM_CUSTOM_COUNTS(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.LastOpenToCreated").c_str(),
-        (iter->second.last_access_time - iter->second.creation_time).
-            InMinutes(),
-        1,
-        base::TimeDelta::FromDays(365).InMinutes(),
-        100);
-    UMA_HISTOGRAM_MEMORY_KB(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.PageSize").c_str(),
-        iter->second.file_size / 1024);
-    UMA_HISTOGRAM_COUNTS(
-        AddHistogramSuffix(
-            client_id, "OfflinePages.DeletePage.AccessCount").c_str(),
-        iter->second.access_count);
     FOR_EACH_OBSERVER(
         Observer, observers_,
         OfflinePageDeleted(iter->second.offline_id, iter->second.client_id));
     offline_pages_.erase(iter);
   }
-  if (offline_ids.size() > 1) {
-    UMA_HISTOGRAM_COUNTS("OfflinePages.BatchDelete.Count",
-                         static_cast<int32_t>(offline_ids.size()));
-    UMA_HISTOGRAM_MEMORY_KB(
-        "OfflinePages.BatchDelete.TotalPageSize", total_size / 1024);
-  }
+
   // Deleting multiple pages always succeeds when it gets to this point.
   InformDeletePageDone(callback, (success || offline_ids.size() > 1)
                                      ? DeletePageResult::SUCCESS