Variations Service now creates field trials from variations seed.

chrome/browser/metrics/variations_service.cc

Index: chrome/browser/metrics/variations_service.cc
diff --git a/chrome/browser/metrics/variations_service.cc b/chrome/browser/metrics/variations_service.cc
index 64a5f003682da2d7dbbf187cbd3f9077300d037e..0d02f1c3a0839b0b56c60f5b1715303a23a449ac 100644
--- a/chrome/browser/metrics/variations_service.cc
+++ b/chrome/browser/metrics/variations_service.cc
@@ -9,6 +9,7 @@
 #include "base/version.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
+#include "base/metrics/field_trial.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/proto/trials_seed.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
@@ -50,40 +51,44 @@ VariationsService* VariationsService::GetInstance() {
   return Singleton<VariationsService>::get();
 }
 
-void VariationsService::LoadVariationsSeed(PrefService* local_prefs) {
-  std::string base64_seed_data = local_prefs->GetString(prefs::kVariationsSeed);
-  std::string seed_data;
+bool VariationsService::CreateTrialsFromSeed(PrefService* local_prefs) {
+  LOG(INFO) << "Creating trials from seed";

[MAD, 2012/05/08 03:09:00]

Either remove all LOGs or use DVLOG(1)s

[jwd, 2012/05/08 03:30:01]

Done.

+  chrome_variations::TrialsSeed seed;
+  if (!LoadVariationsSeed(local_prefs, seed))
+      return false;
 
-  // If the decode process fails, assume the pref value is corrupt, and clear
-  // it.
-  if (!base::Base64Decode(base64_seed_data, &seed_data) ||
-      !variations_seed_.ParseFromString(seed_data)) {
-    VLOG(1) << "Variations Seed data in local pref is corrupt, clearing the "
-            << "pref.";
-    local_prefs->ClearPref(prefs::kVariationsSeed);
+  LOG(INFO) << "about to loop through studies " << seed.study_size();
+  for (int i = 0; i<seed.study_size(); ++i) {

[Alexei Svitkine (slow), 2012/05/08 03:22:54]

Put spaces around the <.

[jwd, 2012/05/08 03:35:18]

Done.

+    CreateTrialFromStudy(seed.study(i));

[MAD, 2012/05/08 03:09:00]

For single line block, we don't use {}

[jwd, 2012/05/08 03:30:01]

Done.

   }
+  return true;
 }
 
 void VariationsService::StartFetchingVariationsSeed() {
+  LOG(INFO) << "start fetching " << kDefaultVariationsServer;
   pending_seed_request_.reset(content::URLFetcher::Create(
       GURL(kDefaultVariationsServer), content::URLFetcher::GET, this));
   pending_seed_request_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                                       net::LOAD_DO_NOT_SAVE_COOKIES);
   pending_seed_request_->SetRequestContext(
       g_browser_process->system_request_context());
+ // TODO(jwd): pull max retries into a contsant
   pending_seed_request_->SetMaxRetries(5);
   pending_seed_request_->Start();
 }
 
 void VariationsService::OnURLFetchComplete(const content::URLFetcher* source) {
+  LOG(INFO) << "got server response";
   DCHECK_EQ(pending_seed_request_.get(), source);
   // When we're done handling the request, the fetcher will be deleted.
   scoped_ptr<const content::URLFetcher> request(
       pending_seed_request_.release());
   if (request->GetStatus().status() != net::URLRequestStatus::SUCCESS ||
-      request->GetResponseCode() != 200)
+      request->GetResponseCode() != 200) {
+    LOG(INFO) << "not a success";
     return;
-
+  }
+  LOG(INFO) << " response was success";
   std::string seed_data;
   request->GetResponseAsString(&seed_data);
 
@@ -146,7 +151,11 @@ bool VariationsService::ShouldAddStudy(const chrome_variations::Study& study) {
 bool VariationsService::CheckStudyChannel(
     const chrome_variations::Study& study,
     chrome::VersionInfo::Channel channel) {
-  for (int i = 0; i < study.channel_size(); ++i) {
+  if (study.channel_size()==0)

[Alexei Svitkine (slow), 2012/05/08 03:22:54]

Put spaces around the ==.

[jwd, 2012/05/08 03:35:18]

Done.

+    // An empty channel list matches all channels.

[MAD, 2012/05/08 03:09:00]

Add {} if you have more than one line, including comments.

[jwd, 2012/05/08 03:35:18]

Done.

+    return true;
+
+  for (int i = 0; i < study.channel_size(); i++) {

[MAD, 2012/05/08 03:09:00]

i++ -> ++i

[jwd, 2012/05/08 03:30:01]

Done.

     if (ConvertStudyChannelToVersionChannel(study.channel(i)) == channel)
       return true;
   }
@@ -164,18 +173,26 @@ bool VariationsService::CheckStudyVersion(const chrome_variations::Study& study,
 
   if (study.has_min_version()) {
     const Version min_version(study.min_version());
-    if (!min_version.IsValid())
+    if (!min_version.IsValid()){
+      LOG(INFO) << 1;
       return false;
-    if (current_version.CompareTo(min_version) < 0)
+    }
+    if (current_version.CompareTo(min_version) < 0){
+      LOG(INFO) << 2;
       return false;
+    }
   }
 
   if (study.has_max_version()) {
     const Version max_version(study.max_version());
-    if (!max_version.IsValid())
+    if (!max_version.IsValid()){
+      LOG(INFO) << 3;
       return false;
-    if (current_version.CompareTo(max_version) > 0)
+    }
+    if (current_version.CompareTo(max_version) > 0){
+      LOG(INFO) << 4;
       return false;
+    }
   }
 
   return true;
@@ -188,17 +205,104 @@ bool VariationsService::CheckStudyDate(const chrome_variations::Study& study,
 
   if (study.has_start_date()) {
     const base::Time start_date =
-        epoch + base::TimeDelta::FromMilliseconds(study.start_date());
-    if (date_time < start_date)
+        epoch + base::TimeDelta::FromSeconds(study.start_date());

[MAD, 2012/05/08 03:09:00]

Good catch, thanks for fixing...

+    if (date_time < start_date){
+      LOG(INFO) << "fail1 ";
       return false;
+    }
   }
 
   if (study.has_expiry_date()) {
     const base::Time expiry_date =
-        epoch + base::TimeDelta::FromMilliseconds(study.expiry_date());
-    if (date_time >= expiry_date)
+        epoch + base::TimeDelta::FromSeconds(study.expiry_date());
+    if (date_time >= expiry_date){
+      LOG(INFO) << "fail2 ";
       return false;
+    }
   }
 
   return true;
 }
+
+bool VariationsService::LoadVariationsSeed(
+    PrefService* local_prefs,
+    chrome_variations::TrialsSeed& seed) {
+  std::string base64_seed_data = local_prefs->GetString(prefs::kVariationsSeed);
+  std::string seed_data;
+
+  // If the decode process fails, assume the pref value is corrupt, and clear
+  // it.
+  if (!base::Base64Decode(base64_seed_data, &seed_data) ||
+      !seed.ParseFromString(seed_data)) {
+    VLOG(1) << "Variations Seed data in local pref is corrupt, clearing the "
+            << "pref.";
+    local_prefs->ClearPref(prefs::kVariationsSeed);
+    return false;
+  }
+  return true;
+}
+
+void VariationsService::CreateTrialFromStudy(
+    const chrome_variations::Study& study) {
+  LOG(INFO) << study.name();
+  if (!ShouldAddStudy(study)){
+    LOG(INFO) << "study rejected";
+    return;
+  }
+  // At the moment, a missing default_experiment_name makes the study invalid.
+  if (!study.has_default_experiment_name()){

[Alexei Svitkine (slow), 2012/05/08 03:22:54]

Put a space before {.

[jwd, 2012/05/08 03:35:18]

Done.

+    DVLOG(1) << study.name() << " has no default experiment defined.";
+    return;
+  }
+  LOG(INFO) << "study accepted";
+
+  const std::string& default_group_name = study.default_experiment_name();
+  base::FieldTrial::Probability divisor = 0;
+
+  bool found_default_group = false;
+  LOG(INFO) << "looping through groups " << study.experiment_size();
+  LOG(INFO) << "looking for " << default_group_name;
+  int default_group_number;
+  bool has_default_group_number;

[MAD, 2012/05/08 03:09:00]

Please initialize these two variables in case we don't enter the loop below...

+  for (int i = 0; i<study.experiment_size(); ++i) {

[Alexei Svitkine (slow), 2012/05/08 03:22:54]

Put spaces around the <.

[jwd, 2012/05/08 03:35:18]

Done.

+    LOG(INFO) << study.experiment(i).name();
+    divisor += study.experiment(i).probability_weight();
+    if (study.experiment(i).name() == default_group_name){
+      LOG(INFO) << "default found " << default_group_name;
+      found_default_group = true;
+      has_default_group_number = study.experiment(i).has_experiment_id();
+      if (has_default_group_number)
+        default_group_number = study.experiment(i).experiment_id();

[MAD, 2012/05/08 03:09:00]

These are not related, the experiment_id, is the GWS ID... /methinks...

[jwd, 2012/05/08 03:30:01]

Ya, I think I was confused about this when I wrote it. I'm getting rid of the
group number stuff. It looks like there is no way to specify the default group
number in the protobuf. 
Done.

[Alexei Svitkine (slow), 2012/05/08 03:31:27]

That parameter is an "out" parameter, so no need to use it at all (just pass
NULL).

[jwd, 2012/05/08 03:35:18]

Ya, I was very confused when I wrote that.

+    }
+  }
+  if (!found_default_group){
+    LOG(INFO) << study.name() << " is missing default experiment from "
+             << "experiment list";
+    // The default group was not found in the list of groups. This study is not
+    //  valid.
+    return;
+  }
+
+  const base::Time epoch = base::Time::UnixEpoch();
+  const base::Time expiry_date =
+      epoch + base::TimeDelta::FromSeconds(study.expiry_date());
+  base::Time::Exploded exploded_end_date;
+  expiry_date.UTCExplode(&exploded_end_date);
+
+  scoped_refptr<base::FieldTrial> trial(
+      base::FieldTrialList::FactoryGetFieldTrial(
+          study.name(), divisor, default_group_name, exploded_end_date.year,
+          exploded_end_date.month, exploded_end_date.day_of_month,
+          has_default_group_number ? &default_group_number : NULL));

[MAD, 2012/05/08 03:09:00]

This is not needed, you don't use it, and you won't need it... The
default_group_number is only needed if one would like to compare the chosen
group to this value... 

[jwd, 2012/05/08 03:30:01]

Ya, got rid of this now.
Done.

+
+  if (study.has_consistency() &&
+      study.consistency()==chrome_variations::Study_Consistency_PERMANENT)

[Alexei Svitkine (slow), 2012/05/08 03:22:54]

Put spaces around the ==.

[jwd, 2012/05/08 03:35:18]

Done.

+    trial->UseOneTimeRandomization();
+
+  for (int i = 0; i<study.experiment_size(); ++i)

[Alexei Svitkine (slow), 2012/05/08 03:22:54]

Put spaces around the <.

[jwd, 2012/05/08 03:35:18]

Done.

+    if (study.experiment(i).name() != default_group_name)

[MAD, 2012/05/08 03:09:00]

Please use {} (for both the for and the if) when there are more than one lines
in the block, even if it's a single statement...

[jwd, 2012/05/08 03:35:18]

Done.

+      trial->AppendGroup(study.experiment(i).name(),
+                        study.experiment(i).probability_weight());

[Alexei Svitkine (slow), 2012/05/08 03:25:26]

You also need to associate the study.experiment(i).experiment_id() here using
the API SteveT added.

[jwd, 2012/05/08 03:35:18]

MAD says that that's not important for M20, but yes I do.

+
+  trial->SetForced();
+}