Remove more //chrome dependencies from VariationsService

chrome/browser/metrics/variations/variations_service.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 "chrome/browser/metrics/variations/variations_service.h"
 
 #include "base/build_time.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/sys_info.h"
 #include "base/task_runner_util.h"
 #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"
 #include "components/variations/pref_names.h"
 #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"
 #include "net/base/net_errors.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/url_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_fetcher.h"
@@ skipping 166 matching lines @@
 // set.
 std::string GetHeaderValue(const net::HttpResponseHeaders* headers,
                            const base::StringPiece& name) {
   std::string value;
   headers->EnumerateHeader(NULL, name, &value);
   return value;
 }
 
 // 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;
 
   ui::ResourceBundle::GetSharedInstance().OverrideLocaleStringResource(
       resource_id, string);
 }
 
 }  // namespace
 
 VariationsService::VariationsService(
@@ skipping 30 matching lines @@
   const base::Version current_version(version_info::GetVersionNumber());
   if (!current_version.IsValid())
     return false;
 
   variations::Study_Channel channel =
       GetChannelForVariations(client_->GetChannel());
   UMA_HISTOGRAM_SPARSE_SLOWLY("Variations.UserChannel", channel);
 
   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();
 
   // Log the "freshness" of the seed that was just used. The freshness is the
   // time between the last successful seed download and now.
   const int64 last_fetch_time_internal =
       local_state_->GetInt64(prefs::kVariationsLastFetchTime);
   if (last_fetch_time_internal) {
     const base::TimeDelta delta =
         now - base::Time::FromInternalValue(last_fetch_time_internal);
@@ skipping 144 matching lines @@
   // This preference will only be written by the policy service, which will fill
   // it according to a value stored in the User Policy.
   registry->RegisterStringPref(prefs::kVariationsRestrictParameter,
                                std::string());
 }
 
 // static
 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
   // indicate that the service should not be used.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kVariationsServerURL)) {
     DVLOG(1) << "Not creating VariationsService in unofficial build without --"
              << switches::kVariationsServerURL << " specified.";
     return result.Pass();
   }
 #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();
 }
 
 void VariationsService::DoActualFetch() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!pending_seed_request_);
 
   pending_seed_request_ = net::URLFetcher::Create(0, variations_server_url_,
                                                   net::URLFetcher::GET, this);
@@ skipping 297 matching lines @@
   // Otherwise, update the pref with the current Chrome version and country.
   base::ListValue new_list_value;
   new_list_value.AppendString(version.GetString());
   new_list_value.AppendString(latest_country);
   local_state_->Set(prefs::kVariationsPermanentConsistencyCountry,
                     new_list_value);
   return latest_country;
 }
 
 }  // namespace chrome_variations