Support VariationsRestrictParameter in VariationsService for Chrome OS

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 <set>
 
 #include "base/base64.h"
 #include "base/build_time.h"
@@ skipping 14 matching lines @@
 #include "googleurl/src/gurl.h"
 #include "net/base/load_flags.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"
 #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 {
 
 // Default server of Variations seed info.
 const char kDefaultVariationsServerURL[] =
     "https://clients4.google.com/chrome-variations/seed";
 const int kMaxRetrySeedFetch = 5;
 
 // TODO(mad): To be removed when we stop updating the NetworkTimeTracker.
@@ skipping 62 matching lines @@
 #else
 #error Unknown platform
 #endif
 }
 
 // Converts |date_time| in Study date format to base::Time.
 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)
     : local_state_(local_state),
       variations_server_url_(GetVariationsServerURL(local_state)),
       create_trials_from_seed_called_(false),
       resource_request_allowed_notifier_(
           new ResourceRequestAllowedNotifier) {
   resource_request_allowed_notifier_->Init(this);
 }
@@ skipping 77 matching lines @@
   return network_time_tracker_.GetNetworkTime(network_time, uncertainty);
 }
 
 // static
 GURL VariationsService::GetVariationsServerURL(PrefService* local_state) {
   std::string server_url_string(CommandLine::ForCurrentProcess()->
       GetSwitchValueASCII(switches::kVariationsServerURL));
   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;
 }
 
 #if defined(OS_WIN)
 void VariationsService::StartGoogleUpdateRegistrySync() {
   registry_syncer_.RequestRegistrySync();
 }
 #endif
 
@@ skipping 415 matching lines @@
 
 void VariationsService::RecordLastFetchTime() {
   // local_state_ is NULL in tests, so check it first.
   if (local_state_) {
     local_state_->SetInt64(prefs::kVariationsLastFetchTime,
                            base::Time::Now().ToInternalValue());
   }
 }
 
 }  // namespace chrome_variations