Change default mode of extension install verification

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 8c1759bfb0d5dd4d5df4368a5ba02d70d57b6222..37a4a3ec25ec8133e1c48786f9e001db8b0455ec 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -545,7 +545,7 @@ void ExtensionService::Init() {
     InstallVerifier* verifier =
         extensions::ExtensionSystem::Get(profile_)->install_verifier();
     if (verifier->NeedsBootstrap())
-      VerifyAllExtensions();
+      VerifyAllExtensions(true /* bootstrap */);

[Finnur, 2014/01/29 10:35:42]

nit: I thought we generally shied away from multi-line comments? 
Can you change this to:
VerifyAllExtensions(true);  // bootstrap=true.

[asargent_no_longer_on_chrome, 2014/01/29 19:52:51]

Done.

     base::MessageLoop::current()->PostDelayedTask(
         FROM_HERE,
         base::Bind(&ExtensionService::GarbageCollectExtensions, AsWeakPtr()),
@@ -563,7 +563,7 @@ void ExtensionService::Init() {
                       base::Time::Now() - begin_time);
 }
 
-void ExtensionService::VerifyAllExtensions() {
+void ExtensionService::VerifyAllExtensions(bool bootstrap) {
   ExtensionIdSet to_add;
   scoped_ptr<ExtensionSet> all_extensions = GenerateInstalledExtensionsSet();
 
@@ -576,10 +576,42 @@ void ExtensionService::VerifyAllExtensions() {
   }
   extensions::ExtensionSystem::Get(profile_)->install_verifier()->AddMany(
       to_add, base::Bind(&ExtensionService::FinishVerifyAllExtensions,
-                         AsWeakPtr()));
+                         AsWeakPtr(), bootstrap));
 }
 
-void ExtensionService::FinishVerifyAllExtensions(bool success) {
+namespace {
+
+enum VerifyAllSuccess {
+  VERIFY_ALL_BOOTSTRAP_SUCCESS,

[Finnur, 2014/01/29 10:35:42]

Nit: We're not consistent on this throughout Chrome, but I tend to explicitly
give the first enum the value of 0. I read somewhere that it is up to the
compiler what values it uses if the developer doesn't specify it and that could
be bad for the histograms if it picked different values on different
platforms/used negative numbers. Anyway, up to you.

[asargent_no_longer_on_chrome, 2014/01/29 19:52:51]

Good advice, done. 

+  VERIFY_ALL_BOOTSTRAP_FAILURE,
+  VERIFY_ALL_NON_BOOTSTRAP_SUCCESS,
+  VERIFY_ALL_NON_BOOTSTRAP_FAILURE,
+
+  // Used in histograms. Do not remove/reorder any entries above, and the below
+  // MAX entry should always come last.
+
+  VERIFY_ALL_SUCCESS_MAX
+};
+
+void LogVerifyAllSuccessHistogram(bool bootstrap, bool success) {
+  VerifyAllSuccess result;
+  if (bootstrap && success)
+    result = VERIFY_ALL_BOOTSTRAP_SUCCESS;
+  else if (bootstrap && !success)
+    result = VERIFY_ALL_BOOTSTRAP_FAILURE;
+  else if (!bootstrap && success)
+    result = VERIFY_ALL_NON_BOOTSTRAP_SUCCESS;
+  else
+    result = VERIFY_ALL_NON_BOOTSTRAP_FAILURE;
+
+  UMA_HISTOGRAM_ENUMERATION("ExtensionService.VerifyAllSuccess",
+                            result, VERIFY_ALL_SUCCESS_MAX);
+}
+
+}  // namespace
+
+void ExtensionService::FinishVerifyAllExtensions(bool bootstrap, bool success) {
+  LogVerifyAllSuccessHistogram(bootstrap, success);
   if (success) {
     // Check to see if any currently unverified extensions became verified.
     InstallVerifier* verifier =
@@ -2178,6 +2210,14 @@ void ExtensionService::OnExtensionInstalled(
   }
 }
 
+namespace {
+
+void LogAddVerifiedSuccess(bool success) {
+  UMA_HISTOGRAM_BOOLEAN("ExtensionService.AddVerified", success);
+}

[Finnur, 2014/01/29 10:35:42]

nit: I would probably prefer to group these utility functions along with the
rest (in the anonymous namespace at the top), instead of interspersed between
member functions. Same goes for LogVerifyAllSuccessHistogram and the enum above
it. 

[asargent_no_longer_on_chrome, 2014/01/29 19:52:51]

Done.

+
+}  // namespace
+
 void ExtensionService::AddNewOrUpdatedExtension(
     const Extension* extension,
     Extension::State initial_state,
@@ -2193,7 +2233,7 @@ void ExtensionService::AddNewOrUpdatedExtension(
   delayed_installs_.Remove(extension->id());
   if (InstallVerifier::NeedsVerification(*extension)) {
     extensions::ExtensionSystem::Get(profile_)->install_verifier()->Add(
-        extension->id(), InstallVerifier::AddResultCallback());
+        extension->id(), base::Bind(LogAddVerifiedSuccess));
   }
   FinishInstallation(extension);
 }