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

chrome/browser/metrics/variations/variations_service.cc

Index: chrome/browser/metrics/variations/variations_service.cc
diff --git a/chrome/browser/metrics/variations/variations_service.cc b/chrome/browser/metrics/variations/variations_service.cc
index 2b95bf6e4aa1b2b0389b7f241fe50b541692edfa..e10076d11b4a2535a4ab1f42d50645f9fa9dc1e7 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"
@@ -99,13 +98,12 @@ 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);
+      create_trials_from_seed_called_(false) {
+  resource_request_allowed_notifier_.AddObserver(this);
 }
 
 VariationsService::~VariationsService() {
-  net::NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
+  resource_request_allowed_notifier_.RemoveObserver(this);
 }
 
 bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) {
@@ -154,12 +152,8 @@ void VariationsService::StartRepeatedVariationsSeedFetch() {
 void VariationsService::FetchVariationsSeed() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
-  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.";
+  if (!resource_request_allowed_notifier_.ResourceRequestsAllowed()) {
+    DVLOG(1) << "Resource requests were not allowed. Waiting for notification.";
     return;
   }
 
@@ -179,7 +173,15 @@ void VariationsService::FetchVariationsSeed() {
 
 void VariationsService::SetWasOfflineDuringLastRequestAttemptForTesting(

[Alexei Svitkine (slow), 2012/09/12 15:15:46]

I don't see your tests calling this anymore.

[SteveT, 2012/09/14 20:25:09]

Removed.

     bool offline) {
-  was_offline_during_last_request_attempt_ = offline;
+  resource_request_allowed_notifier_.
+      SetWasOfflineDuringLastRequestAttemptForTesting(offline);
+}
+
+void VariationsService::OnResourceRequestsAllowed() {

[Alexei Svitkine (slow), 2012/09/12 15:15:46]

Does this guarantee that we won't fetch more than once every 5 hours if e.g. the
network is flapping?

We've had this discussion before and I think the answer may be "yes" due to how
the code is structured. If that's the case, please document it both here as well
as making sure that it's also clear in the comments for the new class (e.g. that
it only notifies when the last "is allowed" returned false, but not otherwise.)

[SteveT, 2012/09/14 20:25:09]

Done here, and already done in the header of the RRAN class (see the 3rd
paragraph).

+  VLOG(1) << "Retrying fetch.";

[Ilya Sherman, 2012/09/11 22:51:27]

nit: DVLOG?

[SteveT, 2012/09/14 20:25:09]

Done.

+  FetchVariationsSeed();
+  if (timer_.IsRunning())
+    timer_.Reset();
 }
 
 // static
@@ -216,26 +218,6 @@ 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();
-  }
-}
-
 bool VariationsService::StoreSeedData(const std::string& seed_data,
                                       const base::Time& seed_date,
                                       PrefService* local_prefs) {