Can now adjust the number of retries before the blacklist is disabled.

chrome/browser/chrome_elf_init_win.cc

Index: chrome/browser/chrome_elf_init_win.cc
diff --git a/chrome/browser/chrome_elf_init_win.cc b/chrome/browser/chrome_elf_init_win.cc
index e09d2c9c3928a28657f54831bcb777657fa13c1d..f979cca7b048470c0b84e239e67028667f9d0f8d 100644
--- a/chrome/browser/chrome_elf_init_win.cc
+++ b/chrome/browser/chrome_elf_init_win.cc
@@ -40,12 +40,15 @@ enum BlacklistSetupEventType {
   // The blacklist setup code failed to execute.
   BLACKLIST_SETUP_FAILED,
 
-  // The blacklist thunk setup code failed to execute.
+  // Depreciated. The blacklist thunk setup code failed to execute.
   BLACKLIST_THUNK_SETUP_FAILED,
 
-  // The blacklist interception code failed to execute.
+  // Depreciated. The blacklist interception code failed to execute.

[Ilya Sherman, 2014/06/13 19:42:57]

nit: "Depreciated" -> "Deprecated" (obsolete vs. diminished in value)

[krstnmnlsn, 2014/06/13 20:55:47]

Done.

   BLACKLIST_INTERCEPTION_FAILED,
 
+  // The blacklist was disabled for this run (after it failed too many times).
+  BLACKLIST_SETUP_DISABLED,

[Ilya Sherman, 2014/06/13 19:42:57]

Did you mean to add this to the enum in histograms.xml?

[krstnmnlsn, 2014/06/13 20:55:47]

Done.

+
   // Always keep this at the end.
   BLACKLIST_SETUP_EVENT_MAX,
 };
@@ -140,26 +143,21 @@ void BrowserBlacklistBeaconSetup() {
   blacklist_registry_key.ReadValueDW(blacklist::kBeaconState, &blacklist_state);
 
   if (blacklist_state == blacklist::BLACKLIST_ENABLED) {
+    // The blacklist was enabled successfully so we record the event (along with
+    // the number of failed previous attempts).
     RecordBlacklistSetupEvent(BLACKLIST_SETUP_RAN_SUCCESSFULLY);
-  } else {
-    switch (blacklist_state) {
-      case blacklist::BLACKLIST_SETUP_RUNNING:
-        RecordBlacklistSetupEvent(BLACKLIST_SETUP_FAILED);
-        break;
-      case blacklist::BLACKLIST_THUNK_SETUP:
-        RecordBlacklistSetupEvent(BLACKLIST_THUNK_SETUP_FAILED);
-        break;
-      case blacklist::BLACKLIST_INTERCEPTING:
-        RecordBlacklistSetupEvent(BLACKLIST_INTERCEPTION_FAILED);
-        break;
-    }
-
-    // Since some part of the blacklist failed, mark it as disabled
-    // for this version.
-    if (blacklist_state != blacklist::BLACKLIST_DISABLED) {
-      blacklist_registry_key.WriteValue(blacklist::kBeaconState,
-                                        blacklist::BLACKLIST_DISABLED);
-    }
+    DWORD attempt_count = 0;
+    blacklist_registry_key.ReadValueDW(blacklist::kBeaconAttemptCount,
+                                       &attempt_count);
+    UMA_HISTOGRAM_COUNTS("Blacklist.RetryAttempts.Success", attempt_count);

[Ilya Sherman, 2014/06/13 19:42:57]

The upper bound for UMA_HISTOGRAM_COUNTS is 1000000.  Do you think such a large
upper bound is useful for this histogram?  I'd guess that
UMA_HISTOGRAM_COUNTS_100 might serve you better, as it'd give you a finer
resolution in the mid-range.

[krstnmnlsn, 2014/06/13 20:55:47]

I was wondering if I should do that, this number should always be very small so
I'll switch it.

+  } else if (blacklist_state == blacklist::BLACKLIST_SETUP_FAILED) {
+    // We can set the state to disabled without checking that the maximum number
+    // of attempts was exceeded because blacklist.cc has already done this.
+    RecordBlacklistSetupEvent(BLACKLIST_SETUP_FAILED);
+    blacklist_registry_key.WriteValue(blacklist::kBeaconState,
+                                      blacklist::BLACKLIST_DISABLED);
+  } else if (blacklist_state == blacklist::BLACKLIST_DISABLED) {
+    RecordBlacklistSetupEvent(BLACKLIST_SETUP_DISABLED);
   }
 
   // Find the last recorded blacklist version.
@@ -168,7 +166,8 @@ void BrowserBlacklistBeaconSetup() {
                                    &blacklist_version);
 
   if (blacklist_version != TEXT(CHROME_VERSION_STRING)) {
-    // The blacklist hasn't been enabled for this version yet, so enable it.
+    // The blacklist hasn't been enabled for this version yet, so enable it
+    // and reset the failure count to zero.
     LONG set_version = blacklist_registry_key.WriteValue(
         blacklist::kBeaconVersion,
         TEXT(CHROME_VERSION_STRING));
@@ -177,6 +176,9 @@ void BrowserBlacklistBeaconSetup() {
         blacklist::kBeaconState,
         blacklist::BLACKLIST_ENABLED);
 
+    blacklist_registry_key.WriteValue(blacklist::kBeaconAttemptCount,
+                                      static_cast<DWORD>(0));
+
     // Only report the blacklist as getting setup when both registry writes
     // succeed, since otherwise the blacklist wasn't properly setup.
     if (set_version == ERROR_SUCCESS && set_state == ERROR_SUCCESS)