Activate the VariationsService for ChromeOS and ensure that it does not ping the server until the E…

chrome/browser/metrics/variations/variations_service_unittest.cc

 // Copyright (c) 2012 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/base64.h"
 #include "base/string_split.h"
 #include "chrome/browser/metrics/proto/study.pb.h"
 #include "chrome/browser/metrics/variations/variations_service.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_pref_service.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chrome_variations {
 
 namespace {
 
+class TestRequestAllowedNotifier : public ResourceRequestAllowedNotifier {
+ public:
+  TestRequestAllowedNotifier();
+  virtual ~TestRequestAllowedNotifier();
+
+  virtual bool ResourceRequestsAllowed() OVERRIDE;
+
+  void SetRequestsAllowed(bool allowed);
+
+  // Notify observers that requests are allowed.
+  void NotifyObservers();
+
+ private:
+  bool requests_allowed_;
+};
+
+TestRequestAllowedNotifier::TestRequestAllowedNotifier()
+    : requests_allowed_(true) {
+}
+
+TestRequestAllowedNotifier::~TestRequestAllowedNotifier() {
+}
+
+bool TestRequestAllowedNotifier::ResourceRequestsAllowed() {
+  return requests_allowed_;
+}
+
+void TestRequestAllowedNotifier::SetRequestsAllowed(bool allowed) {
+  requests_allowed_ = allowed;
+}
+
+// Notify observers that requests are allowed.
+void TestRequestAllowedNotifier::NotifyObservers() {
+  requests_allowed_ = true;
+  MaybeNotifyObservers();
+}
+
 // A test class used to validate expected functionality in VariationsService.
 class TestVariationsService : public VariationsService {
  public:
-  TestVariationsService() : VariationsService(),
-                            fetch_attempted_(false) {
-  }
-  virtual ~TestVariationsService() {}
+  explicit TestVariationsService(ResourceRequestAllowedNotifier* notifier);
+  virtual ~TestVariationsService();
 
   bool fetch_attempted() const { return fetch_attempted_; }
-  void SetFetchAttempted(bool attempted) { fetch_attempted_ = attempted; }
 
  protected:
-  virtual void FetchVariationsSeed() OVERRIDE {
+  virtual void DoActualFetch() OVERRIDE {
     fetch_attempted_ = true;
   }
 
  private:
   bool fetch_attempted_;
 
   DISALLOW_COPY_AND_ASSIGN(TestVariationsService);
 };
 
-// Override NetworkChangeNotifier to simulate connection type changes for tests.
-class TestNetworkChangeNotifier : public net::NetworkChangeNotifier {
- public:
-  TestNetworkChangeNotifier()
-    : net::NetworkChangeNotifier(),
-      connection_type_to_return_(
-          net::NetworkChangeNotifier::CONNECTION_UNKNOWN) {
-  }
+TestVariationsService::TestVariationsService(
+    ResourceRequestAllowedNotifier* notifier)
+    : VariationsService(),
+      fetch_attempted_(false) {
+  SetResourceRequestAllowedNotifierForTesting(notifier);
+  // Set this so StartRepeatedVariationsSeedFetch can be called in tests.
+  create_trials_from_seed_called_ = true;
+}
 
-  void SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::ConnectionType type) {
-    connection_type_to_return_ = type;
-    net::NetworkChangeNotifier::NotifyObserversOfConnectionTypeChange();
-    MessageLoop::current()->RunAllPending();
-  }
-
- private:
-  virtual ConnectionType GetCurrentConnectionType() const OVERRIDE {
-    return connection_type_to_return_;
-  }
-
-  net::NetworkChangeNotifier::ConnectionType connection_type_to_return_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeNotifier);
-};
+TestVariationsService::~TestVariationsService() {
+}
 
 // Converts |time| to Study proto format.
 int64 TimeToProtoTime(const base::Time& time) {
   return (time - base::Time::UnixEpoch()).InSeconds();
 }
 
 // Populates |seed| with simple test data. The resulting seed will contain one
 // study called "test", which contains one experiment called "abc" with
 // probability weight 100. |seed|'s study field will be cleared before adding
 // the new study.
 TrialsSeed CreateTestSeed() {
   TrialsSeed seed;
   Study* study = seed.add_study();
   study->set_name("test");
   study->set_default_experiment_name("abc");
   Study_Experiment* experiment = study->add_experiment();
   experiment->set_name("abc");
   experiment->set_probability_weight(100);
   return seed;
 }
 
