Add some UMA for recent tabs

chrome/browser/android/offline_pages/recent_tab_helper.cc

Index: chrome/browser/android/offline_pages/recent_tab_helper.cc
diff --git a/chrome/browser/android/offline_pages/recent_tab_helper.cc b/chrome/browser/android/offline_pages/recent_tab_helper.cc
index 6f5fc9b3aa3fa722ad8772020c4f17e94b469871..97d8ce1d0467ffabe05283a7109a3fbe700c9fe9 100644
--- a/chrome/browser/android/offline_pages/recent_tab_helper.cc
+++ b/chrome/browser/android/offline_pages/recent_tab_helper.cc
@@ -11,6 +11,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h"
@@ -92,7 +93,7 @@ void RecentTabHelper::StartSnapshot() {
 
   GURL url = web_contents()->GetLastCommittedURL();
   if (!page_model_->CanSavePage(url)) {
-    // TODO(dimich): Add UMA. Bug 608112.
+    UMA_HISTOGRAM_COUNTS("OfflinePages.CanNotSaveRecentPage", 1);

[dewittj, 2016/05/10 23:07:56]

I wonder if we should have an enumerated histogram that shows all the various
ways that recent tab helper fails:
- !CanSavePage
- DeletePageResult is not SUCCESS or NOT_FOUND
- !IsSamePage

[bburns, 2016/05/11 16:21:52]

I don't feel strongly.  Is this detailed in a PRD somewhere? I guess if I were
chosing, I would have different histograms based on the function call rather
than one with multiple failure modes, but I don't super care.

[dewittj, 2016/05/11 16:31:58]

nope, not in the PRD.  I don't super care about the shape either, but it seems
easier to compare the frequency of all types if we record them all together.

Either way, this metric is not correctly implemented. UMA_HISTOGRAM_COUNTS
defines a histogram with 50 buckets ranging from 1 to 1e6, and this always
records the value '1'.  So either you want
UMA_HISTOGRAM_BOOLEAN("OfflinePages.CanSaveRecentPage",
page_model_->CanSavePage(url))

or UMA_HISTOGRAM_ENUMERATION with all the different problems as I commented
before.

[fgorski, 2016/05/11 16:54:23]

Boolean sounds good for this one, as this is a proactive check and isn't a
result of a failure to save.

     return;
   }
 
@@ -180,7 +181,9 @@ void RecentTabHelper::ContinueSnapshotAfterPurge(
 
 void RecentTabHelper::SavePageCallback(OfflinePageModel::SavePageResult result,
                                        int64_t offline_id) {
-  // TODO(dimich): add UMA, including result. Bug 608112.
+  UMA_HISTOGRAM_ENUMERATION(
+      "OfflinePages.SavePageResult.RecentPage", static_cast<int>(result),

[fgorski, 2016/05/11 16:54:23]

I don't think this is needed in this shape and form.
Please look for email from Jian about adding suffixes.

This UMA will be reported in the model, with the right suffix, based on the
ClientID.namespace.

+      static_cast<int>(OfflinePageModel::SavePageResult::RESULT_COUNT));
   ReportSnapshotCompleted();
 }