Use a Finch Experiment to control the Browser Blacklist

Also increase the states stored in the register to prevent
the setup code from running multiple times per version if
it fails to start one time.

BUG=329023

Review URL: https://codereview.chromium.org/120963002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243967 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 130 insertions, 44 deletions

diff --git a/chrome_elf/blacklist.gypi b/chrome_elf/blacklist.gypi
index ab5597d..2db4941 100644
--- a/chrome_elf/blacklist.gypi
+++ b/chrome_elf/blacklist.gypi
@@ -6,6 +6,9 @@
     {
       'target_name': 'blacklist',
       'type': 'static_library',
+      'include_dirs': [
+        '<(SHARED_INTERMEDIATE_DIR)',
+      ],
       'sources': [
         'blacklist/blacklist.cc',
         'blacklist/blacklist.h',
@@ -17,7 +20,8 @@
         # as that would risk pulling in base's link-time dependencies which
         # chrome_elf cannot do.
         '../base/base.gyp:base_static',
-        '../sandbox/sandbox.gyp:sandbox',
+        '../chrome/chrome.gyp:chrome_version_header',
+        '../sandbox/sandbox.gyp:sandbox',        
       ],
     },
     {
diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
index c7d1a5d..4c742e7 100644
--- a/chrome_elf/blacklist/blacklist.cc
+++ b/chrome_elf/blacklist/blacklist.cc
@@ -12,6 +12,7 @@
 #include "sandbox/win/src/internal_types.h"
 #include "sandbox/win/src/sandbox_utils.h"
 #include "sandbox/win/src/service_resolver.h"
+#include "version.h"  // NOLINT
 
 // http://blogs.msdn.com/oldnewthing/archive/2004/10/25/247180.aspx
 extern "C" IMAGE_DOS_HEADER __ImageBase;
@@ -22,6 +23,8 @@ const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount] = {};
 int g_troublesome_dlls_cur_index = 0;
 
 const wchar_t kRegistryBeaconPath[] = L"SOFTWARE\\Google\\Chrome\\BLBeacon";
+const wchar_t kBeaconVersion[] = L"version";
+const wchar_t kBeaconState[] = L"state";
 
 }  // namespace blacklist
 
