Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(163)

Unified Diff: components/offline_pages/offline_page_model_impl.cc

Issue 2314273003: [Offline Pages] Adds duplicate detection metrics. (Closed)
Patch Set: Fix nits and comments. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/offline_pages/offline_page_model_impl.cc
diff --git a/components/offline_pages/offline_page_model_impl.cc b/components/offline_pages/offline_page_model_impl.cc
index 25f2e08a68de91249597a5a83ee048cdcde3bb42..fee523016abe2d12250a06e413becca57781cbeb 100644
--- a/components/offline_pages/offline_page_model_impl.cc
+++ b/components/offline_pages/offline_page_model_impl.cc
@@ -150,7 +150,50 @@ void ReportSavePageResultHistogramAfterSave(const ClientId& client_id,
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.
@@ -168,6 +211,22 @@ void ReportPageHistogramAfterSave(const OfflinePageItem& offline_page) {
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(
+ "OfflinePages.DownloadSavedPageTimeSinceDuplicateSaved",
+ time_since_most_recent_duplicate, base::TimeDelta::FromSeconds(1),
+ base::TimeDelta::FromDays(7), 50);
+ }
+ UMA_HISTOGRAM_CUSTOM_COUNTS("OfflinePages.DownloadSavedPageDuplicateCount",
+ matching_url_count, 1, 20, 10);
+ }
}
void ReportPageHistogramsAfterDelete(
@@ -180,9 +239,20 @@ void ReportPageHistogramsAfterDelete(
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_CUSTOM_COUNTS(
+ "OfflinePages.DownloadDeletedPageDuplicateCount",
+ remaining_pages_with_url, 1, 20, 10);
+ }
+
// 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.
@@ -736,7 +806,7 @@ void OfflinePageModelImpl::OnAddOfflinePageDone(
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));
« no previous file with comments | « no previous file | components/offline_pages/offline_page_model_impl_unittest.cc » ('j') | tools/metrics/histograms/histograms.xml » ('J')

Powered by Google App Engine
This is Rietveld 408576698