Adding unit tests to RLZ code. Refactoring RLZ code to make it more testable.

chrome/browser/rlz/rlz_unittest.cc

Index: chrome/browser/rlz/rlz_unittest.cc
===================================================================
--- chrome/browser/rlz/rlz_unittest.cc	(revision 98233)
+++ chrome/browser/rlz/rlz_unittest.cc	(working copy)
@@ -4,48 +4,548 @@
 
 #include "chrome/browser/rlz/rlz.h"
 
+#include "base/memory/scoped_ptr.h"
+#include "base/stringprintf.h"
 #include "base/path_service.h"
+#include "base/test/test_reg_util_win.h"
+#include "base/utf_string_conversions.h"
 #include "base/win/registry.h"
+#include "chrome/browser/autocomplete/autocomplete.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/rlz/rlz.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/env_vars.h"
+#include "chrome/test/base/testing_browser_process_test.h"
+#include "content/browser/tab_contents/navigation_entry.h"
+#include "content/common/notification_details.h"
+#include "content/common/notification_source.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::win::RegKey;
+using registry_util::RegistryOverrideManager;
+using testing::AssertionResult;
+using testing::AssertionSuccess;
+using testing::AssertionFailure;
 
 namespace {
 
-// Gets rid of registry leftovers from testing. Returns false if there
-// is nothing to clean.
-bool CleanValue(const wchar_t* key_name, const wchar_t* value) {
-  RegKey key;
-  if (key.Open(HKEY_CURRENT_USER, key_name, KEY_READ | KEY_WRITE) !=
-      ERROR_SUCCESS)
-    return false;
-  EXPECT_EQ(ERROR_SUCCESS, key.DeleteValue(value));
-  return true;
+// Registry path to overridden hive.
+const wchar_t kRlzTempHkcu[] = L"rlz_hkcu";
+const wchar_t kRlzTempHklm[] = L"rlz_hklm";
+
+// Dummy RLZ string for the access points.
+const char kOmniboxRlzString[] = "test_omnibox";
+const char kHomepageRlzString[] = "test_homepage";
+
+// Some helper macros to test it a string contains/does not contain a substring.
+
+AssertionResult CmpHelperSTRC(const char* str_expression,
+                              const char* substr_expression,
+                              const char* str,
+                              const char* substr) {
+  if (NULL != strstr(str, substr)) {
+    return AssertionSuccess();
+  }
+
+  return AssertionFailure() << "Expected: (" << substr_expression << ") in ("
+                            << str_expression << "), actual: '"
+                            << substr << "' not in '" << str << "'";
 }
 
-// The chrome events RLZ key lives here.
-const wchar_t kKeyName[] = L"Software\\Google\\Common\\Rlz\\Events\\C";
+AssertionResult CmpHelperSTRNC(const char* str_expression,
+                               const char* substr_expression,
+                               const char* str,
+                               const char* substr) {
+  if (NULL == strstr(str, substr)) {
+    return AssertionSuccess();
+  }
 
+  return AssertionFailure() << "Expected: (" << substr_expression
+                            << ") not in (" << str_expression << "), actual: '"
+                            << substr << "' in '" << str << "'";
+}
+
+#define EXPECT_STR_CONTAINS(str, substr) \
+    EXPECT_PRED_FORMAT2(CmpHelperSTRC, str, substr)
+
+#define EXPECT_STR_NOT_CONTAIN(str, substr) \
+    EXPECT_PRED_FORMAT2(CmpHelperSTRNC, str, substr)
+
 }  // namespace
 
-TEST(RlzLibTest, RecordProductEvent) {
-  DWORD recorded_value = 0;
-  EXPECT_TRUE(RLZTracker::RecordProductEvent(rlz_lib::CHROME,
-      rlz_lib::CHROME_OMNIBOX, rlz_lib::FIRST_SEARCH));
-  const wchar_t kEvent1[] = L"C1F";
-  RegKey key1;
-  EXPECT_EQ(ERROR_SUCCESS, key1.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ));
-  EXPECT_EQ(ERROR_SUCCESS, key1.ReadValueDW(kEvent1, &recorded_value));
-  EXPECT_EQ(1, recorded_value);
-  EXPECT_TRUE(CleanValue(kKeyName, kEvent1));
+// Test class for RLZ tracker.  Makes some member functions public and
+// overrides others to make it easier to test.
+class TestRLZTracker : public RLZTracker {
+ public:
+  using RLZTracker::DelayedInit;
+  using RLZTracker::Observe;
+  using RLZTracker::RLZ_PAGETRANSITION_HOME_PAGE;
 
-  EXPECT_TRUE(RLZTracker::RecordProductEvent(rlz_lib::CHROME,
-      rlz_lib::CHROME_HOME_PAGE, rlz_lib::SET_TO_GOOGLE));
-  const wchar_t kEvent2[] = L"C2S";
-  RegKey key2;
-  EXPECT_EQ(ERROR_SUCCESS, key2.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ));
-  DWORD value = 0;
-  EXPECT_EQ(ERROR_SUCCESS, key2.ReadValueDW(kEvent2, &recorded_value));
-  EXPECT_EQ(1, recorded_value);
-  EXPECT_TRUE(CleanValue(kKeyName, kEvent2));
+  TestRLZTracker() : pingnow_called_(false), assume_io_thread_(false) {
+    set_tracker(this);
+  }
+
+  virtual ~TestRLZTracker() {
+    set_tracker(NULL);
+  }
+
+  bool pingnow_called() {
+    return pingnow_called_;
+  }
+
+  bool assume_io_thread() {
+    return assume_io_thread_;
+  }
+
+  void set_assume_io_thread(bool assume_io_thread) {
+    assume_io_thread_ = assume_io_thread;
+  }
+
+ private:
+  virtual void ScheduleDelayedInit(int delay) OVERRIDE {
+    // If the delay is 0, invoke the delayed init now.  Otherwise,
+    // don't schedule anything, it will be manually called during tests.
+    if (delay == 0)
+      DelayedInit();
+  }
+
+  virtual void ScheduleFinancialPing() OVERRIDE {
+    PingNow(this);
+  }
+
+  virtual bool ScheduleGetAccessPointRlz(rlz_lib::AccessPoint point) OVERRIDE {
+    return !assume_io_thread_;
+  }
+
+  virtual bool SendFinancialPing(const std::wstring& brand,
+                                 const std::wstring& lang,
+                                 const std::wstring& referral,
+                                 bool exclude_id) OVERRIDE {
+    // Don't ping the server during tests.
+    pingnow_called_ = true;
+    return true;
+  }
+
+  bool pingnow_called_;
+  bool assume_io_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestRLZTracker);
+};
+
+class RlzLibTest : public TestingBrowserProcessTest {
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+ protected:
+  RlzLibTest();
+  ~RlzLibTest();
+
+  void SimulateOmniboxUsage();
+  void SimulateHomepageUsage();
+  void InvokeDelayedInit();
+
+  void ExpectEventRecorded(const char* event_name, bool expected);
+  void ExpectRlzPingSent(bool expected);
+
+  TestRLZTracker tracker_;
+  RegistryOverrideManager override_manager_;
+  wchar_t env_var_[64];
+};
+
+RlzLibTest::RlzLibTest() {
 }
+
+RlzLibTest::~RlzLibTest() {
+}
+
+void RlzLibTest::SetUp() {
+  TestingBrowserProcessTest::SetUp();
+
+  // Make sure HEADLESS environment variable is not set.
+  env_var_[0] = 0;
+  DWORD size = ::GetEnvironmentVariableW(
+      ASCIIToWide(env_vars::kHeadless).c_str(), env_var_, arraysize(env_var_));
+  if (size == 0)
+    ASSERT_EQ(ERROR_ENVVAR_NOT_FOUND, ::GetLastError());
+  ASSERT_LT(size, arraysize(env_var_));
+
+  ::SetEnvironmentVariable(ASCIIToWide(env_vars::kHeadless).c_str(), NULL);
+
+  // Before overriding HKLM for the tests, we need to set it up correctly
+  // so that the rlz_lib calls work.  This needs to be done before we do the
+  // override.
+
+  std::wstring temp_hklm_path = base::StringPrintf(
+      L"%ls\\%ls",
+      RegistryOverrideManager::kTempTestKeyPath,
+      kRlzTempHklm);
+
+  base::win::RegKey hklm;
+  ASSERT_EQ(ERROR_SUCCESS, hklm.Create(HKEY_CURRENT_USER,
+                                       temp_hklm_path.c_str(),
+                                       KEY_READ));
+
+  std::wstring temp_hkcu_path = base::StringPrintf(
+      L"%ls\\%ls",
+      RegistryOverrideManager::kTempTestKeyPath,
+      kRlzTempHkcu);
+
+  base::win::RegKey hkcu;
+  ASSERT_EQ(ERROR_SUCCESS, hkcu.Create(HKEY_CURRENT_USER,
+                                       temp_hkcu_path.c_str(),
+                                       KEY_READ));
+
+  rlz_lib::InitializeTempHivesForTesting(hklm, hkcu);
+
+  // Its important to override HKLM before HKCU because of the registry
+  // initialization performed above.
+  override_manager_.OverrideRegistry(HKEY_LOCAL_MACHINE, kRlzTempHklm);
+  override_manager_.OverrideRegistry(HKEY_CURRENT_USER, kRlzTempHkcu);
+}
+
+void RlzLibTest::TearDown() {
+  TestingBrowserProcessTest::TearDown();
+
+  if (env_var_[0] != 0)
+    ::SetEnvironmentVariable(ASCIIToWide(env_vars::kHeadless).c_str(),
+                             env_var_);
+}
+
+void RlzLibTest::SimulateOmniboxUsage() {
+  tracker_.Observe(chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
+                   NotificationService::AllSources(),
+                   Details<AutocompleteLog>(NULL));
+}
+
+void RlzLibTest::SimulateHomepageUsage() {
+  NavigationEntry entry(NULL, 0, GURL(), GURL(), string16(),
+                        TestRLZTracker::RLZ_PAGETRANSITION_HOME_PAGE);
+  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_PENDING,
+                   NotificationService::AllSources(),
+                   Details<NavigationEntry>(&entry));
+}
+
+void RlzLibTest::InvokeDelayedInit() {
+  tracker_.DelayedInit();
+}
+
+void RlzLibTest::ExpectEventRecorded(const char* event_name, bool expected) {
+  char cgi[rlz_lib::kMaxCgiLength];
+  GetProductEventsAsCgi(rlz_lib::CHROME, cgi, arraysize(cgi));
+  if (expected) {
+    EXPECT_STR_CONTAINS(cgi, event_name);
+  } else {
+    EXPECT_STR_NOT_CONTAIN(cgi, event_name);
+  }
+}
+
+void RlzLibTest::ExpectRlzPingSent(bool expected) {
+  EXPECT_EQ(expected, tracker_.pingnow_called());
+}
+
+TEST_F(RlzLibTest, RecordProductEvent) {
+  RLZTracker::RecordProductEvent(rlz_lib::CHROME, rlz_lib::CHROME_OMNIBOX,
+    rlz_lib::FIRST_SEARCH);
+
+  ExpectEventRecorded("C1F", true);
+}
+
+// The events that affect the different RLZ scenarios are the following:
+//
+//  A: the user starts chrome for the first time
+//  B: the user stops chrome
+//  C: the user start a subsequent time
+//  D: the user stops chrome again
+//  I: the RLZTracker::DelayedInit() method is invoked
+//  X: the user performs a search using the omnibox
+//  Y: the user performs a search using the home page
+//
+// The events A to D happen in chronological order, but the other events
+// may happen at any point between A-B or C-D, in no particular order.
+//
+// The visible results of the scenarios are:
+//
+//  C1I event is recorded
+//  C2I event is recorded
+//  C1F event is recorded
+//  C2F event is recorded
+//  C1S event is recorded
+//  C2S event is recorded
+//  RLZ ping sent
+//
+// Variations on the above scenarios:
+//
+//  - if the delay specified to InitRlzDelayed() is negative, then the RLZ
+//    ping should be sent out at the time of event X and not wait for I
+//
+// Also want to test that pre-warming the RLZ string cache works correctly.
+
+TEST_F(RlzLibTest, QuickStopAfterStart) {
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", false);
+  ExpectEventRecorded("C1S", false);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", false);
+  ExpectEventRecorded("C2S", false);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(false);
+}
+
+TEST_F(RlzLibTest, DelayedInitOnly) {
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+  InvokeDelayedInit();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", true);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", true);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, DelayedInitOnlyNoFirstRunNoRlzStrings) {
+  RLZTracker::InitRlzDelayed(false, 20, true, true);
+  InvokeDelayedInit();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", true);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", true);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, DelayedInitOnlyNoFirstRun) {
+  // Set some dummy RLZ strings to simulate that we already ran before and
+  // performed a successful ping to the RLZ server.
+  rlz_lib::SetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, kOmniboxRlzString);
+  rlz_lib::SetAccessPointRlz(rlz_lib::CHROME_HOME_PAGE, kHomepageRlzString);
+
+  RLZTracker::InitRlzDelayed(false, 20, true, true);
+  InvokeDelayedInit();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", false);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", false);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, DelayedInitOnlyNoGoogleDefaultSearchOrHomepage) {
+  RLZTracker::InitRlzDelayed(true, 20, false, false);
+  InvokeDelayedInit();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", false);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", false);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, OmniboxUsageOnly) {
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+  SimulateOmniboxUsage();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", false);
+  ExpectEventRecorded("C1S", false);
+  ExpectEventRecorded("C1F", true);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", false);
+  ExpectEventRecorded("C2S", false);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(false);
+}
+
+TEST_F(RlzLibTest, HomepageUsageOnly) {
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+  SimulateHomepageUsage();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", false);
+  ExpectEventRecorded("C1S", false);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", false);
+  ExpectEventRecorded("C2S", false);
+  ExpectEventRecorded("C2F", true);
+
+  ExpectRlzPingSent(false);
+}
+
+TEST_F(RlzLibTest, UsageBeforeDelayedInit) {
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+  SimulateOmniboxUsage();
+  SimulateHomepageUsage();
+  InvokeDelayedInit();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", true);
+  ExpectEventRecorded("C1F", true);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", true);
+  ExpectEventRecorded("C2F", true);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, OmniboxUsageAfterDelayedInit) {
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+  InvokeDelayedInit();
+  SimulateOmniboxUsage();
+  SimulateHomepageUsage();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", true);
+  ExpectEventRecorded("C1F", true);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", true);
+  ExpectEventRecorded("C2F", true);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, OmniboxUsageSendsPingWhenDelayNegative) {
+  RLZTracker::InitRlzDelayed(true, -20, true, true);
+  SimulateOmniboxUsage();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", true);
+  ExpectEventRecorded("C1S", true);
+  ExpectEventRecorded("C1F", true);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", true);
+  ExpectEventRecorded("C2S", true);
+  ExpectEventRecorded("C2F", false);
+
+  ExpectRlzPingSent(true);
+}
+
+TEST_F(RlzLibTest, HomepageUsageDoesNotSendPingWhenDelayNegative) {
+  RLZTracker::InitRlzDelayed(true, -20, true, true);
+  SimulateHomepageUsage();
+
+  // Omnibox events.
+  ExpectEventRecorded("C1I", false);
+  ExpectEventRecorded("C1S", false);
+  ExpectEventRecorded("C1F", false);
+
+  // Home page events.
+  ExpectEventRecorded("C2I", false);
+  ExpectEventRecorded("C2S", false);
+  ExpectEventRecorded("C2F", true);
+
+  ExpectRlzPingSent(false);
+}
+
+TEST_F(RlzLibTest, GetAccessPointRlzOnIoThread) {
+  // Set dummy RLZ string.
+  rlz_lib::SetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, kOmniboxRlzString);
+
+  std::wstring rlz;
+
+  tracker_.set_assume_io_thread(true);
+  EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+  EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str());
+}
+
+TEST_F(RlzLibTest, GetAccessPointRlzNotOnIoThread) {
+  // Set dummy RLZ string.
+  rlz_lib::SetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, kOmniboxRlzString);
+
+  std::wstring rlz;
+
+  tracker_.set_assume_io_thread(false);
+  EXPECT_FALSE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+}
+
+TEST_F(RlzLibTest, GetAccessPointRlzIsCached) {
+  // Set dummy RLZ string.
+  rlz_lib::SetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, kOmniboxRlzString);
+
+  std::wstring rlz;
+
+  tracker_.set_assume_io_thread(false);
+  EXPECT_FALSE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+
+  tracker_.set_assume_io_thread(true);
+  EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+  EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str());
+
+  tracker_.set_assume_io_thread(false);
+  EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+  EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str());
+}
+
+TEST_F(RlzLibTest, PingInvalidatesRlzCache) {
+  // Set dummy RLZ string.
+  rlz_lib::SetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, kOmniboxRlzString);
+
+  std::wstring rlz;
+
+  // Prime the cache.
+  tracker_.set_assume_io_thread(true);
+  EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+  EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str());
+
+  // Make sure cache is valid.
+  tracker_.set_assume_io_thread(false);
+  EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+  EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str());
+
+  // Perform ping.
+  RLZTracker::InitRlzDelayed(true, 20, true, true);
+  InvokeDelayedInit();
+  ExpectRlzPingSent(true);
+
+  // Make sure cache is now invalid.
+  EXPECT_FALSE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz));
+}
+
+TEST_F(RlzLibTest, ObserveHandlesBadArgs) {
+  NavigationEntry entry(NULL, 0, GURL(), GURL(), string16(),
+                        PageTransition::LINK);
+  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_PENDING,
+                   NotificationService::AllSources(),
+                   Details<NavigationEntry>(NULL));
+  tracker_.Observe(content::NOTIFICATION_NAV_ENTRY_PENDING,
+                   NotificationService::AllSources(),
+                   Details<NavigationEntry>(&entry));
+}