Support VariationsRestrictParameter in VariationsService for Chrome OS

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 b841575b38b3ee073be958b9a294bd7a6f8aaa4f..d39a78ad80c2471eee106cfb613cb2d73ae78a33 100644
--- a/chrome/browser/metrics/variations/variations_service.cc
+++ b/chrome/browser/metrics/variations/variations_service.cc
@@ -32,6 +32,10 @@
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_status.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#endif
+
 namespace chrome_variations {
 
 namespace {
@@ -114,6 +118,19 @@ base::Time ConvertStudyDateToBaseTime(int64 date_time) {
   return base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(date_time);
 }
 
+// Gets the restrict parameter from |local_state| or from Chrome OS settings in
+// the case of that platform.
+std::string GetRestrictParameterPref(PrefService* local_state) {
+  std::string parameter;
+#if defined(OS_CHROMEOS)

[Mattias Nissler (ping if slow), 2013/04/18 10:39:39]

You might want to add a comment here saying that you're reading the cached (i.e.
untrusted value) because this starts early, and you can't reconfigure your
service to use a different parameter at run time (which also justifies that you
don't have an observer for pref changes).

+  chromeos::CrosSettings::Get()->GetString(
+      chromeos::kVariationsRestrictParameter, &parameter);
+#else
+  parameter = local_state->GetString(prefs::kVariationsRestrictParameter);
+#endif
+  return parameter;
+}
+
 }  // namespace
 
 VariationsService::VariationsService(PrefService* local_state)
@@ -211,15 +228,11 @@ GURL VariationsService::GetVariationsServerURL(PrefService* local_state) {
   if (server_url_string.empty())
     server_url_string = kDefaultVariationsServerURL;
   GURL server_url = GURL(server_url_string);
-  if (local_state) {

[Alexei Svitkine (slow), 2013/04/17 21:24:36]

I think you previously had this if statement because of tests under which
local_state is NULL. Is this no longer an issue?

[Mathieu, 2013/04/18 19:17:19]

Still an issue, see GetVariationsServerURL(NULL) above.

-    // Append the "restrict" parameter if it is found in prefs.
-    const std::string restrict_param =
-        local_state->GetString(prefs::kVariationsRestrictParameter);
-    if (!restrict_param.empty())
-      server_url = net::AppendOrReplaceQueryParameter(server_url,
-                                                      "restrict",
-                                                      restrict_param);
-  }
+  const std::string restrict_param = GetRestrictParameterPref(local_state);
+  if (!restrict_param.empty())

[Mattias Nissler (ping if slow), 2013/04/18 10:39:39]

nit: multi-line statements should use braces

[Mathieu, 2013/04/18 19:17:19]

Done.

+    server_url = net::AppendOrReplaceQueryParameter(server_url,
+                                                    "restrict",
+                                                    restrict_param);
   DCHECK(server_url.is_valid());
   return server_url;
 }