-}  // namespace
-
-// A test fixture class for VariationsService tests that require network state
-// simulations.
-class VariationsServiceNetworkTest : public testing::Test {
+// Fixture for tests that instantiate VariationsService.
+// TODO(stevet): This is not great since it creates useless overhead for non-

[SteveT, 2012/09/19 19:27:09]

REVIEWERS: Any thoughts here? I might be able to just create a
CreateVariationsTestLocalState() helper in an anon-namespace which returns a
TestingPerfService with the EulaAccepted pref already set to true (on CrOS)...
and then use that the way Jesse's original tests used it (rather than have this
fixture class).

Does that seem neater?

[Ilya Sherman, 2012/09/19 19:49:01]

Either way seems fine to me, or Alexei's suggestion also seems fine.  If you
keep this approach, a couple of nits below...

[SteveT, 2012/09/20 19:40:18]

This is gone now since we're using that virtual IsEulaAccepted method.

+// ChromeOS tests. Find a better way to handle this.
+class VariationsServiceCreatedTest : public testing::Test {
  public:
-  VariationsServiceNetworkTest()
-    : ui_thread(content::BrowserThread::UI, &message_loop) { }
-  ~VariationsServiceNetworkTest() { }
-
-  void SetWasOfflineDuringLastRequestAttempt(bool offline) {
-    test_service.SetWasOfflineDuringLastRequestAttemptForTesting(offline);
+  VariationsServiceCreatedTest()
+      : scoped_testing_local_state_(
+            static_cast<TestingBrowserProcess*>(g_browser_process)),
+        local_state_(scoped_testing_local_state_.Get()) {
   }
 
-  void SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::ConnectionType type) {
-    notifier.SimulateNetworkConnectionChange(type);
+  // Expose local_state for tests that need to touch the VariationsSeed.
+  TestingPrefService* local_state() { return local_state_; }
+
+#if defined(OS_CHROMEOS)
+  void SetUp() {

[Ilya Sherman, 2012/09/19 19:49:01]

nit: OVERRIDE

[SteveT, 2012/09/20 19:40:18]

This class has been removed.

+    // Have to set the Eula state before creating the notifier as it checks this
+    // at startup.
+    // TODO(stevet): Share this pref name with the wizard_controller file that
+    // defines it.
+    local_state_->SetUserPref("EulaAccepted", Value::CreateBooleanValue(true));

[Alexei Svitkine (slow), 2012/09/19 19:44:28]

I made a comment about this in the test for new notifier class. Ideally, you
should be able to use a test notifier here too to simulate the state you want,
rather than murking around in the prefs.

[SteveT, 2012/09/20 19:40:18]

This class has been removed.

   }
+#endif
+ private:
+  ScopedTestingLocalState scoped_testing_local_state_;
+  TestingPrefService* local_state_;

[Ilya Sherman, 2012/09/19 19:49:01]

nit: This doesn't look like it needs to be a separate member variable -- you
could just use scoped_testing_local_state_ throughout (and rename that one to
local_state_ if you get rid of this one).

[SteveT, 2012/09/20 19:40:18]

This class has been removed.

 
-  bool fetch_attempted() const {
-    return test_service.fetch_attempted();
-  }
+  DISALLOW_COPY_AND_ASSIGN(VariationsServiceCreatedTest);
+};
 
- private:
-  MessageLoopForUI message_loop;
-  content::TestBrowserThread ui_thread;
-  TestNetworkChangeNotifier notifier;
-  TestVariationsService test_service;
-
-  DISALLOW_COPY_AND_ASSIGN(VariationsServiceNetworkTest);
-};
+}  // namespace
 
 TEST(VariationsServiceTest, CheckStudyChannel) {
   const chrome::VersionInfo::Channel channels[] = {
     chrome::VersionInfo::CHANNEL_CANARY,
     chrome::VersionInfo::CHANNEL_DEV,
     chrome::VersionInfo::CHANNEL_BETA,
     chrome::VersionInfo::CHANNEL_STABLE,
   };
   const Study_Channel study_channels[] = {
     Study_Channel_CANARY,
@@ skipping 252 matching lines @@
   EXPECT_FALSE(VariationsService::IsStudyExpired(study, now));
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(expiry_test_cases); ++i) {
     study.set_expiry_date(TimeToProtoTime(expiry_test_cases[i].expiry_date));
     const bool result = VariationsService::IsStudyExpired(study, now);
     EXPECT_EQ(expiry_test_cases[i].expected_result, result)
         << "Case " << i << " failed!";
   }
 }
 
