[Offline Pages] Adds duplicate detection metrics.

components/offline_pages/offline_page_model_impl.cc

 // Copyright 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 "components/offline_pages/offline_page_model_impl.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
@@ skipping 132 matching lines @@
   // Note: The factory creates and owns the histogram.
   base::HistogramBase* histogram = base::LinearHistogram::FactoryGet(
       AddHistogramSuffix(client_id, "OfflinePages.SavePageResult"),
       1,
       static_cast<int>(SavePageResult::RESULT_COUNT),
       static_cast<int>(SavePageResult::RESULT_COUNT) + 1,
       base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(static_cast<int>(result));
 }
 
-void ReportPageHistogramAfterSave(const OfflinePageItem& offline_page) {
+// Goes through the list of offline pages, compiling the following two metrics:
+//   - a count of the pages with the same URL
+//   - The difference between the |created_before| time and the creation time of
+//     the page with the closest creation time before |created_before|.
+// Returns true if there was a page that was saved before |created_before| with
+// a matching URL.
+bool GetMatchingURLCountAndMostRecentCreationTime(
+    const std::map<int64_t, OfflinePageItem>& offline_pages,
+    std::string name_space,
+    const GURL& url,
+    base::Time created_before,
+    int* matching_url_count,
+    base::TimeDelta* most_recent_creation_time) {
+  int count = 0;
+
+  // Create a time that is very old, so that any valid time will be newer than
+  // it.
+  base::Time latest_time;
+  bool matching_page = false;
+
+  for (auto& id_page_pair : offline_pages) {
+    if (id_page_pair.second.client_id.name_space == name_space &&
+        url == id_page_pair.second.url) {
+      count++;
+      base::Time page_creation_time = id_page_pair.second.creation_time;
+      if (page_creation_time < created_before &&
+          page_creation_time > latest_time) {
+        latest_time = page_creation_time;
+        matching_page = true;
+      }
+    }
+  }
+
+  if (matching_url_count != nullptr)
+    *matching_url_count = count;
+  if (most_recent_creation_time != nullptr && latest_time != base::Time())
+    *most_recent_creation_time = created_before - latest_time;
+
+  return matching_page;
+}
+
+void ReportPageHistogramAfterSave(
+    const std::map<int64_t, OfflinePageItem>& offline_pages,
+    const OfflinePageItem& offline_page) {
   // The histogram below is an expansion of the UMA_HISTOGRAM_TIMES
   // macro adapted to allow for a dynamically suffixed histogram name.
   // Note: The factory creates and owns the histogram.
   base::HistogramBase* histogram = base::Histogram::FactoryTimeGet(
       AddHistogramSuffix(offline_page.client_id, "OfflinePages.SavePageTime"),
       base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromSeconds(10),
       50, base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->AddTime(base::Time::Now() - offline_page.creation_time);
 
   // The histogram below is an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS
   // macro adapted to allow for a dynamically suffixed histogram name.
   // Note: The factory creates and owns the histogram.
   // Reported as Kb between 1Kb and 10Mb.
   histogram = base::Histogram::FactoryGet(
       AddHistogramSuffix(offline_page.client_id, "OfflinePages.PageSize"),
       1, 10000, 50, base::HistogramBase::kUmaTargetedHistogramFlag);
   histogram->Add(offline_page.file_size / 1024);
+
+  if (offline_page.client_id.name_space == kDownloadNamespace) {
+    int matching_url_count;
+    base::TimeDelta time_since_most_recent_duplicate;
+    if (GetMatchingURLCountAndMostRecentCreationTime(
+            offline_pages, offline_page.client_id.name_space, offline_page.url,
+            offline_page.creation_time, &matching_url_count,
+            &time_since_most_recent_duplicate)) {
+      UMA_HISTOGRAM_CUSTOM_TIMES(

[Mark P, 2016/09/09 18:41:05]

Do you really care about millisecond accuracy?
You may want to consider making the minimum bucket one second.

Also, please play around with about:histograms with various delays to
investigate the bucketing.  I'm surprised you need 200 buckets.

[dewittj, 2016/09/12 18:22:50]

I wanted reasonable resolution within a few minutes despite using millisecond
accuracy.  I guess our actions would be the same if there were too many within 1
second or too many within 700 ms so second accuracy is fine.  Then I can reduce
the bucket count as well.

+          "OfflinePages.DownloadSavedPageTimeSinceDuplicateSaved",
+          time_since_most_recent_duplicate,
+          base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromDays(7),
+          200);
+    }
+    UMA_HISTOGRAM_COUNTS_100("OfflinePages.DownloadSavedPageDuplicateCount",

[Mark P, 2016/09/09 18:41:04]

Do you really think that users will have up to a 100 copies of a page?  I wonder
if something like a max of 50 with fewer buckets (say, 20) is more appropriate.

[dewittj, 2016/09/12 18:22:50]

Done.

+                             matching_url_count);
+  }
 }
 
 void ReportPageHistogramsAfterDelete(
     const std::map<int64_t, OfflinePageItem>& offline_pages,
     const std::vector<int64_t>& deleted_offline_ids) {
   const int max_minutes = base::TimeDelta::FromDays(365).InMinutes();
   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;
 
+    if (client_id.name_space == kDownloadNamespace) {
+      int remaining_pages_with_url;
+      GetMatchingURLCountAndMostRecentCreationTime(
+          offline_pages, iter->second.client_id.name_space, iter->second.url,
+          base::Time::Max(), &remaining_pages_with_url, nullptr);
+      UMA_HISTOGRAM_COUNTS_100("OfflinePages.DownloadDeletedPageDuplicateCount",
+                               remaining_pages_with_url);
+    }
+
     // The histograms below are an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS
     // macro adapted to allow for a dynamically suffixed histogram name.
     // Note: The factory creates and owns the histogram.
     base::HistogramBase* histogram = base::Histogram::FactoryGet(
         AddHistogramSuffix(client_id, "OfflinePages.PageLifetime"),
         1, max_minutes, 100, base::HistogramBase::kUmaTargetedHistogramFlag);
     histogram->Add((now - iter->second.creation_time).InMinutes());
 
     histogram = base::Histogram::FactoryGet(
         AddHistogramSuffix(
@@ skipping 533 matching lines @@
 
 void OfflinePageModelImpl::OnAddOfflinePageDone(
     OfflinePageArchiver* archiver,
     const SavePageCallback& callback,
     const OfflinePageItem& offline_page,
     bool success) {
   SavePageResult result;
   if (success) {
     offline_pages_[offline_page.offline_id] = offline_page;
     result = SavePageResult::SUCCESS;
-    ReportPageHistogramAfterSave(offline_page);
+    ReportPageHistogramAfterSave(offline_pages_, offline_page);
     offline_event_logger_.RecordPageSaved(
         offline_page.client_id.name_space, offline_page.url.spec(),
         std::to_string(offline_page.offline_id));
   } else {
     result = SavePageResult::STORE_FAILURE;
   }
   InformSavePageDone(callback, result, offline_page.client_id,
                      offline_page.offline_id);
   if (result == SavePageResult::SUCCESS) {
     DeleteExistingPagesWithSameURL(offline_page);
@@ skipping 320 matching lines @@
 void OfflinePageModelImpl::RunWhenLoaded(const base::Closure& task) {
   if (!is_loaded_) {
     delayed_tasks_.push_back(task);
     return;
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
 }
 
 }  // namespace offline_pages