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

Index: chrome/browser/metrics/variations/variations_service_unittest.cc
diff --git a/chrome/browser/metrics/variations/variations_service_unittest.cc b/chrome/browser/metrics/variations/variations_service_unittest.cc
index 0de0800342379ff326e0489f17d10e0fd2ac7ad0..e624b6014d6a7be19910ad88aa7fc621e1947758 100644
--- a/chrome/browser/metrics/variations/variations_service_unittest.cc
+++ b/chrome/browser/metrics/variations/variations_service_unittest.cc
@@ -5,9 +5,11 @@
 #include "base/base64.h"
 #include "base/string_split.h"
 #include "chrome/browser/metrics/proto/study.pb.h"
+#include "chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.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"
@@ -19,50 +21,41 @@ namespace {
 // A test class used to validate expected functionality in VariationsService.
 class TestVariationsService : public VariationsService {
  public:
-  TestVariationsService() : VariationsService(),
-                            fetch_attempted_(false) {
-  }
-  virtual ~TestVariationsService() {}
+  // Note that |test_notifier| may be NULL if the test does not care to perform
+  // mock operations on the notifier.

[Alexei Svitkine (slow), 2012/09/21 15:52:05]

Is the comment about the test_notifier true? I notice none of your tests
actually pass in NULL.

[SteveT, 2012/09/24 15:38:38]

Sorry - I had tried this earlier before taking your advice to directly pass in a
new TestRRAN for tests that do not care. It didn't work but I forgot to remove
this comment.

Remove this comment.

+  explicit TestVariationsService(TestRequestAllowedNotifier* test_notifier);
+  virtual ~TestVariationsService();
 
   bool fetch_attempted() const { return fetch_attempted_; }
-  void SetFetchAttempted(bool attempted) { fetch_attempted_ = attempted; }
+
+  void SetRequestsAllowed(bool allowed);

[Alexei Svitkine (slow), 2012/09/21 15:52:05]

Remove.

[SteveT, 2012/09/24 15:38:38]

Done.

+
+  void NotifyObservers();

[Alexei Svitkine (slow), 2012/09/21 15:52:05]

Remove.

[SteveT, 2012/09/24 15:38:38]

Done.

 
  protected:
-  virtual void FetchVariationsSeed() OVERRIDE {
+  virtual void DoActualFetch() OVERRIDE {
     fetch_attempted_ = true;
   }
 
  private:
   bool fetch_attempted_;
 
+  // Weak pointer to the base class' copy of the notifier.
+  TestRequestAllowedNotifier* test_notifier_;

[Alexei Svitkine (slow), 2012/09/21 15:52:05]

Remove.

[SteveT, 2012/09/24 15:38:38]

Done.

+
   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) {
-  }
-
-  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_;
+TestVariationsService::TestVariationsService(
+    TestRequestAllowedNotifier* test_notifier)
+    : VariationsService(test_notifier),
+      fetch_attempted_(false) {
+  // Set this so StartRepeatedVariationsSeedFetch can be called in tests.
+  SetCreateTrialsFromSeedCalledForTesting(true);
+}

[Alexei Svitkine (slow), 2012/09/21 15:52:05]

Nit: Inline this into the class declaration above.

[SteveT, 2012/09/24 15:38:38]

Done.

 
-  DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeNotifier);
-};
+TestVariationsService::~TestVariationsService() {

[Alexei Svitkine (slow), 2012/09/21 15:52:05]

Nit: Inline this into the class declaration above.

[SteveT, 2012/09/24 15:38:38]

Done.

+}
 
 // Converts |time| to Study proto format.
 int64 TimeToProtoTime(const base::Time& time) {
@@ -86,36 +79,6 @@ TrialsSeed CreateTestSeed() {
 
 }  // namespace
 
-// A test fixture class for VariationsService tests that require network state
-// simulations.
-class VariationsServiceNetworkTest : public testing::Test {
- public:
-  VariationsServiceNetworkTest()
-    : ui_thread(content::BrowserThread::UI, &message_loop) { }
-  ~VariationsServiceNetworkTest() { }
-
-  void SetWasOfflineDuringLastRequestAttempt(bool offline) {
-    test_service.SetWasOfflineDuringLastRequestAttemptForTesting(offline);
-  }
-
-  void SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::ConnectionType type) {
-    notifier.SimulateNetworkConnectionChange(type);
-  }
-
-  bool fetch_attempted() const {
-    return test_service.fetch_attempted();
-  }
-
- private:
-  MessageLoopForUI message_loop;
-  content::TestBrowserThread ui_thread;
-  TestNetworkChangeNotifier notifier;
-  TestVariationsService test_service;
-
-  DISALLOW_COPY_AND_ASSIGN(VariationsServiceNetworkTest);
-};
-
 TEST(VariationsServiceTest, CheckStudyChannel) {
   const chrome::VersionInfo::Channel channels[] = {
     chrome::VersionInfo::CHANNEL_CANARY,
@@ -401,7 +364,7 @@ TEST(VariationsServiceTest, LoadSeed) {
   ASSERT_TRUE(base::Base64Encode(serialized_seed, &base64_serialized_seed));
   pref_service.SetString(prefs::kVariationsSeed, base64_serialized_seed);
 
-  VariationsService variations_service;
+  TestVariationsService variations_service(new TestRequestAllowedNotifier);
   TrialsSeed loaded_seed;
   EXPECT_TRUE(
       variations_service.LoadTrialsSeedFromPref(&pref_service, &loaded_seed));
@@ -435,7 +398,7 @@ TEST(VariationsServiceTest, StoreSeed) {
 
   TrialsSeed seed = CreateTestSeed();
 
-  VariationsService variations_service;
+  TestVariationsService variations_service(new TestRequestAllowedNotifier);
   std::string serialized_seed;
   seed.SerializeToString(&serialized_seed);
   EXPECT_TRUE(
@@ -534,53 +497,20 @@ TEST(VariationsServiceTest, ValidateStudy) {
   EXPECT_FALSE(valid);
 }
 
-TEST_F(VariationsServiceNetworkTest, DoNotFetchIfOffline) {
-  SetWasOfflineDuringLastRequestAttempt(true);
-  SimulateNetworkConnectionChange(net::NetworkChangeNotifier::CONNECTION_NONE);
-  EXPECT_FALSE(fetch_attempted());
-}
-
-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());
+TEST(VariationsServiceTest, ResourceRequestAllowedNotifierTest) {
+  MessageLoopForUI message_loop;
+  content::TestBrowserThread ui_thread(content::BrowserThread::UI,
+                                       &message_loop);
+
+  // Pass ownership to TestVariationsService, but keep a weak pointer to
+  // manipulate it for this test.
+  TestRequestAllowedNotifier* test_notifier = new TestRequestAllowedNotifier;
+  TestVariationsService test_service(test_notifier);
+  test_notifier->SetRequestsAllowed(false);
+  test_service.StartRepeatedVariationsSeedFetch();
+  EXPECT_FALSE(test_service.fetch_attempted());
+  test_notifier->NotifyObservers();
+  EXPECT_TRUE(test_service.fetch_attempted());
 }
 
 }  // namespace chrome_variations