Add some UMA for recent tabs

chrome/browser/android/offline_pages/recent_tab_helper.cc

 // Copyright (c) 2016 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 "chrome/browser/android/offline_pages/recent_tab_helper.h"
 
 #include <queue>
 #include <vector>
 
 #include "base/bind.h"
 #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"
 #include "chrome/browser/android/offline_pages/offline_page_model_factory.h"
 #include "components/offline_pages/offline_page_item.h"
 #include "components/offline_pages/offline_page_model.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
@@ skipping 61 matching lines @@
 // - delete the pages found in the previous step
 // - snapshot the current web contents
 // Along the chain, the original URL is passed and compared, to detect
 // possible navigation and cancel snapshot in that case.
 void RecentTabHelper::StartSnapshot() {
   if (never_do_snapshots_)
     return;
 
   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;
   }
 
   snapshot_url_ = url;
 
   // TODO(dimich): Implement automatic cleanup as per design doc, based on
   // storage limits and page age.
   // This algorithm (remove pages before making sure the save was successful)
   // prefers keeping the storage use low and under control by potentially
   // sacrificing the current snapshot.
@@ skipping 67 matching lines @@
   }
 
   page_model_->SavePage(
       snapshot_url_, client_id(), std::move(archiver),
       base::Bind(&RecentTabHelper::SavePageCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 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();
 }
 
 void RecentTabHelper::ReportSnapshotCompleted() {
   snapshot_controller_->PendingSnapshotCompleted();
 }
 
 bool RecentTabHelper::IsSamePage() const {
   return web_contents() &&
          (web_contents()->GetLastCommittedURL() == snapshot_url_);
 }
 
 ClientId RecentTabHelper::client_id() const {
   return ClientId(kClientNamespace, "");
 }
 
 void RecentTabHelper::SetArchiveFactoryForTest(
     std::unique_ptr<TestArchiveFactory> test_archive_factory) {
   test_archive_factory_ = std::move(test_archive_factory);
 }
 
 void RecentTabHelper::SetTaskRunnerForTest(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   snapshot_controller_.reset(new SnapshotController(task_runner, this));
 }
 
 }  // namespace offline_pages