@@ -140,27 +143,58 @@ bool IsNonBrowserProcess() {
 
 namespace blacklist {
 
-bool CreateBeacon() {
-  HKEY beacon_key = NULL;
-  DWORD disposition = 0;
-  LONG result = ::RegCreateKeyEx(HKEY_CURRENT_USER,
-                                 kRegistryBeaconPath,
-                                 0,
-                                 NULL,
-                                 0,
-                                 KEY_WRITE,
-                                 NULL,
-                                 &beacon_key,
-                                 &disposition);
-  bool success = (result == ERROR_SUCCESS &&
-                  disposition != REG_OPENED_EXISTING_KEY);
-  if (result == ERROR_SUCCESS)
-    ::RegCloseKey(beacon_key);
-  return success;
+bool LeaveSetupBeacon() {
+  DWORD blacklist_state = BLACKLIST_DISABLED;
+  DWORD blacklist_state_size = sizeof(blacklist_state);
+  LONG result = ::RegGetValue(HKEY_CURRENT_USER,
+                              kRegistryBeaconPath,
+                              kBeaconState,
+                              RRF_RT_REG_DWORD,
+                              NULL,
+                              &blacklist_state,
+                              &blacklist_state_size);
+
+  if (blacklist_state != BLACKLIST_ENABLED ||
+      result != ERROR_SUCCESS)
+    return false;
+
+  // If the blacklist wasn't set as enabled for this version, don't
+  // use it.
+  wchar_t key_data[255] = {};
+  DWORD key_data_size = sizeof(key_data);
+  result = ::RegGetValue(HKEY_CURRENT_USER,
+                         blacklist::kRegistryBeaconPath,
+                         blacklist::kBeaconVersion,
+                         RRF_RT_REG_SZ,
+                         NULL,
+                         key_data,
+                         &key_data_size);
+  if (wcscmp(key_data, TEXT(CHROME_VERSION_STRING)) != 0 ||
+      result != ERROR_SUCCESS)
+    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 = ::RegSetKeyValue(HKEY_CURRENT_USER,
+                            kRegistryBeaconPath,
+                            kBeaconState,
+                            REG_DWORD,
+                            &blacklist_state,
+                            sizeof(blacklist_state));
+
+  return (result == ERROR_SUCCESS);
 }
 
-bool ClearBeacon() {
-  LONG result = ::RegDeleteKey(HKEY_CURRENT_USER, kRegistryBeaconPath);
+bool ResetBeacon() {
+  DWORD blacklist_state = BLACKLIST_ENABLED;
+  LONG result = ::RegSetKeyValue(HKEY_CURRENT_USER,
+                                 kRegistryBeaconPath,
+                                 kBeaconState,
+                                 REG_DWORD,
+                                 &blacklist_state,
+                                 sizeof(blacklist_state));
+
   return (result == ERROR_SUCCESS);
 }
 
@@ -212,7 +246,7 @@ bool Initialize(bool force) {
     return false;
 
   // Check to see if a beacon is present, abort if so.
-  if (!force && !CreateBeacon())
+  if (!force && !LeaveSetupBeacon())
     return false;
 
   // Don't try blacklisting on unsupported OS versions.
diff --git a/chrome_elf/blacklist/blacklist.h b/chrome_elf/blacklist/blacklist.h
index 5787ddd..08ca572 100644
--- a/chrome_elf/blacklist/blacklist.h
+++ b/chrome_elf/blacklist/blacklist.h
@@ -16,20 +16,36 @@ extern const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount];
 // Cursor to the current last element in the blacklist.
 extern int g_troublesome_dlls_cur_index;
 
-// The registry path of the blacklist beacon. Exposed here for testing.
+// The registry path of the blacklist beacon.
 extern const wchar_t kRegistryBeaconPath[];
 
-// Attempts to create a beacon in the current user's registry hive.
-// If the beacon already exists or any other error occurs when creating the
-// beacon, returns false. Otherwise returns true.
+// The properties for the blacklist beacon.
+extern const wchar_t kBeaconVersion[];
+extern const wchar_t kBeaconState[];
+
+// 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.
+  BLACKLIST_SETUP_RUNNING,
+  // Always keep this at the end.
+  BLACKLIST_STATE_MAX,
+};
+
+// 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.
-bool CreateBeacon();
+bool LeaveSetupBeacon();
 
-// Looks for the beacon that CreateBeacon() creates and attempts to delete it.
-// Returns true if the beacon was found and deleted.
-bool ClearBeacon();
+// Looks for the 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();
 
 // Adds the given dll name to the blacklist. Returns true if the dll name is in
 // the blacklist when this returns, false on error. Note that this will copy
diff --git a/chrome_elf/blacklist/test/blacklist_test.cc b/chrome_elf/blacklist/test/blacklist_test.cc
index dbb662f..8666a81 100644
--- a/chrome_elf/blacklist/test/blacklist_test.cc
+++ b/chrome_elf/blacklist/test/blacklist_test.cc
@@ -14,6 +14,7 @@
 #include "chrome_elf/blacklist/blacklist.h"
 #include "chrome_elf/blacklist/test/blacklist_test_main_dll.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "version.h"  // NOLINT
 
 const wchar_t kTestDllName1[] = L"blacklist_test_dll_1.dll";
 const wchar_t kTestDllName2[] = L"blacklist_test_dll_2.dll";
@@ -37,9 +38,6 @@ class BlacklistTest : public testing::Test {
   virtual void SetUp() {
     // Force an import from blacklist_test_main_dll.
     InitBlacklistTestDll();
-
-    // Ensure that the beacon state starts off cleared.
-    blacklist::ClearBeacon();
   }
 
   virtual void TearDown() {
@@ -52,17 +50,50 @@ TEST_F(BlacklistTest, Beacon) {
   registry_util::RegistryOverrideManager override_manager;
   override_manager.OverrideRegistry(HKEY_CURRENT_USER, L"beacon_test");
 
-  // First call should succeed as the beacon is newly created.
-  EXPECT_TRUE(blacklist::CreateBeacon());
-
-  // Second call should fail indicating the beacon already existed.
-  EXPECT_FALSE(blacklist::CreateBeacon());
-
-  // First call should find the beacon and delete it.
-  EXPECT_TRUE(blacklist::ClearBeacon());
-
-  // Second call should fail to find the beacon and delete it.
-  EXPECT_FALSE(blacklist::ClearBeacon());
+  // Ensure that the beacon state starts off enabled for this version.
+  DWORD blacklist_state = blacklist::BLACKLIST_ENABLED;
+  LONG result = ::RegSetKeyValue(HKEY_CURRENT_USER,
+                                 blacklist::kRegistryBeaconPath,
+                                 blacklist::kBeaconState,
+                                 REG_DWORD,
+                                 &blacklist_state,
+                                 sizeof(blacklist_state));
+  ASSERT_EQ(result, ERROR_SUCCESS);
+
+  result = ::RegSetKeyValue(HKEY_CURRENT_USER,
+                            blacklist::kRegistryBeaconPath,
+                            blacklist::kBeaconVersion,
+                            REG_SZ,
+                            TEXT(CHROME_VERSION_STRING),
+                            sizeof(TEXT(CHROME_VERSION_STRING)));
+  ASSERT_EQ(result, ERROR_SUCCESS);
+
+  // First call should find the beacon and reset it.
+  EXPECT_TRUE(blacklist::ResetBeacon());
+
+  // 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());
+
+  // Change the version and ensure that the setup fails due to the version
+  // mismatch.
+  base::string16 different_version(L"other_version");
+  ASSERT_NE(different_version, TEXT(CHROME_VERSION_STRING));
+
+  result = ::RegSetKeyValue(HKEY_CURRENT_USER,
+                            blacklist::kRegistryBeaconPath,
+                            blacklist::kBeaconVersion,
+                            REG_SZ,
+                            different_version.c_str(),
+                            sizeof(different_version));
+  ASSERT_EQ(result, ERROR_SUCCESS);
+
+  EXPECT_FALSE(blacklist::LeaveSetupBeacon());
 }
 
 TEST_F(BlacklistTest, AddAndRemoveModules) {
diff --git a/chrome_elf/chrome_elf.gyp b/chrome_elf/chrome_elf.gyp
index 7227fe6..8a4e87e 100644
--- a/chrome_elf/chrome_elf.gyp
+++ b/chrome_elf/chrome_elf.gyp
@@ -49,6 +49,7 @@
       ],
       'include_dirs': [
         '..',
+        '<(SHARED_INTERMEDIATE_DIR)',
       ],
       'dependencies': [
         'chrome_elf_lib',
diff --git a/chrome_elf/chrome_elf_main.cc b/chrome_elf/chrome_elf_main.cc
index 9ad8299..772c906 100644
--- a/chrome_elf/chrome_elf_main.cc
+++ b/chrome_elf/chrome_elf_main.cc
@@ -10,7 +10,7 @@
 #include "chrome_elf/ntdll_cache.h"
 
 void SignalChromeElf() {
-  blacklist::ClearBeacon();
+  blacklist::ResetBeacon();
 }
 
 BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {