Use a Finch Experiment to control the Browser Blacklist

chrome/browser/chrome_elf_init_unittest_win.cc

Index: chrome/browser/chrome_elf_init_unittest_win.cc
diff --git a/chrome/browser/chrome_elf_init_unittest_win.cc b/chrome/browser/chrome_elf_init_unittest_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3e527b9ceaf15b0bf085efc0b462712dd31b072a
--- /dev/null
+++ b/chrome/browser/chrome_elf_init_unittest_win.cc
@@ -0,0 +1,118 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/basictypes.h"
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/test_reg_util_win.h"
+#include "chrome/browser/chrome_elf_init_win.h"

[Alexei Svitkine (slow), 2014/01/06 20:43:05]

This should be the first include. (For a while both ways were used for tests,
but now the best practice is to put this first in the test, supported by the
style guide.)

[csharp, 2014/01/07 20:04:02]

Done.

+#include "chrome/common/chrome_version_info.h"
+#include "chrome_elf/blacklist/blacklist.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ChromeBlacklistTrialTest : public testing::Test {
+ protected:
+  ChromeBlacklistTrialTest() : blacklist_registry_key_(
+                                   HKEY_CURRENT_USER,
+                                   blacklist::kRegistryBeaconPath,
+                                   KEY_QUERY_VALUE | KEY_SET_VALUE) {}
+
+  virtual void SetUp() {

[Alexei Svitkine (slow), 2014/01/06 20:43:05]

Nit: OVERRIDE

[csharp, 2014/01/07 20:04:02]

Done.

+    testing::Test::SetUp();
+
+    registry_util::RegistryOverrideManager override_manager;
+    override_manager.OverrideRegistry(HKEY_CURRENT_USER,
+                                      L"browser_blacklist_test");
+
+
+  }
+
+  DWORD GetBlacklistState() {
+    DWORD blacklist_state = 0;
+    blacklist_registry_key_.ReadValueDW(blacklist::kBeaconState,
+                                        &blacklist_state);
+
+    return blacklist_state;
+  }
+
+  base::string16 GetBlacklistVersion() {
+    base::string16 blacklist_version;
+    blacklist_registry_key_.ReadValue(blacklist::kBeaconVersion,
+                                      &blacklist_version);
+
+    return blacklist_version;
+  }
+
+  base::win::RegKey blacklist_registry_key_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ChromeBlacklistTrialTest);
+};
+
+
+// Ensure that the default trial deletes any existing blacklist beacons.
+TEST_F(ChromeBlacklistTrialTest, DefaultRun) {
+  // Set some dummy values as beacons.
+  blacklist_registry_key_.WriteValue(blacklist::kBeaconState,
+                                     blacklist::BLACKLIST_ENABLED);
+  blacklist_registry_key_.WriteValue(blacklist::kBeaconVersion, L"Data");
+
+  // This setup code should result in the default group, which should remove
+  // all the beacon values.
+  InitializeChromeElf();
+
+  // Ensure that all the beacon values are gone.
+  DWORD state = blacklist::BLACKLIST_STATE_MAX;
+  ASSERT_NE(ERROR_SUCCESS,
+            blacklist_registry_key_.ReadValueDW(blacklist::kBeaconState,

[Alexei Svitkine (slow), 2014/01/06 20:43:05]

Why do you both have helpers in the test class (GetBlacklistState()) while using
the key directly. Either do it one way consistently or the other. Also, the
helper functions don't seem to be checking the return values.

[csharp, 2014/01/07 20:04:02]

Modified this code to use the test helpers.
The test helpers still don't directly check for the return value (ASSERT_EQ
can't be compiled in non-void helper functions), but they will return either an
empty string or an invalid enum (which should be good enough signals that the
get failed).

+                                                &state));
+  ASSERT_EQ(blacklist::BLACKLIST_STATE_MAX, state);
+
+  base::string16 version;
+  ASSERT_NE(ERROR_SUCCESS,
+            blacklist_registry_key_.ReadValue(blacklist::kBeaconVersion,
+                                              &version));
+  ASSERT_EQ(L"", version);
+}
+
+TEST_F(ChromeBlacklistTrialTest, VerifyFirstRun) {
+  BrowserBlacklistBeaconSetup();
+
+  // Verify the state is properly set after the first run.
+  ASSERT_EQ(blacklist::BLACKLIST_ENABLED, GetBlacklistState());
+
+  chrome::VersionInfo version_info;
+  base::string16 version(base::UTF8ToUTF16(version_info.Version()));
+  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.
+  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, VersionChanged) {
+  // Mark the blacklist as disabled for an older version, so it should
+  // get enabled for this new version.
+  blacklist_registry_key_.WriteValue(blacklist::kBeaconVersion,
+                                     L"old_version");
+  blacklist_registry_key_.WriteValue(blacklist::kBeaconState,
+                                     blacklist::BLACKLIST_DISABLED);
+
+  BrowserBlacklistBeaconSetup();
+
+  // The beacon should now be marked as enabled for the current version.
+  ASSERT_EQ(blacklist::BLACKLIST_ENABLED, GetBlacklistState());
+
+  chrome::VersionInfo version_info;
+  base::string16 expected_version(base::UTF8ToUTF16(version_info.Version()));
+  ASSERT_EQ(expected_version, GetBlacklistVersion());
+}