summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorkrstnmnlsn@chromium.org <krstnmnlsn@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-06-17 01:44:56 +0000
committerkrstnmnlsn@chromium.org <krstnmnlsn@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-06-17 01:44:56 +0000
commit37374fc0a1e42b777a741eebf2646a7b0ff46801 (patch)
tree5ade0f5e9d18c600a2804f3732261c7d963c78a8
parent18b1b1c4e504e2790e0d9c6366a594768a7c51f8 (diff)
downloadchromium_src-37374fc0a1e42b777a741eebf2646a7b0ff46801.zip
chromium_src-37374fc0a1e42b777a741eebf2646a7b0ff46801.tar.gz
chromium_src-37374fc0a1e42b777a741eebf2646a7b0ff46801.tar.bz2
Can now adjust the number of retries before the blacklist is disabled.
Also added UMA recording of the blacklist disabled and retry succeeded events. Deprecating the blacklist start up events 'BLACKLIST_THUNK_SETUP_FAILED' and 'BLACKLIST_INTERCEPTION_FAILED' as we are switching over to a more coarse grained recording of the set up status (this information can be gathered from crash reporting now anyways). BUG= Review URL: https://codereview.chromium.org/311893005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277614 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/chrome_elf_init_unittest_win.cc50
-rw-r--r--chrome/browser/chrome_elf_init_win.cc46
-rw-r--r--chrome_elf/blacklist/blacklist.cc140
-rw-r--r--chrome_elf/blacklist/blacklist.h16
-rw-r--r--chrome_elf/blacklist/test/blacklist_test.cc119
-rw-r--r--chrome_elf/chrome_elf_constants.cc3
-rw-r--r--chrome_elf/chrome_elf_constants.h19
-rw-r--r--tools/metrics/histograms/histograms.xml12
8 files changed, 250 insertions, 155 deletions
diff --git a/chrome/browser/chrome_elf_init_unittest_win.cc b/chrome/browser/chrome_elf_init_unittest_win.cc
index f98ee08..c75edc0 100644
--- a/chrome/browser/chrome_elf_init_unittest_win.cc
+++ b/chrome/browser/chrome_elf_init_unittest_win.cc
@@ -117,55 +117,29 @@ TEST_F(ChromeBlacklistTrialTest, VerifyFirstRun) {
ASSERT_EQ(version, GetBlacklistVersion());
}
-TEST_F(ChromeBlacklistTrialTest, SetupFailed) {
- // Set the registry to indicate that the blacklist setup is running,
- // which means it failed to run correctly last time for this version.
+TEST_F(ChromeBlacklistTrialTest, BlacklistFailed) {
+ // Ensure when the blacklist set up failed we set the state to disabled for
+ // future runs.
blacklist_registry_key_->WriteValue(blacklist::kBeaconVersion,
TEXT(CHROME_VERSION_STRING));
blacklist_registry_key_->WriteValue(blacklist::kBeaconState,
- blacklist::BLACKLIST_SETUP_RUNNING);
-
- BrowserBlacklistBeaconSetup();
-
- // Since the blacklist setup failed, it should now be disabled.
- ASSERT_EQ(blacklist::BLACKLIST_DISABLED, GetBlacklistState());
-}
-
-TEST_F(ChromeBlacklistTrialTest, ThunkSetupFailed) {
- // Set the registry to indicate that the blacklist thunk setup is running,
- // which means it failed to run correctly last time for this version.
- blacklist_registry_key_->WriteValue(blacklist::kBeaconVersion,
- TEXT(CHROME_VERSION_STRING));
- blacklist_registry_key_->WriteValue(blacklist::kBeaconState,
- blacklist::BLACKLIST_THUNK_SETUP);
-
- BrowserBlacklistBeaconSetup();
-
- // Since the blacklist thunk setup failed, it should now be disabled.
- ASSERT_EQ(blacklist::BLACKLIST_DISABLED, GetBlacklistState());
-}
-
-TEST_F(ChromeBlacklistTrialTest, InterceptionFailed) {
- // Set the registry to indicate that an interception is running,
- // which means it failed to run correctly last time for this version.
- blacklist_registry_key_->WriteValue(blacklist::kBeaconVersion,
- TEXT(CHROME_VERSION_STRING));
- blacklist_registry_key_->WriteValue(blacklist::kBeaconState,
- blacklist::BLACKLIST_INTERCEPTING);
+ blacklist::BLACKLIST_SETUP_FAILED);
BrowserBlacklistBeaconSetup();
- // Since an interception failed, the blacklist should now be disabled.
ASSERT_EQ(blacklist::BLACKLIST_DISABLED, GetBlacklistState());
}
TEST_F(ChromeBlacklistTrialTest, VersionChanged) {
- // Mark the blacklist as disabled for an older version, so it should
- // get enabled for this new version.
+ // Mark the blacklist as disabled for an older version, it should
+ // get enabled for this new version. Also record a non-zero number of
+ // setup failures, which should be reset to zero.
blacklist_registry_key_->WriteValue(blacklist::kBeaconVersion,
L"old_version");
blacklist_registry_key_->WriteValue(blacklist::kBeaconState,
blacklist::BLACKLIST_DISABLED);
+ blacklist_registry_key_->WriteValue(blacklist::kBeaconAttemptCount,
+ blacklist::kBeaconMaxAttempts);
BrowserBlacklistBeaconSetup();
@@ -175,6 +149,12 @@ TEST_F(ChromeBlacklistTrialTest, VersionChanged) {
chrome::VersionInfo version_info;
base::string16 expected_version(base::UTF8ToUTF16(version_info.Version()));
ASSERT_EQ(expected_version, GetBlacklistVersion());
+
+ // The counter should be reset.
+ DWORD attempt_count = blacklist::kBeaconMaxAttempts;
+ blacklist_registry_key_->ReadValueDW(blacklist::kBeaconAttemptCount,
+ &attempt_count);
+ ASSERT_EQ(static_cast<DWORD>(0), attempt_count);
}
TEST_F(ChromeBlacklistTrialTest, AddFinchBlacklistToRegistry) {
diff --git a/chrome/browser/chrome_elf_init_win.cc b/chrome/browser/chrome_elf_init_win.cc
index e09d2c9..c78ca17 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.
+ // Deprecated. The blacklist thunk setup code failed to execute.
BLACKLIST_THUNK_SETUP_FAILED,
- // The blacklist interception code failed to execute.
+ // Deprecated. The blacklist interception code failed to execute.
BLACKLIST_INTERCEPTION_FAILED,
+ // The blacklist was disabled for this run (after it failed too many times).
+ BLACKLIST_SETUP_DISABLED,
+
// 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_100("Blacklist.RetryAttempts.Success", attempt_count);
+ } 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)
diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
index 8107ffc..055b0ab 100644
--- a/chrome_elf/blacklist/blacklist.cc
+++ b/chrome_elf/blacklist/blacklist.cc
@@ -59,20 +59,54 @@ namespace {
// determine if the blacklist is enabled for them.
bool g_blacklist_initialized = false;
-// Record that the thunk setup completed succesfully and close the registry
-// key handle since it is no longer needed.
-void RecordSuccessfulThunkSetup(HKEY* key) {
- if (key != NULL) {
- DWORD blacklist_state = blacklist::BLACKLIST_SETUP_RUNNING;
- ::RegSetValueEx(*key,
- blacklist::kBeaconState,
- 0,
- REG_DWORD,
- reinterpret_cast<LPBYTE>(&blacklist_state),
- sizeof(blacklist_state));
- ::RegCloseKey(*key);
- key = NULL;
+// Helper to set DWORD registry values.
+DWORD SetDWValue(HKEY* key, const wchar_t* property, DWORD value) {
+ return ::RegSetValueEx(*key,
+ property,
+ 0,
+ REG_DWORD,
+ reinterpret_cast<LPBYTE>(&value),
+ sizeof(value));
+}
+
+bool GenerateStateFromBeaconAndAttemptCount(HKEY* key, DWORD blacklist_state) {
+ LONG result = 0;
+ if (blacklist_state == blacklist::BLACKLIST_SETUP_RUNNING) {
+ // Some part of the blacklist setup failed last time. If this has occured
+ // blacklist::kBeaconMaxAttempts times in a row we switch the state to
+ // failed and skip setting up the blacklist.
+ DWORD attempt_count = 0;
+ DWORD attempt_count_size = sizeof(attempt_count);
+ result = ::RegQueryValueEx(*key,
+ blacklist::kBeaconAttemptCount,
+ 0,
+ NULL,
+ reinterpret_cast<LPBYTE>(&attempt_count),
+ &attempt_count_size);
+
+ if (result == ERROR_FILE_NOT_FOUND)
+ attempt_count = 0;
+ else if (result != ERROR_SUCCESS)
+ return false;
+
+ ++attempt_count;
+ SetDWValue(key, blacklist::kBeaconAttemptCount, attempt_count);
+
+ if (attempt_count >= blacklist::kBeaconMaxAttempts) {
+ blacklist_state = blacklist::BLACKLIST_SETUP_FAILED;
+ SetDWValue(key, blacklist::kBeaconState, blacklist_state);
+ return false;
+ }
+ } else if (blacklist_state == blacklist::BLACKLIST_ENABLED) {
+ // If the blacklist succeeded on the previous run reset the failure
+ // counter.
+ result =
+ SetDWValue(key, blacklist::kBeaconAttemptCount, static_cast<DWORD>(0));
+ if (result != ERROR_SUCCESS) {
+ return false;
+ }
}
+ return true;
}
} // namespace
@@ -102,7 +136,7 @@ bool LeaveSetupBeacon() {
return false;
// Retrieve the current blacklist state.
- DWORD blacklist_state = BLACKLIST_DISABLED;
+ DWORD blacklist_state = BLACKLIST_STATE_MAX;
DWORD blacklist_state_size = sizeof(blacklist_state);
DWORD type = 0;
result = ::RegQueryValueEx(key,
@@ -112,21 +146,18 @@ bool LeaveSetupBeacon() {
reinterpret_cast<LPBYTE>(&blacklist_state),
&blacklist_state_size);
- if (blacklist_state != BLACKLIST_ENABLED ||
- result != ERROR_SUCCESS || type != REG_DWORD) {
+ if (blacklist_state == BLACKLIST_DISABLED || result != ERROR_SUCCESS ||
+ type != REG_DWORD) {
+ ::RegCloseKey(key);
+ return false;
+ }
+
+ if (!GenerateStateFromBeaconAndAttemptCount(&key, blacklist_state)) {
::RegCloseKey(key);
return false;
}
- // Mark the blacklist setup code as running so if it crashes the blacklist
- // won't be enabled for the next run.
- blacklist_state = BLACKLIST_SETUP_RUNNING;
- result = ::RegSetValueEx(key,
- kBeaconState,
- 0,
- REG_DWORD,
- reinterpret_cast<LPBYTE>(&blacklist_state),
- sizeof(blacklist_state));
+ result = SetDWValue(&key, kBeaconState, BLACKLIST_SETUP_RUNNING);
::RegCloseKey(key);
return (result == ERROR_SUCCESS);
@@ -147,15 +178,28 @@ bool ResetBeacon() {
if (result != ERROR_SUCCESS)
return false;
- DWORD blacklist_state = BLACKLIST_ENABLED;
- result = ::RegSetValueEx(key,
- kBeaconState,
- 0,
- REG_DWORD,
- reinterpret_cast<LPBYTE>(&blacklist_state),
- sizeof(blacklist_state));
- ::RegCloseKey(key);
+ DWORD blacklist_state = BLACKLIST_STATE_MAX;
+ DWORD blacklist_state_size = sizeof(blacklist_state);
+ DWORD type = 0;
+ result = ::RegQueryValueEx(key,
+ kBeaconState,
+ 0,
+ &type,
+ reinterpret_cast<LPBYTE>(&blacklist_state),
+ &blacklist_state_size);
+ if (result != ERROR_SUCCESS || type != REG_DWORD) {
+ ::RegCloseKey(key);
+ return false;
+ }
+
+ // Reaching this point with the setup running state means the setup
+ // succeeded and so we reset to enabled. Any other state indicates that setup
+ // was skipped; in that case we leave the state alone for later recording.
+ if (blacklist_state == BLACKLIST_SETUP_RUNNING)
+ result = SetDWValue(&key, kBeaconState, BLACKLIST_ENABLED);
+
+ ::RegCloseKey(key);
return (result == ERROR_SUCCESS);
}
@@ -253,7 +297,8 @@ bool Initialize(bool force) {
if (IsNonBrowserProcess())
return false;
- // Check to see if a beacon is present, abort if so.
+ // Check to see if the blacklist beacon is still set to running (indicating a
+ // failure) or disabled, and abort if so.
if (!force && !LeaveSetupBeacon())
return false;
@@ -268,30 +313,6 @@ bool Initialize(bool force) {
if (!thunk)
return false;
- // Record that we are starting the thunk setup code.
- HKEY key = NULL;
- DWORD disposition = 0;
- LONG result = ::RegCreateKeyEx(HKEY_CURRENT_USER,
- kRegistryBeaconPath,
- 0,
- NULL,
- REG_OPTION_NON_VOLATILE,
- KEY_QUERY_VALUE | KEY_SET_VALUE,
- NULL,
- &key,
- &disposition);
- if (result == ERROR_SUCCESS) {
- DWORD blacklist_state = BLACKLIST_THUNK_SETUP;
- ::RegSetValueEx(key,
- kBeaconState,
- 0,
- REG_DWORD,
- reinterpret_cast<LPBYTE>(&blacklist_state),
- sizeof(blacklist_state));
- } else {
- key = NULL;
- }
-
BYTE* thunk_storage = reinterpret_cast<BYTE*>(&g_thunk_storage);
// Mark the thunk storage as readable and writeable, since we
@@ -301,7 +322,6 @@ bool Initialize(bool force) {
sizeof(g_thunk_storage),
PAGE_EXECUTE_READWRITE,
&old_protect)) {
- RecordSuccessfulThunkSetup(&key);
return false;
}
@@ -353,8 +373,6 @@ bool Initialize(bool force) {
PAGE_EXECUTE_READ,
&old_protect);
- RecordSuccessfulThunkSetup(&key);
-
AddDllsFromRegistryToBlacklist();
return NT_SUCCESS(ret) && page_executable;
diff --git a/chrome_elf/blacklist/blacklist.h b/chrome_elf/blacklist/blacklist.h
index 7b8d7a6..58cc16b 100644
--- a/chrome_elf/blacklist/blacklist.h
+++ b/chrome_elf/blacklist/blacklist.h
@@ -21,16 +21,16 @@ extern const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount];
extern NtMapViewOfSectionFunction g_nt_map_view_of_section_func;
#endif
-// Attempts to leave a beacon in the current user's registry hive.
-// If the blacklist beacon doesn't say it is enabled or there are any other
-// errors when creating the beacon, returns false. Otherwise returns true.
-// The intent of the beacon is to act as an extra failure mode protection
-// whereby if Chrome for some reason fails to start during blacklist setup,
-// it will skip blacklisting on the subsequent run.
+// Attempts to leave a beacon in the current user's registry hive. If the
+// blacklist beacon doesn't say it is enabled or there are any other errors when
+// creating the beacon, returns false. Otherwise returns true. The intent of the
+// beacon is to act as an extra failure mode protection whereby if Chrome
+// repeatedly fails to start during blacklist setup, it will skip blacklisting
+// on the subsequent run.
bool LeaveSetupBeacon();
-// Looks for the beacon that LeaveSetupBeacon() creates and resets it to
-// to show the setup was successful.
+// Looks for the setup running beacon that LeaveSetupBeacon() creates and resets
+// it to to show the setup was successful.
// Returns true if the beacon was successfully set to BLACKLIST_ENABLED.
bool ResetBeacon();
diff --git a/chrome_elf/blacklist/test/blacklist_test.cc b/chrome_elf/blacklist/test/blacklist_test.cc
index 1f91956..2e3f9a4 100644
--- a/chrome_elf/blacklist/test/blacklist_test.cc
+++ b/chrome_elf/blacklist/test/blacklist_test.cc
@@ -42,10 +42,25 @@ __declspec(dllimport) bool TestDll_SuccessfullyBlocked(
int* size);
}
+namespace {
+
class BlacklistTest : public testing::Test {
+ protected:
+ BlacklistTest() : override_manager_() {
+ override_manager_.OverrideRegistry(HKEY_CURRENT_USER, L"beacon_test");
+ }
+
+ scoped_ptr<base::win::RegKey> blacklist_registry_key_;
+ registry_util::RegistryOverrideManager override_manager_;
+
+ private:
virtual void SetUp() {
// Force an import from blacklist_test_main_dll.
InitBlacklistTestDll();
+ blacklist_registry_key_.reset(
+ new base::win::RegKey(HKEY_CURRENT_USER,
+ blacklist::kRegistryBeaconPath,
+ KEY_QUERY_VALUE | KEY_SET_VALUE));
}
virtual void TearDown() {
@@ -55,28 +70,19 @@ class BlacklistTest : public testing::Test {
}
};
-namespace {
-
struct TestData {
const wchar_t* dll_name;
const wchar_t* dll_beacon;
} test_data[] = {{kTestDllName2, kDll2Beacon}, {kTestDllName3, kDll3Beacon}};
TEST_F(BlacklistTest, Beacon) {
- registry_util::RegistryOverrideManager override_manager;
- override_manager.OverrideRegistry(HKEY_CURRENT_USER, L"beacon_test");
-
- base::win::RegKey blacklist_registry_key(HKEY_CURRENT_USER,
- blacklist::kRegistryBeaconPath,
- KEY_QUERY_VALUE | KEY_SET_VALUE);
-
- // Ensure that the beacon state starts off enabled for this version.
- LONG result = blacklist_registry_key.WriteValue(blacklist::kBeaconState,
- blacklist::BLACKLIST_ENABLED);
+ // Ensure that the beacon state starts off 'running' for this version.
+ LONG result = blacklist_registry_key_->WriteValue(
+ blacklist::kBeaconState, blacklist::BLACKLIST_SETUP_RUNNING);
EXPECT_EQ(ERROR_SUCCESS, result);
- result = blacklist_registry_key.WriteValue(blacklist::kBeaconVersion,
- TEXT(CHROME_VERSION_STRING));
+ result = blacklist_registry_key_->WriteValue(blacklist::kBeaconVersion,
+ TEXT(CHROME_VERSION_STRING));
EXPECT_EQ(ERROR_SUCCESS, result);
// First call should find the beacon and reset it.
@@ -84,12 +90,6 @@ TEST_F(BlacklistTest, Beacon) {
// First call should succeed as the beacon is enabled.
EXPECT_TRUE(blacklist::LeaveSetupBeacon());
-
- // Second call should fail indicating the beacon wasn't set as enabled.
- EXPECT_FALSE(blacklist::LeaveSetupBeacon());
-
- // Resetting the beacon should work when setup beacon is present.
- EXPECT_TRUE(blacklist::ResetBeacon());
}
TEST_F(BlacklistTest, AddAndRemoveModules) {
@@ -244,4 +244,83 @@ TEST_F(BlacklistTest, AddDllsFromRegistryToBlacklist) {
CheckBlacklistedDllsNotLoaded();
}
+void TestResetBeacon(scoped_ptr<base::win::RegKey>& key,
+ DWORD input_state,
+ DWORD expected_output_state) {
+ LONG result = key->WriteValue(blacklist::kBeaconState, input_state);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+
+ EXPECT_TRUE(blacklist::ResetBeacon());
+ DWORD blacklist_state = blacklist::BLACKLIST_STATE_MAX;
+ result = key->ReadValueDW(blacklist::kBeaconState, &blacklist_state);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+ EXPECT_EQ(expected_output_state, blacklist_state);
+}
+
+TEST_F(BlacklistTest, ResetBeacon) {
+ // Ensure that ResetBeacon resets properly on successful runs and not on
+ // failed or disabled runs.
+ TestResetBeacon(blacklist_registry_key_,
+ blacklist::BLACKLIST_SETUP_RUNNING,
+ blacklist::BLACKLIST_ENABLED);
+
+ TestResetBeacon(blacklist_registry_key_,
+ blacklist::BLACKLIST_SETUP_FAILED,
+ blacklist::BLACKLIST_SETUP_FAILED);
+
+ TestResetBeacon(blacklist_registry_key_,
+ blacklist::BLACKLIST_DISABLED,
+ blacklist::BLACKLIST_DISABLED);
+}
+
+TEST_F(BlacklistTest, SetupFailed) {
+ // Ensure that when the number of failed tries reaches the maximum allowed,
+ // the blacklist state is set to failed.
+ LONG result = blacklist_registry_key_->WriteValue(
+ blacklist::kBeaconState, blacklist::BLACKLIST_SETUP_RUNNING);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+
+ // Set the attempt count so that on the next failure the blacklist is
+ // disabled.
+ result = blacklist_registry_key_->WriteValue(
+ blacklist::kBeaconAttemptCount, blacklist::kBeaconMaxAttempts - 1);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+
+ EXPECT_FALSE(blacklist::LeaveSetupBeacon());
+
+ DWORD attempt_count = 0;
+ blacklist_registry_key_->ReadValueDW(blacklist::kBeaconAttemptCount,
+ &attempt_count);
+ EXPECT_EQ(attempt_count, blacklist::kBeaconMaxAttempts);
+
+ DWORD blacklist_state = blacklist::BLACKLIST_STATE_MAX;
+ result = blacklist_registry_key_->ReadValueDW(blacklist::kBeaconState,
+ &blacklist_state);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+ EXPECT_EQ(blacklist_state, blacklist::BLACKLIST_SETUP_FAILED);
+}
+
+TEST_F(BlacklistTest, SetupSucceeded) {
+ // Starting with the enabled beacon should result in the setup running state
+ // and the attempt counter reset to zero.
+ LONG result = blacklist_registry_key_->WriteValue(
+ blacklist::kBeaconState, blacklist::BLACKLIST_ENABLED);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+ result = blacklist_registry_key_->WriteValue(blacklist::kBeaconAttemptCount,
+ blacklist::kBeaconMaxAttempts);
+ EXPECT_EQ(ERROR_SUCCESS, result);
+
+ EXPECT_TRUE(blacklist::LeaveSetupBeacon());
+
+ DWORD blacklist_state = blacklist::BLACKLIST_STATE_MAX;
+ blacklist_registry_key_->ReadValueDW(blacklist::kBeaconState,
+ &blacklist_state);
+ EXPECT_EQ(blacklist_state, blacklist::BLACKLIST_SETUP_RUNNING);
+
+ DWORD attempt_count = blacklist::kBeaconMaxAttempts;
+ blacklist_registry_key_->ReadValueDW(blacklist::kBeaconAttemptCount,
+ &attempt_count);
+ EXPECT_EQ(static_cast<DWORD>(0), attempt_count);
+}
+
} // namespace
diff --git a/chrome_elf/chrome_elf_constants.cc b/chrome_elf/chrome_elf_constants.cc
index 1dbc749..a7ee1c9 100644
--- a/chrome_elf/chrome_elf_constants.cc
+++ b/chrome_elf/chrome_elf_constants.cc
@@ -21,5 +21,8 @@ const wchar_t kRegistryFinchListPath[] =
L"SOFTWARE\\Google\\Chrome\\BLFinchList";
const wchar_t kBeaconVersion[] = L"version";
const wchar_t kBeaconState[] = L"state";
+const wchar_t kBeaconAttemptCount[] = L"failed_count";
+
+const DWORD kBeaconMaxAttempts = 2;
} // namespace blacklist
diff --git a/chrome_elf/chrome_elf_constants.h b/chrome_elf/chrome_elf_constants.h
index 4ff45b2..92e44ca 100644
--- a/chrome_elf/chrome_elf_constants.h
+++ b/chrome_elf/chrome_elf_constants.h
@@ -7,6 +7,8 @@
#ifndef CHROME_ELF_CHROME_ELF_CONSTANTS_H_
#define CHROME_ELF_CHROME_ELF_CONSTANTS_H_
+#include <windows.h>
+
// directory names
extern const wchar_t kAppDataDirName[];
extern const wchar_t kCanaryAppDataDirName[];
@@ -25,20 +27,21 @@ extern const wchar_t kRegistryFinchListPath[];
// The properties for the blacklist beacon.
extern const wchar_t kBeaconVersion[];
extern const wchar_t kBeaconState[];
+extern const wchar_t kBeaconAttemptCount[];
+
+// The number of failures that can occur on startup with the beacon enabled
+// before we give up and turn off the blacklist.
+extern const DWORD kBeaconMaxAttempts;
// The states for the blacklist setup code.
enum BlacklistState {
BLACKLIST_DISABLED = 0,
BLACKLIST_ENABLED,
- // The blacklist setup code is running. If this is still set at startup,
- // it means the last setup crashed.
+ // The blacklist setup code is running. If this is the state at startup, it
+ // means the last setup crashed.
BLACKLIST_SETUP_RUNNING,
- // The blacklist thunk setup code is running. If this is still set at startup,
- // it means the last setup crashed during thunk setup.
- BLACKLIST_THUNK_SETUP,
- // The blacklist code is currently intercepting MapViewOfSection. If this is
- // still set at startup, it means we crashed during interception.
- BLACKLIST_INTERCEPTING,
+ // If the last setup crashed, we reassign the state to failed.
+ BLACKLIST_SETUP_FAILED,
// Always keep this at the end.
BLACKLIST_STATE_MAX,
};
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 08e9b14..fc734a7 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -1852,12 +1852,21 @@ Therefore, the affected-histogram name has to have at least one dot in it.
</summary>
</histogram>
+<histogram name="Blacklist.RetryAttempts.Success">
+ <owner>csharp@chromium.org</owner>
+ <owner>krstnmnlsn@chromium.org</owner>
+ <summary>
+ Records the number of attempts needed before the blacklist is properly set
+ up. This is logged immediately after a successful setup.
+ </summary>
+</histogram>
+
<histogram name="Blacklist.Setup" enum="BlacklistSetup">
<owner>csharp@chromium.org</owner>
<summary>
Records the successes and failures when running the browser blacklist setup
code. Used to determine if the blacklist is working as intended during
- startup(since the blacklist runs before crash reporting is set up). This
+ startup (since the blacklist runs before crash reporting is set up). This
only occurs on Windows.
</summary>
</histogram>
@@ -33661,6 +33670,7 @@ Therefore, the affected-histogram name has to have at least one dot in it.
<int value="2" label="Blacklist failed."/>
<int value="3" label="Blacklist thunk setup failed."/>
<int value="4" label="Blacklist interception failed."/>
+ <int value="5" label="Blacklist disabled."/>
</enum>
<enum name="BluetoothPairingMethod" type="int">