Simulate variation seeds using installed version string.

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/build_time.h"
 #include "base/command_line.h"
@@ skipping 19 matching lines @@
 #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"
 #include "ui/base/device_form_factor.h"
 #include "url/gurl.h"
 
+#if !defined(OS_ANDROID) && !defined(OS_IOS) && !defined(OS_CHROMEOS)
+#include "chrome/browser/upgrade_detector_impl.h"
+#endif
+
 #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[] =
@@ skipping 52 matching lines @@
   return "android";
 #elif defined(OS_LINUX) || defined(OS_BSD) || defined(OS_SOLARIS)
   // Default BSD and SOLARIS to Linux to not break those builds, although these
   // platforms are not officially supported by Chrome.
   return "linux";
 #else
 #error Unknown platform
 #endif
 }
 
+// Gets the version number to use for variations seed simulation. Must be called
+// on a thread where IO is allowed.
+base::Version GetVersionForSimulation() {
+#if !defined(OS_ANDROID) && !defined(OS_IOS) && !defined(OS_CHROMEOS)
+  const base::Version installed_version =
+      UpgradeDetectorImpl::GetCurrentlyInstalledVersion();
+  if (installed_version.IsValid())
+    return installed_version;
+#endif  // !defined(OS_ANDROID) && !defined(OS_IOS) && !defined(OS_CHROMEOS)
+
+  // TODO(asvitkine): Get the version that will be used on restart instead of
+  // the current version on Android, iOS and ChromeOS.
+  return base::Version(chrome::VersionInfo().Version());
+}
+
 // Gets the restrict parameter from |policy_pref_service| or from Chrome OS
 // settings in the case of that platform.
 std::string GetRestrictParameterPref(PrefService* policy_pref_service) {
   std::string parameter;
 #if defined(OS_CHROMEOS)
   chromeos::CrosSettings::Get()->GetString(
       chromeos::kVariationsRestrictParameter, &parameter);
 #else
   if (policy_pref_service) {
     parameter =
@@ skipping 80 matching lines @@
 
 VariationsService::VariationsService(
     PrefService* local_state,
     metrics::MetricsStateManager* state_manager)
     : local_state_(local_state),
       state_manager_(state_manager),
       policy_pref_service_(local_state),
       seed_store_(local_state),
       create_trials_from_seed_called_(false),
       initial_request_completed_(false),
-      resource_request_allowed_notifier_(
-          new ResourceRequestAllowedNotifier) {
+      resource_request_allowed_notifier_(new ResourceRequestAllowedNotifier),
+      weak_ptr_factory_(this) {
   resource_request_allowed_notifier_->Init(this);
 }
 
 VariationsService::VariationsService(
     ResourceRequestAllowedNotifier* notifier,
     PrefService* local_state,
     metrics::MetricsStateManager* state_manager)
     : local_state_(local_state),
       state_manager_(state_manager),
       policy_pref_service_(local_state),
       seed_store_(local_state),
       create_trials_from_seed_called_(false),
       initial_request_completed_(false),
-      resource_request_allowed_notifier_(notifier) {
+      resource_request_allowed_notifier_(notifier),
+      weak_ptr_factory_(this) {
   resource_request_allowed_notifier_->Init(this);
 }

[Ilya Sherman, 2014/06/17 21:24:59]

nit: It would be nice if there weren't multiple constructors for this class...
(not necessarily something that needs to be addressed as part of this CL, but
just something I'm noticing from looking at this code).  (Alternately, it would
be nice if we could C++11 already.)

[Alexei Svitkine (slow), 2014/06/17 22:15:54]

I think this can be cleaned up. File http://crbug.com/385843

[Ilya Sherman, 2014/06/17 22:27:53]

Thanks :)

 
 VariationsService::~VariationsService() {
 }
 
 bool VariationsService::CreateTrialsFromSeed() {
   create_trials_from_seed_called_ = true;
 
   VariationsSeed seed;
   if (!seed_store_.LoadSeed(&seed))
     return false;
@@ skipping 157 matching lines @@
     time_since_last_fetch = now - last_request_started_time_;
   UMA_HISTOGRAM_CUSTOM_COUNTS("Variations.TimeSinceLastFetchAttempt",
                               time_since_last_fetch.InMinutes(), 0,
                               base::TimeDelta::FromDays(7).InMinutes(), 50);
   last_request_started_time_ = now;
 }
 
 void VariationsService::StoreSeed(const std::string& seed_data,
                                   const std::string& seed_signature,
                                   const base::Time& date_fetched) {
-  VariationsSeed seed;
+  scoped_ptr<VariationsSeed> seed(new VariationsSeed);
   if (!seed_store_.StoreSeedData(seed_data, seed_signature, date_fetched,
-                                 &seed)) {
+                                 seed.get())) {
     return;
   }
   RecordLastFetchTime();
 
   // Perform seed simulation only if |state_manager_| is not-NULL. The state
   // manager may be NULL for some unit tests.
   if (!state_manager_)
     return;
 
-  const base::ElapsedTimer timer;
-
-  // TODO(asvitkine): Get the version that will be used on restart instead of
-  // the current version (i.e. if an update has been downloaded).
-  const chrome::VersionInfo current_version_info;
-  if (!current_version_info.is_valid())
-    return;
-
-  const base::Version current_version(current_version_info.Version());
-  if (!current_version.IsValid())
-    return;
-
-  scoped_ptr<const base::FieldTrial::EntropyProvider> entropy_provider =
-      state_manager_->CreateEntropyProvider();
-  VariationsSeedSimulator seed_simulator(*entropy_provider);
-
-  VariationsSeedSimulator::Result result = seed_simulator.SimulateSeedStudies(
-      seed, g_browser_process->GetApplicationLocale(),
-      GetReferenceDateForExpiryChecks(local_state_), current_version,
-      GetChannelForVariations(), GetCurrentFormFactor(), GetHardwareClass());
-
-  UMA_HISTOGRAM_COUNTS_100("Variations.SimulateSeed.NormalChanges",
-                           result.normal_group_change_count);
-  UMA_HISTOGRAM_COUNTS_100("Variations.SimulateSeed.KillBestEffortChanges",
-                           result.kill_best_effort_group_change_count);
-  UMA_HISTOGRAM_COUNTS_100("Variations.SimulateSeed.KillCriticalChanges",
-                           result.kill_critical_group_change_count);
-
-  UMA_HISTOGRAM_TIMES("Variations.SimulateSeed.Duration", timer.Elapsed());
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::FILE, FROM_HERE,

[Ilya Sherman, 2014/06/17 21:24:59]

I believe that the shared blocking thread pool is preferred over the file thread
these days.

[Alexei Svitkine (slow), 2014/06/17 22:15:54]

When I try that, I get:

../../chrome/browser/metrics/variations/variations_service.cc:439:46: error: no
member named 'PostTaskAndReplyWithResult' in 'base::SequencedWorkerPool'
  content::BrowserThread::GetBlockingPool()->PostTaskAndReplyWithResult(
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ^

I suppose base::SequencedWorkerPool can be expanded with that API, but I'd
rather not shave an extra yak unless you insist. Let me know what you prefer.

[Ilya Sherman, 2014/06/17 22:27:53]

Can you use PostTaskAndReplyWithResult() from base/task_runner_util.h?

[Alexei Svitkine (slow), 2014/06/18 15:18:55]

Done. Kind of silly that we have a bunch of different ways to do this, depending
on the situation (instance methods, static methods, free-standing functions,
sigh). Thanks for finding the right one here.

+      base::Bind(&GetVersionForSimulation),
+      base::Bind(&VariationsService::PerformSimulationWithVersion,
+                 weak_ptr_factory_.GetWeakPtr(), base::Passed(&seed)));
 }
 
 void VariationsService::FetchVariationsSeed() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   const ResourceRequestAllowedNotifier::State state =
       resource_request_allowed_notifier_->GetResourceRequestsAllowedState();
   RecordRequestsAllowedHistogram(ResourceRequestStateToHistogramValue(state));
   if (state != ResourceRequestAllowedNotifier::ALLOWED) {
     DVLOG(1) << "Resource requests were not allowed. Waiting for notification.";
@@ skipping 81 matching lines @@
   // to call this method again until another failed attempt occurs.
   RecordRequestsAllowedHistogram(RESOURCE_REQUESTS_ALLOWED_NOTIFIED);
   DVLOG(1) << "Retrying fetch.";
   DoActualFetch();
 
   // This service must have created a scheduler in order for this to be called.
   DCHECK(request_scheduler_.get());
   request_scheduler_->Reset();
 }
 
+void VariationsService::PerformSimulationWithVersion(
+    scoped_ptr<VariationsSeed> seed,
+    const base::Version& version) {
+  if (version.IsValid())
+    return;
+
+  const base::ElapsedTimer timer;
+
+  scoped_ptr<const base::FieldTrial::EntropyProvider> entropy_provider =
+      state_manager_->CreateEntropyProvider();
+  VariationsSeedSimulator seed_simulator(*entropy_provider);
+
+  VariationsSeedSimulator::Result result = seed_simulator.SimulateSeedStudies(
+      *seed, g_browser_process->GetApplicationLocale(),
+      GetReferenceDateForExpiryChecks(local_state_), version,
+      GetChannelForVariations(), GetCurrentFormFactor(), GetHardwareClass());
+
+  UMA_HISTOGRAM_COUNTS_100("Variations.SimulateSeed.NormalChanges",
+                           result.normal_group_change_count);
+  UMA_HISTOGRAM_COUNTS_100("Variations.SimulateSeed.KillBestEffortChanges",
+                           result.kill_best_effort_group_change_count);
+  UMA_HISTOGRAM_COUNTS_100("Variations.SimulateSeed.KillCriticalChanges",
+                           result.kill_critical_group_change_count);
+
+  UMA_HISTOGRAM_TIMES("Variations.SimulateSeed.Duration", timer.Elapsed());
+}
+
 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