Simulate variation seeds using installed version string.

chrome/browser/metrics/variations/variations_service.cc

Index: chrome/browser/metrics/variations/variations_service.cc
===================================================================
--- chrome/browser/metrics/variations/variations_service.cc	(revision 277793)
+++ chrome/browser/metrics/variations/variations_service.cc	(working copy)
@@ -37,6 +37,10 @@
 #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
@@ -109,6 +113,21 @@
 #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) {
@@ -209,8 +228,8 @@
       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);
 }
 
@@ -224,7 +243,8 @@
       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 :)

 
@@ -404,9 +424,9 @@
 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();
@@ -416,35 +436,11 @@
   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() {
@@ -546,6 +542,33 @@
   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_) {