[Offline Pages] Linking storage manager and model with UMAs.

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 678552d88933c47df06cbfe4ec617b1e2e23604b..5656abddc42bf9c866f63da387e79dcc50178bb9 100644
--- a/components/offline_pages/offline_page_model.cc
+++ b/components/offline_pages/offline_page_model.cc
@@ -534,8 +534,21 @@ void OfflinePageModel::OnExpirePageDone(int64_t offline_id,
   // TODO(romax): Report UMA about successful expiration.

[jianli, 2016/05/26 20:37:52]

Do you also want to address this comment by adding a UMA?

[romax, 2016/05/27 02:01:37]

yes, but it would come with the model-storage manager hookup, I think I'll just
do it in this CL.

   if (success) {
     auto iter = offline_pages_.find(offline_id);
-    if (iter != offline_pages_.end())
+    if (iter != offline_pages_.end()) {

[jianli, 2016/05/26 20:37:52]

nit: simpler to do early return for non-match case to avoid extra nesting.

[romax, 2016/05/27 02:01:37]

Done.

       iter->second.expiration_time = expiration_time;
+      ClientId client_id = iter->second.client_id;
+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          AddHistogramSuffix(client_id, "OfflinePages.ExpirePage.PageLifetime")
+              .c_str(),
+          (expiration_time - iter->second.creation_time).InMinutes(), 1,
+          base::TimeDelta::FromDays(60).InMinutes(), 10);

[jianli, 2016/05/26 20:37:52]

Are 10 buckets enough?

[romax, 2016/05/27 02:01:37]

I misunderstood this argument, changing to 120.

+      UMA_HISTOGRAM_CUSTOM_COUNTS(
+          AddHistogramSuffix(client_id,
+                             "OfflinePages.ExpirePage.TimeSinceLastAccess")
+              .c_str(),
+          (expiration_time - iter->second.last_access_time).InMinutes(), 1,
+          base::TimeDelta::FromDays(60).InMinutes(), 10);
+    }
   }
 }