Index: chrome/browser/safe_browsing/safe_browsing_database.cc |
diff --git a/chrome/browser/safe_browsing/safe_browsing_database.cc b/chrome/browser/safe_browsing/safe_browsing_database.cc |
index 0f11795edb63e2dafcbbedf451ae1771a38d25a5..6359b7037d41b6b3d8a1acc6ad7aa9b09115bb74 100644 |
--- a/chrome/browser/safe_browsing/safe_browsing_database.cc |
+++ b/chrome/browser/safe_browsing/safe_browsing_database.cc |
@@ -1329,7 +1329,7 @@ void SafeBrowsingDatabaseNew::UpdateFinished(bool update_succeeded) { |
DownloadDBFilename(filename_base_), |
download_store_.get(), |
FAILURE_DOWNLOAD_DATABASE_UPDATE_FINISH); |
- UMA_HISTOGRAM_COUNTS("SB2.DownloadDatabaseKilobytes", |
+ UMA_HISTOGRAM_COUNTS("SB2.DatabaseKilobytes.Download", |
static_cast<int>(size_bytes / 1024)); |
} |
@@ -1352,7 +1352,7 @@ void SafeBrowsingDatabaseNew::UpdateFinished(bool update_succeeded) { |
ExtensionBlacklistDBFilename(filename_base_), |
extension_blacklist_store_.get(), |
FAILURE_EXTENSION_BLACKLIST_UPDATE_FINISH); |
- UMA_HISTOGRAM_COUNTS("SB2.ExtensionBlacklistKilobytes", |
+ UMA_HISTOGRAM_COUNTS("SB2.DatabaseKilobytes.ExtensionBlacklist", |
static_cast<int>(size_bytes / 1024)); |
} |
@@ -1397,6 +1397,10 @@ void SafeBrowsingDatabaseNew::UpdateWhitelistStore( |
return; |
} |
+ const int64 file_size = GetFileSizeOrZero(store_filename); |
+ RecordSBWhitelistCountsHistogram("SB2.DatabaseKilobytes", whitelist_id, |
+ static_cast<int>(file_size / 1024)); |
+ |
#if defined(OS_MACOSX) |
base::mac::SetFileBackupExclusion(store_filename); |
#endif |
@@ -1484,15 +1488,7 @@ void SafeBrowsingDatabaseNew::UpdatePrefixSetUrlStore( |
UMA_HISTOGRAM_LONG_TIMES("SB2.BuildFilter", base::TimeTicks::Now() - before); |
- { |
- // Persist the prefix set to disk. Do not grab the lock to avoid contention |
- // while writing to disk. This is safe as only this thread can ever modify |
- // |state_manager_|'s prefix sets anyways. |
- scoped_ptr<ReadTransaction> txn = |
- state_manager_.BeginReadTransactionNoLockOnMainThread(); |
- WritePrefixSet(db_filename, txn->GetPrefixSet(prefix_set_id), |
- write_failure_type); |
- } |
+ WritePrefixSet(db_filename, prefix_set_id, write_failure_type); |
// Gather statistics. |
if (got_counters && metric->GetIOCounters(&io_after)) { |
@@ -1511,8 +1507,8 @@ void SafeBrowsingDatabaseNew::UpdatePrefixSetUrlStore( |
} |
const int64 file_size = GetFileSizeOrZero(db_filename); |
- UMA_HISTOGRAM_COUNTS("SB2.DatabaseKilobytes", |
- static_cast<int>(file_size / 1024)); |
+ RecordPrefixSetCountsHistogram("SB2.DatabaseKilobytes", prefix_set_id, |
+ static_cast<int>(file_size / 1024)); |
Scott Hess - ex-Googler
2014/12/18 23:57:55
I find the prefix-set references misleading, here.
gab
2014/12/19 15:25:54
Right but I was trying to avoid introducing yet an
|
#if defined(OS_MACOSX) |
base::mac::SetFileBackupExclusion(db_filename); |
@@ -1532,8 +1528,15 @@ void SafeBrowsingDatabaseNew::UpdateIpBlacklistStore() { |
return; |
} |
+ const base::FilePath ip_blacklist_filename = |
+ IpBlacklistDBFilename(filename_base_); |
+ |
+ const int64 file_size = GetFileSizeOrZero(ip_blacklist_filename); |
+ UMA_HISTOGRAM_COUNTS("SB2.DatabaseKilobytes.IPBlacklist", |
+ static_cast<int>(file_size / 1024)); |
+ |
#if defined(OS_MACOSX) |
- base::mac::SetFileBackupExclusion(IpBlacklistDBFilename(filename_base_)); |
+ base::mac::SetFileBackupExclusion(ip_blacklist_filename); |
#endif |
LoadIpBlacklist(full_hashes); |
@@ -1676,10 +1679,17 @@ bool SafeBrowsingDatabaseNew::Delete() { |
} |
void SafeBrowsingDatabaseNew::WritePrefixSet(const base::FilePath& db_filename, |
- const PrefixSet* prefix_set, |
+ PrefixSetId prefix_set_id, |
FailureType write_failure_type) { |
DCHECK(thread_checker_.CalledOnValidThread()); |
+ // Do not grab the lock to avoid contention while writing to disk. This is |
+ // safe as only this thread can ever modify |state_manager_|'s prefix sets |
+ // anyways. |
+ scoped_ptr<ReadTransaction> txn = |
+ state_manager_.BeginReadTransactionNoLockOnMainThread(); |
+ const PrefixSet* prefix_set = txn->GetPrefixSet(prefix_set_id); |
+ |
if (!prefix_set) |
return; |
@@ -1690,8 +1700,8 @@ void SafeBrowsingDatabaseNew::WritePrefixSet(const base::FilePath& db_filename, |
UMA_HISTOGRAM_TIMES("SB2.PrefixSetWrite", base::TimeTicks::Now() - before); |
const int64 file_size = GetFileSizeOrZero(prefix_set_filename); |
- UMA_HISTOGRAM_COUNTS("SB2.PrefixSetKilobytes", |
- static_cast<int>(file_size / 1024)); |
+ RecordPrefixSetCountsHistogram("SB2.PrefixSetKilobytes", prefix_set_id, |
+ static_cast<int>(file_size / 1024)); |
if (!write_ok) |
RecordFailure(write_failure_type); |
@@ -1798,3 +1808,50 @@ SafeBrowsingDatabaseNew::GetUnsynchronizedPrefixGetHashCacheForTesting() { |
scoped_ptr<ReadTransaction> txn = state_manager_.BeginReadTransaction(); |
return txn->prefix_gethash_cache(); |
} |
+ |
+void SafeBrowsingDatabaseNew::RecordPrefixSetCountsHistogram( |
+ const std::string& histogram_name_base, |
+ PrefixSetId prefix_set_id, |
+ int sample) { |
+ std::string histogram_name_full(histogram_name_base); |
+ switch (prefix_set_id) { |
+ case PrefixSetId::BROWSE: |
+ histogram_name_full.append(".Browse"); |
+ break; |
+ case PrefixSetId::SIDE_EFFECT_FREE_WHITELIST: |
+ histogram_name_full.append(".SideEffectFreeWhitelist"); |
+ break; |
+ case PrefixSetId::UNWANTED_SOFTWARE: |
+ histogram_name_full.append(".UnwantedSoftware"); |
+ break; |
+ } |
+ |
+ // Histogram properties as in UMA_HISTOGRAM_COUNTS macro. |
+ base::HistogramBase* histogram_pointer = base::Histogram::FactoryGet( |
+ histogram_name_full, 1, 1000000, 50, |
+ base::HistogramBase::kUmaTargetedHistogramFlag); |
+ |
+ histogram_pointer->Add(sample); |
+} |
+ |
+void SafeBrowsingDatabaseNew::RecordSBWhitelistCountsHistogram( |
+ const std::string& histogram_name_base, |
+ SBWhitelistId whitelist_id, |
+ int sample) { |
+ std::string histogram_name_full(histogram_name_base); |
+ switch (whitelist_id) { |
+ case SBWhitelistId::CSD: |
+ histogram_name_full.append(".CSD"); |
+ break; |
+ case SBWhitelistId::DOWNLOAD: |
+ histogram_name_full.append(".DownloadWhitelist"); |
+ break; |
+ } |
+ |
+ // Histogram properties as in UMA_HISTOGRAM_COUNTS macro. |
+ base::HistogramBase* histogram_pointer = base::Histogram::FactoryGet( |
+ histogram_name_full, 1, 1000000, 50, |
+ base::HistogramBase::kUmaTargetedHistogramFlag); |
+ |
+ histogram_pointer->Add(sample); |
+} |
Scott Hess - ex-Googler
2014/12/18 23:57:55
I think these might usefully use a helper on the o
gab
2014/12/19 15:25:53
It felt to me like that avoiding the readability o
|