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

components/offline_pages/offline_page_model.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/offline_pages/offline_page_model.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 513 matching lines @@
         offline_page, base::Bind(&OfflinePageModel::OnExpirePageDone,
                                  weak_ptr_factory_.GetWeakPtr(), offline_id,
                                  expiration_time));
   }
   archive_manager_->DeleteMultipleArchives(paths_to_delete, callback);
 }
 
 void OfflinePageModel::OnExpirePageDone(int64_t offline_id,
                                         const base::Time& expiration_time,
                                         bool success) {
   // 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);
+    }
   }
 }
 
 ClientPolicyController* OfflinePageModel::GetPolicyController() {
   return policy_controller_.get();
 }
 
 OfflinePageMetadataStore* OfflinePageModel::GetStoreForTesting() {
   return store_.get();
 }
@@ skipping 309 matching lines @@
 void OfflinePageModel::RunWhenLoaded(const base::Closure& task) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(task);
     return;
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
 }
 
 }  // namespace offline_pages