Remove more //chrome dependencies from VariationsService

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 b906cac0266f76d1e487e3e858892ace9572ac73..f1c5ed769e777cfe78a88c5aa1665ea7d31c3239 100644
--- a/chrome/browser/metrics/variations/variations_service.cc
+++ b/chrome/browser/metrics/variations/variations_service.cc
@@ -15,8 +15,6 @@
 #include "base/timer/elapsed_timer.h"
 #include "base/values.h"
 #include "base/version.h"
-#include "chrome/browser/metrics/variations/generated_resources_map.h"
-#include "chrome/common/chrome_switches.h"
 #include "components/metrics/metrics_state_manager.h"
 #include "components/network_time/network_time_tracker.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -24,6 +22,7 @@
 #include "components/variations/proto/variations_seed.pb.h"
 #include "components/variations/variations_seed_processor.h"
 #include "components/variations/variations_seed_simulator.h"
+#include "components/variations/variations_switches.h"
 #include "components/variations/variations_url_constants.h"
 #include "components/version_info/version_info.h"
 #include "net/base/load_flags.h"
@@ -210,8 +209,10 @@ std::string GetHeaderValue(const net::HttpResponseHeaders* headers,
 
 // Overrides the string resource sepecified by |hash| with |string| in the
 // resource bundle. Used as a callback passed to the variations seed processor.
-void OverrideUIString(uint32_t hash, const base::string16& string) {
-  int resource_id = GetResourceIndex(hash);
+void OverrideUIString(VariationsServiceClient* client,

[Alexei Svitkine (slow), 2015/08/27 18:27:10]

Why not have the whole OverrideUIString() function be part of the client
interface?

[blundell, 2015/08/27 18:33:01]

In the interest of sharing as much code as possible -- I wouldn't expect this
behavior to change per-embedder. Can change if you feel strongly.

[Alexei Svitkine (slow), 2015/08/27 18:44:00]

I see. I'm thinking that if variations/service might be re-used by other things,
like Chromecast, that they might not want the dependency on the resource bundle
stuff.

But I guess there's also a dependency on FormFactor that's in ui/ too.

I dunno... I still feel like abstracting GetIndexForResource() is weird - I'd
rather keep this whole function together.

Logically, if we do want to refactor things out, I'd rather have a helper class
that gets initialized with the two arrays from generated_resources_map.h and
contains the complete OverrideUIString() logic. But if not going to that length,
I'd prefer that OverrideUIString() stays as-is and just moves to the client.

[blundell, 2015/08/27 19:05:58]

Done. Ideally this wouldn't have to be componentized at all, at least for the
purposes of iOS. But at this time I can't figure out what to do about the
generated_resources_map stuff.

+                      uint32_t hash,
+                      const base::string16& string) {
+  int resource_id = client->GetIndexForResource(hash);
   if (resource_id == -1)
     return;
 
@@ -262,12 +263,14 @@ bool VariationsService::CreateTrialsFromSeed() {
 
   const std::string latest_country =
       local_state_->GetString(prefs::kVariationsCountry);
+  // Note that passing |client_| via base::Unretained below is safe because
+  // the callback is executed synchronously.
   variations::VariationsSeedProcessor().CreateTrialsFromSeed(
       seed, client_->GetApplicationLocale(),
       GetReferenceDateForExpiryChecks(local_state_), current_version, channel,
       GetCurrentFormFactor(), GetHardwareClass(), latest_country,
       LoadPermanentConsistencyCountry(current_version, latest_country),
-      base::Bind(&OverrideUIString));
+      base::Bind(&OverrideUIString, base::Unretained(client_.get())));
 
   const base::Time now = base::Time::Now();
 
@@ -432,7 +435,8 @@ void VariationsService::RegisterProfilePrefs(
 scoped_ptr<VariationsService> VariationsService::Create(
     scoped_ptr<VariationsServiceClient> client,
     PrefService* local_state,
-    metrics::MetricsStateManager* state_manager) {
+    metrics::MetricsStateManager* state_manager,
+    const char* disable_network_switch) {
   scoped_ptr<VariationsService> result;
 #if !defined(GOOGLE_CHROME_BUILD)
   // Unless the URL was provided, unsupported builds should return NULL to
@@ -446,7 +450,7 @@ scoped_ptr<VariationsService> VariationsService::Create(
 #endif
   result.reset(new VariationsService(
       client.Pass(), new web_resource::ResourceRequestAllowedNotifier(
-                         local_state, switches::kDisableBackgroundNetworking),
+                         local_state, disable_network_switch),
       local_state, state_manager));
   return result.Pass();
 }