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

Side by Side Diff: components/offline_pages/offline_page_model_impl.cc

Issue 2314273003: [Offline Pages] Adds duplicate detection metrics. (Closed)
Patch Set: rebase 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 unified diff | Download patch
« no previous file with comments | « no previous file | components/offline_pages/offline_page_model_impl_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2016 The Chromium Authors. All rights reserved. 1 // Copyright 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/offline_pages/offline_page_model_impl.h" 5 #include "components/offline_pages/offline_page_model_impl.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <limits> 8 #include <limits>
9 9
10 #include "base/bind.h" 10 #include "base/bind.h"
(...skipping 133 matching lines...) Expand 10 before | Expand all | Expand 10 after
144 // Note: The factory creates and owns the histogram. 144 // Note: The factory creates and owns the histogram.
145 base::HistogramBase* histogram = base::LinearHistogram::FactoryGet( 145 base::HistogramBase* histogram = base::LinearHistogram::FactoryGet(
146 AddHistogramSuffix(client_id, "OfflinePages.SavePageResult"), 146 AddHistogramSuffix(client_id, "OfflinePages.SavePageResult"),
147 1, 147 1,
148 static_cast<int>(SavePageResult::RESULT_COUNT), 148 static_cast<int>(SavePageResult::RESULT_COUNT),
149 static_cast<int>(SavePageResult::RESULT_COUNT) + 1, 149 static_cast<int>(SavePageResult::RESULT_COUNT) + 1,
150 base::HistogramBase::kUmaTargetedHistogramFlag); 150 base::HistogramBase::kUmaTargetedHistogramFlag);
151 histogram->Add(static_cast<int>(result)); 151 histogram->Add(static_cast<int>(result));
152 } 152 }
153 153
154 void ReportPageHistogramAfterSave(const OfflinePageItem& offline_page, 154 // Goes through the list of offline pages, compiling the following two metrics:
155 const base::Time& save_time) { 155 // - a count of the pages with the same URL
156 // - The difference between the |created_before| time and the creation time of
157 // the page with the closest creation time before |created_before|.
158 // Returns true if there was a page that was saved before |created_before| with
159 // a matching URL.
160 bool GetMatchingURLCountAndMostRecentCreationTime(
161 const std::map<int64_t, OfflinePageItem>& offline_pages,
162 std::string name_space,
163 const GURL& url,
164 base::Time created_before,
165 int* matching_url_count,
166 base::TimeDelta* most_recent_creation_time) {
167 int count = 0;
168
169 // Create a time that is very old, so that any valid time will be newer than
170 // it.
171 base::Time latest_time;
172 bool matching_page = false;
173
174 for (auto& id_page_pair : offline_pages) {
175 if (id_page_pair.second.client_id.name_space == name_space &&
176 url == id_page_pair.second.url) {
177 count++;
178 base::Time page_creation_time = id_page_pair.second.creation_time;
179 if (page_creation_time < created_before &&
180 page_creation_time > latest_time) {
181 latest_time = page_creation_time;
182 matching_page = true;
183 }
184 }
185 }
186
187 if (matching_url_count != nullptr)
188 *matching_url_count = count;
189 if (most_recent_creation_time != nullptr && latest_time != base::Time())
190 *most_recent_creation_time = created_before - latest_time;
191
192 return matching_page;
193 }
194
195 void ReportPageHistogramAfterSave(
196 const std::map<int64_t, OfflinePageItem>& offline_pages,
197 const OfflinePageItem& offline_page,
198 const base::Time& save_time) {
156 // The histogram below is an expansion of the UMA_HISTOGRAM_TIMES 199 // The histogram below is an expansion of the UMA_HISTOGRAM_TIMES
157 // macro adapted to allow for a dynamically suffixed histogram name. 200 // macro adapted to allow for a dynamically suffixed histogram name.
158 // Note: The factory creates and owns the histogram. 201 // Note: The factory creates and owns the histogram.
159 base::HistogramBase* histogram = base::Histogram::FactoryTimeGet( 202 base::HistogramBase* histogram = base::Histogram::FactoryTimeGet(
160 AddHistogramSuffix(offline_page.client_id, "OfflinePages.SavePageTime"), 203 AddHistogramSuffix(offline_page.client_id, "OfflinePages.SavePageTime"),
161 base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromSeconds(10), 204 base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromSeconds(10),
162 50, base::HistogramBase::kUmaTargetedHistogramFlag); 205 50, base::HistogramBase::kUmaTargetedHistogramFlag);
163 histogram->AddTime(save_time - offline_page.creation_time); 206 histogram->AddTime(save_time - offline_page.creation_time);
164 207
165 // The histogram below is an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS 208 // The histogram below is an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS
166 // macro adapted to allow for a dynamically suffixed histogram name. 209 // macro adapted to allow for a dynamically suffixed histogram name.
167 // Note: The factory creates and owns the histogram. 210 // Note: The factory creates and owns the histogram.
168 // Reported as Kb between 1Kb and 10Mb. 211 // Reported as Kb between 1Kb and 10Mb.
169 histogram = base::Histogram::FactoryGet( 212 histogram = base::Histogram::FactoryGet(
170 AddHistogramSuffix(offline_page.client_id, "OfflinePages.PageSize"), 213 AddHistogramSuffix(offline_page.client_id, "OfflinePages.PageSize"),
171 1, 10000, 50, base::HistogramBase::kUmaTargetedHistogramFlag); 214 1, 10000, 50, base::HistogramBase::kUmaTargetedHistogramFlag);
172 histogram->Add(offline_page.file_size / 1024); 215 histogram->Add(offline_page.file_size / 1024);
216
217 if (offline_page.client_id.name_space == kDownloadNamespace) {
218 int matching_url_count;
219 base::TimeDelta time_since_most_recent_duplicate;
220 if (GetMatchingURLCountAndMostRecentCreationTime(
221 offline_pages, offline_page.client_id.name_space, offline_page.url,
222 offline_page.creation_time, &matching_url_count,
223 &time_since_most_recent_duplicate)) {
224 // Using CUSTOM_COUNTS instead of time-oriented histogram to record
225 // samples in seconds rather than milliseconds.
226 UMA_HISTOGRAM_CUSTOM_COUNTS(
227 "OfflinePages.DownloadSavedPageTimeSinceDuplicateSaved",
228 time_since_most_recent_duplicate.InSeconds(),
229 base::TimeDelta::FromSeconds(1).InSeconds(),
230 base::TimeDelta::FromDays(7).InSeconds(), 50);
231 }
232 UMA_HISTOGRAM_CUSTOM_COUNTS("OfflinePages.DownloadSavedPageDuplicateCount",
233 matching_url_count, 1, 20, 10);
234 }
173 } 235 }
174 236
175 void ReportPageHistogramsAfterDelete( 237 void ReportPageHistogramsAfterDelete(
176 const std::map<int64_t, OfflinePageItem>& offline_pages, 238 const std::map<int64_t, OfflinePageItem>& offline_pages,
177 const std::vector<int64_t>& deleted_offline_ids, 239 const std::vector<int64_t>& deleted_offline_ids,
178 const base::Time& delete_time) { 240 const base::Time& delete_time) {
179 const int max_minutes = base::TimeDelta::FromDays(365).InMinutes(); 241 const int max_minutes = base::TimeDelta::FromDays(365).InMinutes();
180 int64_t total_size = 0; 242 int64_t total_size = 0;
181 for (int64_t offline_id : deleted_offline_ids) { 243 for (int64_t offline_id : deleted_offline_ids) {
182 auto iter = offline_pages.find(offline_id); 244 auto iter = offline_pages.find(offline_id);
183 if (iter == offline_pages.end()) 245 if (iter == offline_pages.end())
184 continue; 246 continue;
247
185 total_size += iter->second.file_size; 248 total_size += iter->second.file_size;
186 ClientId client_id = iter->second.client_id; 249 ClientId client_id = iter->second.client_id;
187 250
251 if (client_id.name_space == kDownloadNamespace) {
252 int remaining_pages_with_url;
253 GetMatchingURLCountAndMostRecentCreationTime(
254 offline_pages, iter->second.client_id.name_space, iter->second.url,
255 base::Time::Max(), &remaining_pages_with_url, nullptr);
256 UMA_HISTOGRAM_CUSTOM_COUNTS(
257 "OfflinePages.DownloadDeletedPageDuplicateCount",
258 remaining_pages_with_url, 1, 20, 10);
259 }
260
188 // The histograms below are an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS 261 // The histograms below are an expansion of the UMA_HISTOGRAM_CUSTOM_COUNTS
189 // macro adapted to allow for a dynamically suffixed histogram name. 262 // macro adapted to allow for a dynamically suffixed histogram name.
190 // Note: The factory creates and owns the histogram. 263 // Note: The factory creates and owns the histogram.
191 base::HistogramBase* histogram = base::Histogram::FactoryGet( 264 base::HistogramBase* histogram = base::Histogram::FactoryGet(
192 AddHistogramSuffix(client_id, "OfflinePages.PageLifetime"), 265 AddHistogramSuffix(client_id, "OfflinePages.PageLifetime"),
193 1, max_minutes, 100, base::HistogramBase::kUmaTargetedHistogramFlag); 266 1, max_minutes, 100, base::HistogramBase::kUmaTargetedHistogramFlag);
194 histogram->Add((delete_time - iter->second.creation_time).InMinutes()); 267 histogram->Add((delete_time - iter->second.creation_time).InMinutes());
195 268
196 histogram = base::Histogram::FactoryGet( 269 histogram = base::Histogram::FactoryGet(
197 AddHistogramSuffix( 270 AddHistogramSuffix(
(...skipping 535 matching lines...) Expand 10 before | Expand all | Expand 10 after
733 806
734 void OfflinePageModelImpl::OnAddOfflinePageDone( 807 void OfflinePageModelImpl::OnAddOfflinePageDone(
735 OfflinePageArchiver* archiver, 808 OfflinePageArchiver* archiver,
736 const SavePageCallback& callback, 809 const SavePageCallback& callback,
737 const OfflinePageItem& offline_page, 810 const OfflinePageItem& offline_page,
738 bool success) { 811 bool success) {
739 SavePageResult result; 812 SavePageResult result;
740 if (success) { 813 if (success) {
741 offline_pages_[offline_page.offline_id] = offline_page; 814 offline_pages_[offline_page.offline_id] = offline_page;
742 result = SavePageResult::SUCCESS; 815 result = SavePageResult::SUCCESS;
743 ReportPageHistogramAfterSave(offline_page, GetCurrentTime()); 816 ReportPageHistogramAfterSave(offline_pages_, offline_page,
817 GetCurrentTime());
744 offline_event_logger_.RecordPageSaved( 818 offline_event_logger_.RecordPageSaved(
745 offline_page.client_id.name_space, offline_page.url.spec(), 819 offline_page.client_id.name_space, offline_page.url.spec(),
746 std::to_string(offline_page.offline_id)); 820 std::to_string(offline_page.offline_id));
747 } else { 821 } else {
748 result = SavePageResult::STORE_FAILURE; 822 result = SavePageResult::STORE_FAILURE;
749 } 823 }
750 InformSavePageDone(callback, result, offline_page.client_id, 824 InformSavePageDone(callback, result, offline_page.client_id,
751 offline_page.offline_id); 825 offline_page.offline_id);
752 if (result == SavePageResult::SUCCESS) { 826 if (result == SavePageResult::SUCCESS) {
753 DeleteExistingPagesWithSameURL(offline_page); 827 DeleteExistingPagesWithSameURL(offline_page);
(...skipping 327 matching lines...) Expand 10 before | Expand all | Expand 10 after
1081 } 1155 }
1082 1156
1083 base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task); 1157 base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
1084 } 1158 }
1085 1159
1086 base::Time OfflinePageModelImpl::GetCurrentTime() const { 1160 base::Time OfflinePageModelImpl::GetCurrentTime() const {
1087 return testing_clock_ ? testing_clock_->Now() : base::Time::Now(); 1161 return testing_clock_ ? testing_clock_->Now() : base::Time::Now();
1088 } 1162 }
1089 1163
1090 } // namespace offline_pages 1164 } // namespace offline_pages
OLDNEW
« no previous file with comments | « no previous file | components/offline_pages/offline_page_model_impl_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698