-TEST(VariationsServiceTest, LoadSeed) {
-  TestingPrefService pref_service;
-
-  VariationsService::RegisterPrefs(&pref_service);
-
+TEST_F(VariationsServiceCreatedTest, LoadSeed) {
   // Store good seed data to test if loading from prefs works.
   TrialsSeed seed = CreateTestSeed();
 
   std::string serialized_seed;
   seed.SerializeToString(&serialized_seed);
   std::string base64_serialized_seed;
   ASSERT_TRUE(base::Base64Encode(serialized_seed, &base64_serialized_seed));
-  pref_service.SetString(prefs::kVariationsSeed, base64_serialized_seed);
+  local_state()->SetString(prefs::kVariationsSeed, base64_serialized_seed);
 
   VariationsService variations_service;
   TrialsSeed loaded_seed;
   EXPECT_TRUE(
-      variations_service.LoadTrialsSeedFromPref(&pref_service, &loaded_seed));
+      variations_service.LoadTrialsSeedFromPref(local_state(), &loaded_seed));
 
   std::string serialized_loaded_seed;
   loaded_seed.SerializeToString(&serialized_loaded_seed);
   // Check that the loaded data is the same as the original.
   EXPECT_EQ(serialized_seed, serialized_loaded_seed);
   // Make sure the pref hasn't been changed.
   EXPECT_FALSE(
-      pref_service.FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
+      local_state()->FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
   EXPECT_EQ(base64_serialized_seed,
-            pref_service.GetString(prefs::kVariationsSeed));
+            local_state()->GetString(prefs::kVariationsSeed));
 
   // Check that loading a bad seed returns false and clears the pref.
-  pref_service.ClearPref(prefs::kVariationsSeed);
-  pref_service.SetString(prefs::kVariationsSeed, "this should fail");
+  local_state()->ClearPref(prefs::kVariationsSeed);
+  local_state()->SetString(prefs::kVariationsSeed, "this should fail");
   EXPECT_FALSE(
-      pref_service.FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
+      local_state()->FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
   EXPECT_FALSE(
-      variations_service.LoadTrialsSeedFromPref(&pref_service, &loaded_seed));
+      variations_service.LoadTrialsSeedFromPref(local_state(), &loaded_seed));
   EXPECT_TRUE(
-      pref_service.FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
+      local_state()->FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
 }
 
-TEST(VariationsServiceTest, StoreSeed) {
-  TestingPrefService pref_service;
-
-  VariationsService::RegisterPrefs(&pref_service);
+TEST_F(VariationsServiceCreatedTest, StoreSeed) {
   const base::Time now = base::Time::Now();
 
   TrialsSeed seed = CreateTestSeed();
 
   VariationsService variations_service;
   std::string serialized_seed;
   seed.SerializeToString(&serialized_seed);
   EXPECT_TRUE(
-      variations_service.StoreSeedData(serialized_seed, now, &pref_service));
+      variations_service.StoreSeedData(serialized_seed, now, local_state()));
   // Make sure the pref was actually set.
   EXPECT_FALSE(
-      pref_service.FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
+      local_state()->FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
 
   std::string loaded_serialized_seed =
-      pref_service.GetString(prefs::kVariationsSeed);
+      local_state()->GetString(prefs::kVariationsSeed);
   std::string decoded_serialized_seed;
   ASSERT_TRUE(base::Base64Decode(loaded_serialized_seed,
                                  &decoded_serialized_seed));
   // Make sure the stored seed from pref is the same as the seed we created.
   EXPECT_EQ(serialized_seed, decoded_serialized_seed);
 
   // Check if trying to store a bad seed leaves the pref unchanged.
-  pref_service.ClearPref(prefs::kVariationsSeed);
+  local_state()->ClearPref(prefs::kVariationsSeed);
   EXPECT_FALSE(
-      variations_service.StoreSeedData("this should fail", now, &pref_service));
+      variations_service.StoreSeedData("this should fail", now, local_state()));
   EXPECT_TRUE(
-      pref_service.FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
+      local_state()->FindPreference(prefs::kVariationsSeed)->IsDefaultValue());
 }
 
 TEST(VariationsServiceTest, ValidateStudy) {
   Study study;
   study.set_default_experiment_name("def");
 
   Study_Experiment* experiment = study.add_experiment();
   experiment->set_name("abc");
   experiment->set_probability_weight(100);
 
@@ skipping 56 matching lines @@
       &total_probability);
   ASSERT_TRUE(valid);
   Study_Experiment* repeated_group = study.add_experiment();
   repeated_group->set_name("abc");
   repeated_group->set_probability_weight(1);
   valid = VariationsService::ValidateStudyAndComputeTotalProbability(study,
       &total_probability);
   EXPECT_FALSE(valid);
 }
 
