Revert 243967 "Use a Finch Experiment to control the Browser Bla..."

> 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

TBR=csharp@chromium.org

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

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

6 files changed, 44 insertions, 130 deletions

diff --git a/chrome_elf/blacklist.gypi b/chrome_elf/blacklist.gypi
index 2db4941..ab5597d 100644
--- a/chrome_elf/blacklist.gypi
+++ b/chrome_elf/blacklist.gypi
@@ -6,9 +6,6 @@
     {
       'target_name': 'blacklist',
       'type': 'static_library',
-      'include_dirs': [
-        '<(SHARED_INTERMEDIATE_DIR)',
-      ],
       'sources': [
         'blacklist/blacklist.cc',
         'blacklist/blacklist.h',
@@ -20,8 +17,7 @@
         # as that would risk pulling in base's link-time dependencies which
         # chrome_elf cannot do.
         '../base/base.gyp:base_static',
-        '../chrome/chrome.gyp:chrome_version_header',
-        '../sandbox/sandbox.gyp:sandbox',        
+        '../sandbox/sandbox.gyp:sandbox',
       ],
     },
     {
diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
index 4c742e7..c7d1a5d 100644
--- a/chrome_elf/blacklist/blacklist.cc
+++ b/chrome_elf/blacklist/blacklist.cc
@@ -12,7 +12,6 @@
 #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;
@@ -23,8 +22,6 @@ 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
 
@@ -143,58 +140,27 @@ bool IsNonBrowserProcess() {
 
 namespace blacklist {
 
-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 ResetBeacon() {
-  DWORD blacklist_state = BLACKLIST_ENABLED;
-  LONG result = ::RegSetKeyValue(HKEY_CURRENT_USER,
+bool CreateBeacon() {
+  HKEY beacon_key = NULL;
+  DWORD disposition = 0;
+  LONG result = ::RegCreateKeyEx(HKEY_CURRENT_USER,
                                  kRegistryBeaconPath,
-                                 kBeaconState,
-                                 REG_DWORD,
-                                 &blacklist_state,
-                                 sizeof(blacklist_state));
+                                 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 ClearBeacon() {
+  LONG result = ::RegDeleteKey(HKEY_CURRENT_USER, kRegistryBeaconPath);
   return (result == ERROR_SUCCESS);
 }
 
@@ -246,7 +212,7 @@ bool Initialize(bool force) {
     return false;
 
   // Check to see if a beacon is present, abort if so.
-  if (!force && !LeaveSetupBeacon())
+  if (!force && !CreateBeacon())
     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 08ca572..5787ddd 100644
--- a/chrome_elf/blacklist/blacklist.h
+++ b/chrome_elf/blacklist/blacklist.h
@@ -16,36 +16,20 @@ 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.
+// The registry path of the blacklist beacon. Exposed here for testing.
 extern const wchar_t kRegistryBeaconPath[];
 
-// 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.
+// 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 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 LeaveSetupBeacon();
+bool CreateBeacon();
 
-// 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();
+// Looks for the beacon that CreateBeacon() creates and attempts to delete it.
+// Returns true if the beacon was found and deleted.
+bool ClearBeacon();
 
 // 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 8666a81..dbb662f 100644
--- a/chrome_elf/blacklist/test/blacklist_test.cc
+++ b/chrome_elf/blacklist/test/blacklist_test.cc
@@ -14,7 +14,6 @@
 #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";
@@ -38,6 +37,9 @@ 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() {
@@ -50,50 +52,17 @@ TEST_F(BlacklistTest, Beacon) {
   registry_util::RegistryOverrideManager override_manager;
   override_manager.OverrideRegistry(HKEY_CURRENT_USER, L"beacon_test");
 
-  // 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());
+  // 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());
 }
 
 TEST_F(BlacklistTest, AddAndRemoveModules) {
diff --git a/chrome_elf/chrome_elf.gyp b/chrome_elf/chrome_elf.gyp
index 8a4e87e..7227fe6 100644
--- a/chrome_elf/chrome_elf.gyp
+++ b/chrome_elf/chrome_elf.gyp
@@ -49,7 +49,6 @@
       ],
       '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 772c906..9ad8299 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::ResetBeacon();
+  blacklist::ClearBeacon();
 }
 
 BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {