Extensions: Add some content verification UMA.
For computed_hashes.json and verified_contents.json:
Time how long it takes of successfully reading the files and
serializing them. Also record how frequently serialization
fails.
ComputedHashesInitTime/ComputedHashesReadResult
VerifiedContentsInitTime/VerifiedContentsInitResult
Record if fetching verified_contents succeeds or fails.
And the most important one "ReadContentHashTime", would give a
high level overview of how much time we spend on just "getting the
stuff", discarding any oddities around file missing and such.
Bug: 859729
Change-Id: Iaef0d495a55bd910811b7cd3b8a560ed6b52c719
Reviewed-on: https://chromium-review.googlesource.com/1121382
Commit-Queue: Istiaque Ahmed <[email protected]>
Reviewed-by: Ilya Sherman <[email protected]>
Reviewed-by: Devlin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#573054}diff --git a/extensions/browser/computed_hashes.cc b/extensions/browser/computed_hashes.cc
index cfe0a9e8..e0a365b 100644
--- a/extensions/browser/computed_hashes.cc
+++ b/extensions/browser/computed_hashes.cc
@@ -12,7 +12,9 @@
#include "base/files/file_util.h"
#include "base/json/json_reader.h"
#include "base/json/json_writer.h"
+#include "base/metrics/histogram_macros.h"
#include "base/stl_util.h"
+#include "base/timer/elapsed_timer.h"
#include "base/values.h"
#include "crypto/secure_hash.h"
#include "crypto/sha2.h"
@@ -28,6 +30,43 @@
const int kVersion = 2;
} // namespace computed_hashes
+namespace {
+
+// Helper to record UMA for ComputedHashes::Reader::InitFromFile.
+// Records failure UMA if RecordSuccess() isn't explicitly called.
+class ScopedUMARecorder {
+ public:
+ ScopedUMARecorder() = default;
+
+ ~ScopedUMARecorder() {
+ if (recorded_)
+ return;
+ RecordImpl(false);
+ }
+
+ void RecordSuccess() {
+ recorded_ = true;
+ RecordImpl(true);
+ }
+
+ private:
+ void RecordImpl(bool succeeded) {
+ UMA_HISTOGRAM_BOOLEAN(
+ "Extensions.ContentVerification.ComputedHashesReadResult", succeeded);
+ if (succeeded) {
+ UMA_HISTOGRAM_TIMES(
+ "Extensions.ContentVerification.ComputedHashesInitTime",
+ timer_.Elapsed());
+ }
+ }
+
+ bool recorded_ = false;
+ base::ElapsedTimer timer_;
+ DISALLOW_COPY_AND_ASSIGN(ScopedUMARecorder);
+};
+
+} // namespace
+
ComputedHashes::Reader::Reader() {
}
@@ -35,6 +74,7 @@
}
bool ComputedHashes::Reader::InitFromFile(const base::FilePath& path) {
+ ScopedUMARecorder uma_recorder;
std::string contents;
if (!base::ReadFileToString(path, &contents))
return false;
@@ -96,6 +136,7 @@
}
}
}
+ uma_recorder.RecordSuccess();
return true;
}
diff --git a/extensions/browser/content_verifier.cc b/extensions/browser/content_verifier.cc
index 1e3901c..c7063684 100644
--- a/extensions/browser/content_verifier.cc
+++ b/extensions/browser/content_verifier.cc
@@ -10,10 +10,12 @@
#include "base/files/file_path.h"
#include "base/memory/weak_ptr.h"
+#include "base/metrics/histogram_macros.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "base/task_scheduler/post_task.h"
#include "base/threading/thread_restrictions.h"
+#include "base/timer/elapsed_timer.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "extensions/browser/content_hash_fetcher.h"
@@ -230,10 +232,13 @@
void Cancel() { cancelled_checker->Cancel(); }
+ base::TimeDelta elapsed() const { return elapsed_timer.Elapsed(); }
+
scoped_refptr<IsCancelledChecker> cancelled_checker;
// TODO(lazyboy): Use std::list?
std::vector<ContentHashCallback> callbacks;
bool force_missing_computed_hashes_creation = false;
+ base::ElapsedTimer elapsed_timer;
};
using IsCancelledCallback = base::RepeatingCallback<bool(void)>;
@@ -326,6 +331,8 @@
auto iter = callback_infos_.find(key);
DCHECK(iter != callback_infos_.end());
auto& callback_info = iter->second;
+ UMA_HISTOGRAM_TIMES("Extensions.ContentVerification.ReadContentHashTime",
+ callback_info.elapsed());
for (auto& callback : callback_info.callbacks)
std::move(callback).Run(content_hash);
diff --git a/extensions/browser/content_verifier/content_hash.cc b/extensions/browser/content_verifier/content_hash.cc
index 119a0cb..18aa94f 100644
--- a/extensions/browser/content_verifier/content_hash.cc
+++ b/extensions/browser/content_verifier/content_hash.cc
@@ -219,6 +219,7 @@
return;
}
+ RecordFetchResult(true);
scoped_refptr<ContentHash> hash =
new ContentHash(key, std::move(verified_contents), nullptr);
const bool did_fetch_verified_contents = true;
@@ -232,6 +233,7 @@
const ExtensionKey& key,
CreatedCallback created_callback,
const IsCancelledCallback& is_cancelled) {
+ RecordFetchResult(false);
// NOTE: bare new because ContentHash constructor is private.
scoped_refptr<ContentHash> content_hash =
new ContentHash(key, nullptr, nullptr);
@@ -239,6 +241,11 @@
.Run(content_hash, is_cancelled && is_cancelled.Run());
}
+// static
+void ContentHash::RecordFetchResult(bool success) {
+ UMA_HISTOGRAM_BOOLEAN("Extensions.ContentVerification.FetchResult", success);
+}
+
bool ContentHash::CreateHashes(const base::FilePath& hashes_file,
const IsCancelledCallback& is_cancelled) {
base::ElapsedTimer timer;
diff --git a/extensions/browser/content_verifier/content_hash.h b/extensions/browser/content_verifier/content_hash.h
index 50fbd6f..26f4a64 100644
--- a/extensions/browser/content_verifier/content_hash.h
+++ b/extensions/browser/content_verifier/content_hash.h
@@ -157,6 +157,8 @@
CreatedCallback created_callback,
const IsCancelledCallback& is_cancelled);
+ static void RecordFetchResult(bool success);
+
// Computes hashes for all files in |key_.extension_root|, and uses
// a ComputedHashes::Writer to write that information into |hashes_file|.
// Returns true on success.
diff --git a/extensions/browser/verified_contents.cc b/extensions/browser/verified_contents.cc
index 52cba1b..654a4dd 100644
--- a/extensions/browser/verified_contents.cc
+++ b/extensions/browser/verified_contents.cc
@@ -10,7 +10,9 @@
#include "base/base64url.h"
#include "base/files/file_util.h"
#include "base/json/json_reader.h"
+#include "base/metrics/histogram_macros.h"
#include "base/strings/string_util.h"
+#include "base/timer/elapsed_timer.h"
#include "base/values.h"
#include "build/build_config.h"
#include "components/crx_file/id_util.h"
@@ -59,6 +61,41 @@
return NULL;
}
+// Helper to record UMA for results of initializing verified_contents.json file.
+// TODO(lazyboy): Merge this with ScopedUMARecorder in computed_hashes.cc.
+class ScopedUMARecorder {
+ public:
+ ScopedUMARecorder() = default;
+
+ ~ScopedUMARecorder() {
+ if (recorded_)
+ return;
+ RecordImpl(false);
+ }
+
+ void RecordSuccess() {
+ recorded_ = true;
+ RecordImpl(true);
+ }
+
+ private:
+ void RecordImpl(bool success) {
+ if (success) {
+ UMA_HISTOGRAM_TIMES(
+ "Extensions.ContentVerification.VerifiedContentsInitTime",
+ timer_.Elapsed());
+ }
+ UMA_HISTOGRAM_BOOLEAN(
+ "Extensions.ContentVerification.VerifiedContentsInitResult", success);
+ }
+
+ private:
+ base::ElapsedTimer timer_;
+ bool recorded_ = false;
+
+ DISALLOW_COPY_AND_ASSIGN(ScopedUMARecorder);
+};
+
#if defined(OS_WIN)
// Returns true if |path| ends with (.| )+.
// |out_path| will contain "." and/or " " suffix removed from |path|.
@@ -106,6 +143,7 @@
// ]
// }
bool VerifiedContents::InitFrom(const base::FilePath& path) {
+ ScopedUMARecorder uma_recorder;
std::string payload;
if (!GetPayload(path, &payload))
return false;
@@ -117,8 +155,9 @@
std::string item_id;
if (!dictionary->GetString(kItemIdKey, &item_id) ||
- !crx_file::id_util::IdIsValid(item_id))
+ !crx_file::id_util::IdIsValid(item_id)) {
return false;
+ }
extension_id_ = item_id;
std::string version_string;
@@ -143,8 +182,9 @@
int block_size = 0;
int hash_block_size = 0;
if (!hashes->GetInteger(kBlockSizeKey, &block_size) ||
- !hashes->GetInteger(kHashBlockSizeKey, &hash_block_size))
+ !hashes->GetInteger(kHashBlockSizeKey, &hash_block_size)) {
return false;
+ }
block_size_ = block_size;
// We don't support using a different block_size and hash_block_size at
@@ -168,8 +208,9 @@
!data->GetString(kRootHashKey, &encoded_root_hash) ||
!base::Base64UrlDecode(encoded_root_hash,
base::Base64UrlDecodePolicy::IGNORE_PADDING,
- &root_hash))
+ &root_hash)) {
return false;
+ }
base::FilePath file_path =
base::FilePath::FromUTF8Unsafe(file_path_string);
base::FilePath::StringType lowercase_file_path =
@@ -190,6 +231,7 @@
break;
}
+ uma_recorder.RecordSuccess();
return true;
}
