Activate the VariationsService for ChromeOS and ensure that it does not ping the server until the EULA is accepted.

We accomplish this by adding an OnEulaAccepted method to the WizardControllerObserver and have the VariationsService be notified on that method when the EULA is accepted.

BUG=146865
TEST=Start CrOS on a new machine and ensure that, with a network connection, no requests are made to the variations server before the EULA is accepted. As soon as the EULA is accepted, ensure that a request is made. Also ensure that a request is made each time at startup after the first time the EULA is accepted.


Review URL: https://chromiumcodereview.appspot.com/10917120

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

8 files changed, 699 insertions, 163 deletions

diff --git a/chrome/browser/metrics/variations/resource_request_allowed_notifier.cc b/chrome/browser/metrics/variations/resource_request_allowed_notifier.cc
new file mode 100644
index 0000000..302df10
--- /dev/null
+++ b/chrome/browser/metrics/variations/resource_request_allowed_notifier.cc
@@ -0,0 +1,131 @@
+// 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 "chrome/browser/metrics/variations/resource_request_allowed_notifier.h"
+
+#include "base/metrics/histogram.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_service.h"
+
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/login/wizard_controller.h"
+#endif
+
+ResourceRequestAllowedNotifier::ResourceRequestAllowedNotifier()
+    : observer_requested_permission_(false),
+      was_waiting_for_network_(false),
+#if defined(OS_CHROMEOS)
+      was_waiting_for_user_to_accept_eula_(false),
+#endif
+      observer_(NULL) {
+}
+
+ResourceRequestAllowedNotifier::~ResourceRequestAllowedNotifier() {
+  if (observer_)
+    net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
+}
+
+void ResourceRequestAllowedNotifier::Init(Observer* observer) {
+  DCHECK(!observer_ && observer);
+  observer_ = observer;
+
+  net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
+
+  // Check this state during initialization. It is not expected to change until
+  // the corresponding notification is received.
+  was_waiting_for_network_ = net::NetworkChangeNotifier::IsOffline();
+#if defined(OS_CHROMEOS)
+  was_waiting_for_user_to_accept_eula_ = NeedsEulaAcceptance();
+  if (was_waiting_for_user_to_accept_eula_) {
+    // Note that this must listen on AllSources due to the difficulty in knowing
+    // when the WizardController instance is created, and to avoid over-coupling
+    // the Chrome OS code with the VariationsService by directly attaching as an
+    // observer. This is OK because WizardController is essentially a singleton.
+    registrar_.Add(this, chrome::NOTIFICATION_WIZARD_EULA_ACCEPTED,
+                   content::NotificationService::AllSources());
+  }
+#endif
+}
+
+bool ResourceRequestAllowedNotifier::ResourceRequestsAllowed() {
+  // The observer requested permission. Return the current criteria state and
+  // set a flag to remind this class to notify the observer once the criteria
+  // is met.
+  observer_requested_permission_ = true;
+  return
+#if defined(OS_CHROMEOS)
+      !was_waiting_for_user_to_accept_eula_ &&
+#endif
+      !was_waiting_for_network_;
+}
+
+void ResourceRequestAllowedNotifier::
+    SetWasWaitingForNetworkForTesting(bool waiting) {
+  was_waiting_for_network_ = waiting;
+}
+
+#if defined(OS_CHROMEOS)
+void ResourceRequestAllowedNotifier::
+    SetWasWaitingForEulaForTesting(bool waiting) {
+  was_waiting_for_user_to_accept_eula_ = waiting;
+}
+#endif
+
+void ResourceRequestAllowedNotifier::MaybeNotifyObserver() {
+  // Need to ensure that all criteria are met before notifying observers.
+  if (observer_requested_permission_ && ResourceRequestsAllowed()) {
+    DVLOG(1) << "Notifying observer of state change.";
+    observer_->OnResourceRequestsAllowed();
+    // Reset this so the observer is not informed again unless they check
+    // ResourceRequestsAllowed again.
+    observer_requested_permission_ = false;
+  }
+}
+
+#if defined(OS_CHROMEOS)
+bool ResourceRequestAllowedNotifier::NeedsEulaAcceptance() {
+#if defined(GOOGLE_CHROME_BUILD)
+  return !chromeos::WizardController::IsEulaAccepted();
+#else
+  // On unofficial builds, there is no notion of a EULA.
+  return false;
+#endif
+}
+#endif
+
+void ResourceRequestAllowedNotifier::OnConnectionTypeChanged(
+    net::NetworkChangeNotifier::ConnectionType type) {
+  // Only attempt to notify observers if this was previously waiting for the
+  // network to reconnect, and new network state is actually available. This
+  // prevents the notifier from notifying the observer if the notifier was never
+  // waiting on the network, or if the network changes from one online state
+  // to another (for example, Wifi to 3G, or Wifi to Wifi, if the network were
+  // flaky).
+  if (was_waiting_for_network_ &&
+      type != net::NetworkChangeNotifier::CONNECTION_NONE) {
+    was_waiting_for_network_ = false;
+    DVLOG(1) << "Network came back online.";
+    MaybeNotifyObserver();
+  }
+}
+
+#if defined(OS_CHROMEOS)
+void ResourceRequestAllowedNotifier::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_WIZARD_EULA_ACCEPTED, type);
+  // This should only ever be received once. Remove it after this call.
+  DCHECK(!registrar_.IsEmpty());
+  registrar_.Remove(this, chrome::NOTIFICATION_WIZARD_EULA_ACCEPTED,
+                    content::NotificationService::AllSources());
+
+  // This flag should have been set if this was waiting on the EULA
+  // notification.
+  DCHECK(was_waiting_for_user_to_accept_eula_);
+  DVLOG(1) << "EULA was accepted.";
+  was_waiting_for_user_to_accept_eula_ = false;
+  MaybeNotifyObserver();
+}
+#endif
diff --git a/chrome/browser/metrics/variations/resource_request_allowed_notifier.h b/chrome/browser/metrics/variations/resource_request_allowed_notifier.h
new file mode 100644
index 0000000..6a7241e
--- /dev/null
+++ b/chrome/browser/metrics/variations/resource_request_allowed_notifier.h
@@ -0,0 +1,117 @@
+// 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.
+
+#ifndef CHROME_BROWSER_METRICS_VARIATIONS_RESOURCE_REQUEST_ALLOWED_NOTIFIER_H_
+#define CHROME_BROWSER_METRICS_VARIATIONS_RESOURCE_REQUEST_ALLOWED_NOTIFIER_H_
+
+#include "base/observer_list.h"
+#include "net/base/network_change_notifier.h"
+
+#if defined(OS_CHROMEOS)
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#endif
+
+// This class informs an interested observer when resource requests over the
+// network are permitted.
+//
+// Currently, the criteria for allowing resource requests include:
+//  1. The network is currently available.
+//  2. The EULA was accepted by the user (ChromeOS only).
+//
+// Interested services should add themselves as an observer of
+// ResourceRequestAllowedNotifier and check ResourceRequestsAllowed() to see if
+// requests are permitted. If it returns true, they can go ahead and make their
+// request. If it returns false, ResourceRequestAllowedNotifier will notify the
+// service when the criteria is met.
+//
+// If ResourceRequestsAllowed returns true the first time,
+// ResourceRequestAllowedNotifier will not notify the service in the future.
+//
+// Note that this class handles the criteria state for a single service, so
+// services should keep their own instance of this class rather than sharing a
+// global instance.
+class ResourceRequestAllowedNotifier :
+#if defined(OS_CHROMEOS)
+    public content::NotificationObserver,
+#endif
+    public net::NetworkChangeNotifier::ConnectionTypeObserver {
+ public:
+  // Observes resource request allowed state changes.
+  class Observer {
+   public:
+    virtual void OnResourceRequestsAllowed() = 0;
+  };
+
+  ResourceRequestAllowedNotifier();
+  virtual ~ResourceRequestAllowedNotifier();
+
+  // Sets |observer| as the service to be notified by this instance, and
+  // performs initial checks on the criteria. |observer| may not be NULL.
+  // This is to be called immediately after construction of an instance of
+  // ResourceRequestAllowedNotifier to pass it the interested service.
+  void Init(Observer* observer);
+
+  // Returns true iff all resource request criteria are met. If not, this call
+  // will set some flags so it knows to notify the observer if the criteria
+  // changes. Note that the observer will never be notified unless it calls this
+  // method first. This is virtual so it can be overriden for tests.
+  virtual bool ResourceRequestsAllowed();
+
+  void SetWasWaitingForNetworkForTesting(bool waiting);
+#if defined(OS_CHROMEOS)
+  void SetWasWaitingForEulaForTesting(bool waiting);
+#endif
+
+ protected:
+  // Notifies the observer if all criteria needed for resource requests are met.
+  // This is protected so it can be called from subclasses for testing.
+  void MaybeNotifyObserver();
+
+#if defined(OS_CHROMEOS)
+  // On official builds, returns true iff the EULA needs to be accepted. This
+  // always returns false on unofficial builds since there is no notion of a
+  // EULA.
+  //
+  // This is virtual so it can be overriden by test classes to avoid making them
+  // aware of the ChromeOS details. This is protected so it call be overriden in
+  // subclasses for testing.
+  virtual bool NeedsEulaAcceptance();
+#endif
+
+ private:
+  // net::NetworkChangeNotifier::ConnectionTypeObserver overrides:
+  virtual void OnConnectionTypeChanged(
+      net::NetworkChangeNotifier::ConnectionType type) OVERRIDE;
+
+#if defined(OS_CHROMEOS)
+  // content::NotificationObserver overrides:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+#endif
+
+  // Tracks whether or not the observer/service depending on this class actually
+  // requested permission to make a request or not. If it did not, then this
+  // class should not notify it even if the criteria is met.
+  bool observer_requested_permission_;
+
+  // Tracks network connectivity criteria.
+  bool was_waiting_for_network_;
+
+#if defined(OS_CHROMEOS)
+  // Tracks EULA acceptance criteria.
+  bool was_waiting_for_user_to_accept_eula_;
+
+  // Used to listen for the EULA accepted notification.
+  content::NotificationRegistrar registrar_;
+#endif
+
+  // Observing service interested in request permissions.
+  Observer* observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceRequestAllowedNotifier);
+};
+
+#endif  // CHROME_BROWSER_METRICS_VARIATIONS_RESOURCE_REQUEST_ALLOWED_NOTIFIER_H_
diff --git a/chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.cc b/chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.cc
new file mode 100644
index 0000000..665cf86
--- /dev/null
+++ b/chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.cc
@@ -0,0 +1,52 @@
+// 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 "chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.h"
+
+TestRequestAllowedNotifier::TestRequestAllowedNotifier() :
+#if defined(OS_CHROMEOS)
+      needs_eula_acceptance_(true),
+#endif
+      override_requests_allowed_(false),
+      requests_allowed_(true) {
+}
+
+TestRequestAllowedNotifier::~TestRequestAllowedNotifier() {
+}
+
+#if defined(OS_CHROMEOS)
+void TestRequestAllowedNotifier::SetNeedsEulaAcceptance(bool needs_acceptance) {
+  needs_eula_acceptance_ = needs_acceptance;
+}
+#endif
+
+void TestRequestAllowedNotifier::SetRequestsAllowedOverride(bool allowed) {
+  override_requests_allowed_ = true;
+  requests_allowed_ = allowed;
+}
+
+void TestRequestAllowedNotifier::NotifyObserver() {
+  // Force the allowed state to true. This forces MaybeNotifyObserver to always
+  // notify observers, as MaybeNotifyObserver checks ResourceRequestsAllowed.
+  override_requests_allowed_ = true;
+  requests_allowed_ = true;
+  MaybeNotifyObserver();
+}
+
+bool TestRequestAllowedNotifier::ResourceRequestsAllowed() {
+  // Call ResourceRequestAllowedNotifier::ResourceRequestsAllowed once to
+  // simulate that the user requested permission. Only return that result if
+  // the override flag was set.
+  bool requests_allowed =
+      ResourceRequestAllowedNotifier::ResourceRequestsAllowed();
+  if (override_requests_allowed_)
+    return requests_allowed_;
+  return requests_allowed;
+}
+
+#if defined(OS_CHROMEOS)
+bool TestRequestAllowedNotifier::NeedsEulaAcceptance() {
+  return needs_eula_acceptance_;
+}
+#endif
diff --git a/chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.h b/chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.h
new file mode 100644
index 0000000..06222fb
--- /dev/null
+++ b/chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.h
@@ -0,0 +1,52 @@
+// 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.
+
+#ifndef CHROME_BROWSER_METRICS_VARIATIONS_RESOURCE_REQUEST_ALLOWED_NOTIFIER_TEST_UTIL_H_
+#define CHROME_BROWSER_METRICS_VARIATIONS_RESOURCE_REQUEST_ALLOWED_NOTIFIER_TEST_UTIL_H_
+
+#include "chrome/browser/metrics/variations/resource_request_allowed_notifier.h"
+
+// A subclass of ResourceRequestAllowedNotifier used to expose some
+// functionality for testing.
+//
+// By default, the constructor sets this class to override
+// ResourceRequestsAllowed, so its state can be set with SetRequestsAllowed.
+// This is meant for higher level tests of services to ensure they adhere to the
+// notifications of the ResourceRequestAllowedNotifier. Lower level tests can
+// disable this by calling SetRequestsAllowedOverride with the value they want
+// it to return.
+class TestRequestAllowedNotifier : public ResourceRequestAllowedNotifier {
+ public:
+  TestRequestAllowedNotifier();
+  virtual ~TestRequestAllowedNotifier();
+
+#if defined(OS_CHROMEOS)
+  void SetNeedsEulaAcceptance(bool needs_acceptance);
+#endif
+
+  // Makes ResourceRequestsAllowed return |allowed| when it is called.
+  void SetRequestsAllowedOverride(bool allowed);
+
+  // Notify observers that requests are allowed. This will only work if
+  // the observer is expecting a notification.
+  void NotifyObserver();
+
+  virtual bool ResourceRequestsAllowed() OVERRIDE;
+
+ protected:
+#if defined(OS_CHROMEOS)
+  virtual bool NeedsEulaAcceptance() OVERRIDE;
+#endif
+
+ private:
+#if defined(OS_CHROMEOS)
+  bool needs_eula_acceptance_;
+#endif
+  bool override_requests_allowed_;
+  bool requests_allowed_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestRequestAllowedNotifier);
+};
+
+#endif  // CHROME_BROWSER_METRICS_VARIATIONS_RESOURCE_REQUEST_ALLOWED_NOTIFIER_TEST_UTIL_H_
diff --git a/chrome/browser/metrics/variations/resource_request_allowed_notifier_unittest.cc b/chrome/browser/metrics/variations/resource_request_allowed_notifier_unittest.cc
new file mode 100644
index 0000000..782140e
--- /dev/null
+++ b/chrome/browser/metrics/variations/resource_request_allowed_notifier_unittest.cc
@@ -0,0 +1,244 @@
+// 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 "chrome/browser/metrics/variations/resource_request_allowed_notifier_test_util.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_pref_service.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/test_browser_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// 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) {
+  }
+
+  // Simulates a change of the connection type to |type|. This will notify any
+  // objects that are NetworkChangeNotifiers.
+  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_;
+  }
+
+  // The currently simulated network connection type. If this is set to
+  // CONNECTION_NONE, then NetworkChangeNotifier::IsOffline will return true.
+  net::NetworkChangeNotifier::ConnectionType connection_type_to_return_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeNotifier);
+};
+
+// A test fixture class for ResourceRequestAllowedNotifier tests that require
+// network state simulations. This also acts as the service implementing the
+// ResourceRequestAllowedNotifier::Observer interface.
+class ResourceRequestAllowedNotifierTest
+    : public testing::Test,
+      public ResourceRequestAllowedNotifier::Observer {
+ public:
+  ResourceRequestAllowedNotifierTest()
+    : ui_thread(content::BrowserThread::UI, &message_loop),
+      was_notified_(false) {
+#if defined(OS_CHROMEOS)
+    // Set this flag to true so the Init call sets up the wait on the EULA.
+    SetNeedsEulaAcceptance(true);
+#endif
+    resource_request_allowed_notifier_.Init(this);
+  }
+  ~ResourceRequestAllowedNotifierTest() { }
+
+  bool was_notified() const { return was_notified_; }
+
+  // ResourceRequestAllowedNotifier::Observer override:
+  virtual void OnResourceRequestsAllowed() OVERRIDE {
+    was_notified_ = true;
+  }
+
+  // Network manipulation methods:
+  void SetWasWaitingForNetwork(bool waiting) {
+    resource_request_allowed_notifier_.
+        SetWasWaitingForNetworkForTesting(waiting);
+  }
+
+  void SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::ConnectionType type) {
+    network_notifier.SimulateNetworkConnectionChange(type);
+  }
+
+  // Simulate a resource request from the test service.
+  void SimulateResourceRequest() {
+    resource_request_allowed_notifier_.ResourceRequestsAllowed();
+  }
+
+#if defined(OS_CHROMEOS)
+  // Eula manipulation methods:
+  void SetNeedsEulaAcceptance(bool needs_acceptance) {
+    resource_request_allowed_notifier_.SetNeedsEulaAcceptance(needs_acceptance);
+  }
+
+  void SetWasWaitingForEula(bool waiting) {
+    resource_request_allowed_notifier_.SetWasWaitingForEulaForTesting(waiting);
+  }
+
+  void SimulateEulaAccepted() {
+    SetNeedsEulaAcceptance(false);
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_WIZARD_EULA_ACCEPTED,
+        content::NotificationService::AllSources(),
+        content::NotificationService::NoDetails());
+  }
+
+  // Used in tests involving the EULA. Disables both the EULA accepted state
+  // and the network.
+  void DisableEulaAndNetwork() {
+    SetWasWaitingForNetwork(true);
+    SimulateNetworkConnectionChange(
+        net::NetworkChangeNotifier::CONNECTION_NONE);
+    SetWasWaitingForEula(true);
+    SetNeedsEulaAcceptance(true);
+  }
+#endif
+
+  virtual void SetUp() OVERRIDE {
+    // Assume the test service has already requested permission, as all tests
+    // just test that criteria changes notify the server.
+#if defined(OS_CHROMEOS)
+    // Set default EULA state to done (not waiting and EULA accepted) to
+    // simplify non-ChromeOS tests.
+    SetWasWaitingForEula(false);
+    SetNeedsEulaAcceptance(false);
+#endif
+  }
+
+ private:
+  MessageLoopForUI message_loop;
+  content::TestBrowserThread ui_thread;
+  TestNetworkChangeNotifier network_notifier;
+  TestRequestAllowedNotifier resource_request_allowed_notifier_;
+  bool was_notified_;
+
+  DISALLOW_COPY_AND_ASSIGN(ResourceRequestAllowedNotifierTest);
+};
+
+TEST_F(ResourceRequestAllowedNotifierTest, DoNotNotifyIfOffline) {
+  SimulateResourceRequest();
+  SetWasWaitingForNetwork(true);
+  SimulateNetworkConnectionChange(net::NetworkChangeNotifier::CONNECTION_NONE);
+  EXPECT_FALSE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, DoNotNotifyIfOnlineToOnline) {
+  SimulateResourceRequest();
+  SetWasWaitingForNetwork(false);
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  EXPECT_FALSE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, NotifyOnReconnect) {
+  SimulateResourceRequest();
+  SetWasWaitingForNetwork(true);
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  EXPECT_TRUE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, NoNotifyOnWardriving) {
+  SimulateResourceRequest();
+  SetWasWaitingForNetwork(false);
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_FALSE(was_notified());
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_3G);
+  EXPECT_FALSE(was_notified());
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_4G);
+  EXPECT_FALSE(was_notified());
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_FALSE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, NoNotifyOnFlakyConnection) {
+  SimulateResourceRequest();
+  SetWasWaitingForNetwork(false);
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_FALSE(was_notified());
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_NONE);
+  EXPECT_FALSE(was_notified());
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_FALSE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, NoRequestNoNotify) {
+  // Ensure that if the observing service does not request access, it does not
+  // get notified, even if the criteria is met. Note that this is done by not
+  // calling SimulateResourceRequest here.
+  SetWasWaitingForNetwork(true);
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
+  EXPECT_FALSE(was_notified());
+}
+
+#if defined(OS_CHROMEOS)
+TEST_F(ResourceRequestAllowedNotifierTest, EulaOnlyNetworkOffline) {
+  SimulateResourceRequest();
+  DisableEulaAndNetwork();
+
+  SimulateEulaAccepted();
+  EXPECT_FALSE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, EulaFirst) {
+  SimulateResourceRequest();
+  DisableEulaAndNetwork();
+
+  SimulateEulaAccepted();
+  EXPECT_FALSE(was_notified());
+
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_TRUE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, NetworkFirst) {
+  SimulateResourceRequest();
+  DisableEulaAndNetwork();
+
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_FALSE(was_notified());
+
+  SimulateEulaAccepted();
+  EXPECT_TRUE(was_notified());
+}
+
+TEST_F(ResourceRequestAllowedNotifierTest, NoRequestNoNotifyEula) {
+  // Ensure that if the observing service does not request access, it does not
+  // get notified, even if the criteria is met. Note that this is done by not
+  // calling SimulateResourceRequest here.
+  DisableEulaAndNetwork();
+
+  SimulateNetworkConnectionChange(
+      net::NetworkChangeNotifier::CONNECTION_WIFI);
+  EXPECT_FALSE(was_notified());
+
+  SimulateEulaAccepted();
+  EXPECT_FALSE(was_notified());
+}
+#endif  // OS_CHROMEOS
diff --git a/chrome/browser/metrics/variations/variations_service.cc b/chrome/browser/metrics/variations/variations_service.cc
index 2b95bf6..131f646 100644
--- a/chrome/browser/metrics/variations/variations_service.cc
+++ b/chrome/browser/metrics/variations/variations_service.cc
@@ -11,7 +11,6 @@
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/field_trial.h"
-#include "base/metrics/histogram.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
@@ -100,12 +99,19 @@ GURL GetVariationsServerURL() {
 VariationsService::VariationsService()
     : variations_server_url_(GetVariationsServerURL()),
       create_trials_from_seed_called_(false),
-      was_offline_during_last_request_attempt_(false) {
-  net::NetworkChangeNotifier::AddConnectionTypeObserver(this);
+      resource_request_allowed_notifier_(
+          new ResourceRequestAllowedNotifier) {
+  resource_request_allowed_notifier_->Init(this);
+}
+
+VariationsService::VariationsService(ResourceRequestAllowedNotifier* notifier)
+    : variations_server_url_(GetVariationsServerURL()),
+      create_trials_from_seed_called_(false),
+      resource_request_allowed_notifier_(notifier) {
+  resource_request_allowed_notifier_->Init(this);
 }
 
 VariationsService::~VariationsService() {
-  net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
 }
 
 bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) {
@@ -151,18 +157,18 @@ void VariationsService::StartRepeatedVariationsSeedFetch() {
                this, &VariationsService::FetchVariationsSeed);
 }
 
-void VariationsService::FetchVariationsSeed() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+// static
+void VariationsService::RegisterPrefs(PrefService* prefs) {
+  prefs->RegisterStringPref(prefs::kVariationsSeed, std::string());
+  prefs->RegisterInt64Pref(prefs::kVariationsSeedDate,
+                           base::Time().ToInternalValue());
+}
 
-  was_offline_during_last_request_attempt_ =
-      net::NetworkChangeNotifier::IsOffline();
-  UMA_HISTOGRAM_BOOLEAN("Variations.NetworkAvailability",
-                        !was_offline_during_last_request_attempt_);
-  if (was_offline_during_last_request_attempt_) {
-    DVLOG(1) << "Network was offline.";
-    return;
-  }
+void VariationsService::SetCreateTrialsFromSeedCalledForTesting(bool called) {
+  create_trials_from_seed_called_ = called;
+}
 
+void VariationsService::DoActualFetch() {
   pending_seed_request_.reset(net::URLFetcher::Create(
       variations_server_url_, net::URLFetcher::GET, this));
   pending_seed_request_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
@@ -177,16 +183,15 @@ void VariationsService::FetchVariationsSeed() {
   pending_seed_request_->Start();
 }
 
-void VariationsService::SetWasOfflineDuringLastRequestAttemptForTesting(
-    bool offline) {
-  was_offline_during_last_request_attempt_ = offline;
-}
+void VariationsService::FetchVariationsSeed() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-// static
-void VariationsService::RegisterPrefs(PrefService* prefs) {
-  prefs->RegisterStringPref(prefs::kVariationsSeed, std::string());
-  prefs->RegisterInt64Pref(prefs::kVariationsSeedDate,
-                           base::Time().ToInternalValue());
+  if (!resource_request_allowed_notifier_->ResourceRequestsAllowed()) {
+    DVLOG(1) << "Resource requests were not allowed. Waiting for notification.";
+    return;
+  }
+
+  DoActualFetch();
 }
 
 void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
@@ -216,24 +221,17 @@ void VariationsService::OnURLFetchComplete(const net::URLFetcher* source) {
   StoreSeedData(seed_data, response_date, g_browser_process->local_state());
 }
 
-void VariationsService::OnConnectionTypeChanged(
-    net::NetworkChangeNotifier::ConnectionType type) {
-  // If the connection type is back online, start a request if the last request
-  // failed due to being offline.
-  if (was_offline_during_last_request_attempt_ &&
-      type != net::NetworkChangeNotifier::CONNECTION_NONE) {
-    VLOG(1) << "Retrying fetch due to network reconnect.";
-    FetchVariationsSeed();
-
-    // Since FetchVariationsSeed was explicitly called here, reset the timer to
-    // avoid retrying for a full period.
-    // net::NetworkChangeNotifier::IsOffline may be inconsistent with |type|, so
-    // we check if FetchVariationsSeed set
-    // |was_offline_during_last_request_attempt_| to true before we reset the
-    // timer.
-    if (!was_offline_during_last_request_attempt_ && timer_.IsRunning())
-      timer_.Reset();
-  }
+void VariationsService::OnResourceRequestsAllowed() {
+  // Note that this only attempts to fetch the seed at most once per period
+  // (kSeedFetchPeriodHours). This works because
+  // |resource_request_allowed_notifier_| only calls this method if an
+  // attempt was made earlier that fails (which implies that the period had
+  // elapsed). After a successful attempt is made, the notifier will know not
+  // to call this method again until another failed attempt occurs.
+  DVLOG(1) << "Retrying fetch.";
+  DoActualFetch();
+  if (timer_.IsRunning())
+    timer_.Reset();
 }
 
 bool VariationsService::StoreSeedData(const std::string& seed_data,
diff --git a/chrome/browser/metrics/variations/variations_service.h b/chrome/browser/metrics/variations/variations_service.h
index 3f34121..4f55d4a 100644
--- a/chrome/browser/metrics/variations/variations_service.h
+++ b/chrome/browser/metrics/variations/variations_service.h
@@ -15,9 +15,9 @@
 #include "base/timer.h"
 #include "chrome/browser/metrics/proto/study.pb.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
+#include "chrome/browser/metrics/variations/resource_request_allowed_notifier.h"
 #include "chrome/common/chrome_version_info.h"
 #include "googleurl/src/gurl.h"
-#include "net/base/network_change_notifier.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
 class PrefService;
@@ -28,9 +28,14 @@ namespace chrome_variations {
 // new seed data from the variations server.
 class VariationsService
     : public net::URLFetcherDelegate,
-      public net::NetworkChangeNotifier::ConnectionTypeObserver{
+      public ResourceRequestAllowedNotifier::Observer {
  public:
   VariationsService();
+
+  // This constructor exists for injecting a mock notifier. It is meant for
+  // testing only. This instance will take ownership of |notifier|.
+  explicit VariationsService(ResourceRequestAllowedNotifier* notifier);
+
   virtual ~VariationsService();
 
   // Creates field trials based on Variations Seed loaded from local prefs. If
@@ -43,17 +48,16 @@ class VariationsService
   // |CreateTrialsFromSeed|.
   void StartRepeatedVariationsSeedFetch();
 
-  // Starts the fetching process once, where |OnURLFetchComplete| is called with
-  // the response. If the network is down at the time, sets a flag to retry when
-  // the network is back online. This is virtual so it can be overriden for
-  // testing.
-  virtual void FetchVariationsSeed();
+  // Register Variations related prefs in Local State.
+  static void RegisterPrefs(PrefService* prefs);
 
   // Exposed for testing.
-  void SetWasOfflineDuringLastRequestAttemptForTesting(bool offline);
+  void SetCreateTrialsFromSeedCalledForTesting(bool called);
 
-  // Register Variations related prefs in Local State.
-  static void RegisterPrefs(PrefService* prefs);
+ protected:
+  // Starts the fetching process once, where |OnURLFetchComplete| is called with
+  // the response.
+  virtual void DoActualFetch();
 
  private:
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, CheckStudyChannel);
@@ -70,12 +74,15 @@ class VariationsService
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, StoreSeed);
   FRIEND_TEST_ALL_PREFIXES(VariationsServiceTest, ValidateStudy);
 
+  // Checks if prerequisites for fetching the Variations seed are met, and if
+  // so, performs the actual fetch using |DoActualFetch|.
+  void FetchVariationsSeed();
+
   // net::URLFetcherDelegate implementation:
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
-  // net::NetworkChangeNotifier::ConnectionTypeObserver implementation.
-  virtual void OnConnectionTypeChanged(
-      net::NetworkChangeNotifier::ConnectionType type) OVERRIDE;
+  // ResourceRequestAllowedNotifier::Observer implementation:
+  virtual void OnResourceRequestsAllowed() OVERRIDE;
 
   // Store the given seed data to the given local prefs. Note that |seed_data|
   // is assumed to be the raw serialized protobuf data stored in a string. It
@@ -146,14 +153,14 @@ class VariationsService
   // it gets called prior to |StartRepeatedVariationsSeedFetch|.
   bool create_trials_from_seed_called_;
 
-  // Tracks whether or not the last seed request attempt failed due to being
-  // offline.
-  bool was_offline_during_last_request_attempt_;
-
   // The timer used to repeatedly ping the server. Keep this as an instance
   // member so if VariationsService goes out of scope, the timer is
   // automatically canceled.
   base::RepeatingTimer<VariationsService> timer_;
+
+  // Helper class used to tell this service if it's allowed to make network
+  // resource requests.
+  scoped_ptr<ResourceRequestAllowedNotifier> resource_request_allowed_notifier_;
 };
 
 }  // namespace chrome_variations
diff --git a/chrome/browser/metrics/variations/variations_service_unittest.cc b/chrome/browser/metrics/variations/variations_service_unittest.cc
index 0de0800..c18c702 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,16 +21,20 @@ namespace {
 // A test class used to validate expected functionality in VariationsService.
 class TestVariationsService : public VariationsService {
  public:
-  TestVariationsService() : VariationsService(),
-                            fetch_attempted_(false) {
+  explicit TestVariationsService(TestRequestAllowedNotifier* test_notifier)
+      : VariationsService(test_notifier),
+        fetch_attempted_(false) {
+    // Set this so StartRepeatedVariationsSeedFetch can be called in tests.
+    SetCreateTrialsFromSeedCalledForTesting(true);
+  }
+
+  virtual ~TestVariationsService() {
   }
-  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;
   }
 
@@ -38,32 +44,6 @@ class TestVariationsService : public VariationsService {
   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_;
-
-  DISALLOW_COPY_AND_ASSIGN(TestNetworkChangeNotifier);
-};
-
 // Converts |time| to Study proto format.
 int64 TimeToProtoTime(const base::Time& time) {
   return (time - base::Time::UnixEpoch()).InSeconds();
@@ -86,36 +66,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 +351,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 +385,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 +484,38 @@ TEST(VariationsServiceTest, ValidateStudy) {
   EXPECT_FALSE(valid);
 }
 
-TEST_F(VariationsServiceNetworkTest, DoNotFetchIfOffline) {
-  SetWasOfflineDuringLastRequestAttempt(true);
-  SimulateNetworkConnectionChange(net::NetworkChangeNotifier::CONNECTION_NONE);
-  EXPECT_FALSE(fetch_attempted());
-}
+TEST(VariationsServiceTest, RequestsInitiallyNotAllowed) {
+  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());
-}
+  // 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_F(VariationsServiceNetworkTest, FetchOnReconnect) {
-  SetWasOfflineDuringLastRequestAttempt(true);
-  SimulateNetworkConnectionChange(
-      net::NetworkChangeNotifier::CONNECTION_ETHERNET);
-  EXPECT_TRUE(fetch_attempted());
-}
+  // Force the notifier to initially disallow requests.
+  test_notifier->SetRequestsAllowedOverride(false);
+  test_service.StartRepeatedVariationsSeedFetch();
+  EXPECT_FALSE(test_service.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_notifier->NotifyObserver();
+  EXPECT_TRUE(test_service.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, RequestsInitiallyAllowed) {
+  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->SetRequestsAllowedOverride(true);
+  test_service.StartRepeatedVariationsSeedFetch();
+  EXPECT_TRUE(test_service.fetch_attempted());
 }
 
 }  // namespace chrome_variations