-TEST_F(VariationsServiceNetworkTest, DoNotFetchIfOffline) {
-  SetWasOfflineDuringLastRequestAttempt(true);
-  SimulateNetworkConnectionChange(net::NetworkChangeNotifier::CONNECTION_NONE);
-  EXPECT_FALSE(fetch_attempted());
-}
+TEST_F(VariationsServiceCreatedTest, ResourceRequestAllowedNotifierTest) {
+  MessageLoopForUI message_loop;
+  content::TestBrowserThread ui_thread(content::BrowserThread::UI,
+                                       &message_loop);
 
-TEST_F(VariationsServiceNetworkTest, DoNotFetchIfOnlineToOnline) {
-  SetWasOfflineDuringLastRequestAttempt(false);
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  EXPECT_FALSE(fetch_attempted());
-}
-
-TEST_F(VariationsServiceNetworkTest, FetchOnReconnect) {
-  SetWasOfflineDuringLastRequestAttempt(true);
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  EXPECT_TRUE(fetch_attempted());
-}
-
-TEST_F(VariationsServiceNetworkTest, NoFetchOnWardriving) {
-  SetWasOfflineDuringLastRequestAttempt(false);
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_WIFI);
-  EXPECT_FALSE(fetch_attempted());
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_3G);
-  EXPECT_FALSE(fetch_attempted());
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_4G);
-  EXPECT_FALSE(fetch_attempted());
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_WIFI);
-  EXPECT_FALSE(fetch_attempted());
-}
-
-TEST_F(VariationsServiceNetworkTest, NoFetchOnFlakyConnection) {
-  SetWasOfflineDuringLastRequestAttempt(false);
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_WIFI);
-  EXPECT_FALSE(fetch_attempted());
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_NONE);
-  EXPECT_FALSE(fetch_attempted());
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_WIFI);
-  EXPECT_FALSE(fetch_attempted());
+  TestRequestAllowedNotifier* notifier = new TestRequestAllowedNotifier;
+  // Pass ownership to TestVariationsService, but keep a weak pointer to
+  // manipulate it for this test.
+  TestVariationsService test_service(notifier);
+  notifier->SetRequestsAllowed(false);
+  test_service.StartRepeatedVariationsSeedFetch();
+  EXPECT_FALSE(test_service.fetch_attempted());
+  notifier->NotifyObservers();
+  EXPECT_TRUE(test_service.fetch_attempted());
 }
 
 }  // namespace chrome_variations