Supporting study definitions without default groups and end_date filtering.

end_date is a replacement for expiry_date, which is deprecated by this change. The difference is that end_date is acts as a normal filter, and causes study to not be created when the date is past. expiry_date caused the study to be created, and forced to the default group.

This change introduces a generic default group name, which is used for the field trial API when a study doesn't specify one.

BUG=677976

Review-Url: https://codereview.chromium.org/2614443002
Cr-Commit-Position: refs/heads/master@{#441487}

components/variations/processed_study.cc

@@ -21,11 +21,6 @@ bool ValidateStudyAndComputeTotalProbability(
     base::FieldTrial::Probability* total_probability,
     bool* all_assignments_to_one_group,
     std::string* single_feature_name) {
-  // At the moment, a missing default_experiment_name makes the study invalid.
-  if (study.default_experiment_name().empty()) {
-    DVLOG(1) << study.name() << " has no default experiment defined.";
-    return false;
-  }
   if (study.filter().has_min_version() &&
       !base::Version::IsValidWildcardString(study.filter().min_version())) {
     DVLOG(1) << study.name() << " has invalid min version: "
@@ -92,9 +87,11 @@ bool ValidateStudyAndComputeTotalProbability(
       found_default_group = true;
   }
 
-  if (!found_default_group) {
-    DVLOG(1) << study.name() << " is missing default experiment in its "
-             << "experiment list";
+  // Specifying a default experiment is optional, so finding it in the
+  // experiment list is only required when it is specified.
+  if (!study.default_experiment_name().empty() && !found_default_group) {
+    DVLOG(1) << study.name() << " is missing default experiment ("
+             << study.default_experiment_name() << ") in its experiment list";
     // The default group was not found in the list of groups. This study is not
     // valid.
     return false;
@@ -113,6 +110,10 @@ bool ValidateStudyAndComputeTotalProbability(
 
 }  // namespace
 
+// static
+const std::string ProcessedStudy::kGenericDefaultExperimentName =
+    "VariationsDefaultExperiment";
+
 ProcessedStudy::ProcessedStudy()
     : study_(NULL),
       total_probability_(0),
@@ -150,6 +151,13 @@ int ProcessedStudy::GetExperimentIndexByName(const std::string& name) const {
   return -1;
 }
 
+const std::string& ProcessedStudy::GetDefaultExperimentName() const {
+  if (study_->default_experiment_name().empty())
+    return kGenericDefaultExperimentName;
+
+  return study_->default_experiment_name();
+}
+
 // static
 bool ProcessedStudy::ValidateAndAppendStudy(
     const Study* study,

components/variations/processed_study.h

@@ -18,6 +18,10 @@ class Study;
 // such as whether the study is expired and its total probability.
 class ProcessedStudy {
  public:
+  // The default group used when a study doesn't specify one. This is needed
+  // because the field trial api requires a default group name.
+  static const std::string kGenericDefaultExperimentName;
+
   ProcessedStudy();
   ~ProcessedStudy();
 
@@ -43,6 +47,10 @@ class ProcessedStudy {
   // experiment is found.
   int GetExperimentIndexByName(const std::string& name) const;
 
+  // Gets the default experiment name for the study, or a generic one if none is
+  // specified.
+  const std::string& GetDefaultExperimentName() const;
+
   static bool ValidateAndAppendStudy(
       const Study* study,
       bool is_expired,

components/variations/proto/study.proto

@@ -17,6 +17,7 @@ message Study {
   // Ex: "my_study"
   required string name = 1;
 
+  // DEPRECATED: Prefer end_date instead.
   // The expiry date of the study in Unix time format. (Seconds since midnight
   // January 1, 1970 UTC). See: http://en.wikipedia.org/wiki/Unix_time
   //
@@ -38,7 +39,8 @@ message Study {
   optional Consistency consistency = 7 [default = SESSION];
 
   // Name of the experiment that gets the default experience. This experiment
-  // must be included in the list below.
+  // must be included in the list below. If not specified, a generic default
+  // experiment name is used.
   // Ex: "default"
   optional string default_experiment_name = 8;
 
@@ -198,13 +200,22 @@ message Study {
   // Filtering criteria specifying whether this study is applicable to a given
   // Chrome instance.
   //
-  // Next tag: 13
+  // Next tag: 14
   message Filter {
     // The start date of the study in Unix time format. (Seconds since midnight
     // January 1, 1970 UTC). See: http://en.wikipedia.org/wiki/Unix_time
     // Ex: 1330893974  (corresponds to 2012-03-04 20:46:14Z)
     optional int64 start_date = 1;
 
+    // The end date of the study in Unix time format. (Seconds since midnight
+    // January 1, 1970 UTC). See: http://en.wikipedia.org/wiki/Unix_time
+    // Ex: 1330893974  (corresponds to 2012-03-04 20:46:14Z)
+    // Mutually exclusive with expiry_date. The difference between end_date and
+    // expiry_date is that, when end_date is past, the field trial will not be
+    // created. When expiry_date is past, the trial is still created, but will
+    // be disabled, causing it to select its default group.
+    optional int64 end_date = 13;
+
     // The minimum Chrome version for this study, allowing a trailing '*'
     // character for pattern matching. Inclusive. (To check for a match, iterate
     // over each component checking >= until a * or end of string is reached.)

components/variations/study_filtering.cc

@@ -143,6 +143,16 @@ bool CheckStudyStartDate(const Study_Filter& filter,
   return true;
 }
 
+bool CheckStudyEndDate(const Study_Filter& filter,
+                       const base::Time& date_time) {
+  if (filter.has_end_date()) {
+    const base::Time end_date = ConvertStudyDateToBaseTime(filter.end_date());
+    return end_date >= date_time;
+  }
+
+  return true;
+}
+
 bool CheckStudyVersion(const Study_Filter& filter,
                        const base::Version& version) {
   if (filter.has_min_version()) {
@@ -225,6 +235,11 @@ bool ShouldAddStudy(
       return false;
     }
 
+    if (!CheckStudyEndDate(study.filter(), reference_date)) {
+      DVLOG(1) << "Filtered out study " << study.name() << " due to end date.";
+      return false;
+    }
+
     if (!CheckStudyHardwareClass(study.filter(), hardware_class)) {
       DVLOG(1) << "Filtered out study " << study.name() <<
                   " due to hardware_class.";

components/variations/study_filtering.h

@@ -43,6 +43,9 @@ bool CheckStudyPlatform(const Study_Filter& filter, Study_Platform platform);
 bool CheckStudyStartDate(const Study_Filter& filter,
                          const base::Time& date_time);
 
+// Checks whether a study is applicable for the given date/time per |filter|.
+bool CheckStudyEndDate(const Study_Filter& filter, const base::Time& date_time);
+
 // Checks whether a study is applicable for the given version per |filter|.
 bool CheckStudyVersion(const Study_Filter& filter,
                        const base::Version& version);

components/variations/study_filtering_unittest.cc

@@ -227,9 +227,11 @@ TEST(VariationsStudyFilteringTest, CheckStudyStartDate) {
     const base::Time start_date;
     bool expected_result;
   } start_test_cases[] = {
-    { now - delta, true },
-    { now, true },
-    { now + delta, false },
+      {now - delta, true},
+      // Note, the proto start_date is truncated to seconds, but the reference
+      // date isn't.
+      {now, true},
+      {now + delta, false},
   };
 
   Study_Filter filter;
@@ -245,6 +247,29 @@ TEST(VariationsStudyFilteringTest, CheckStudyStartDate) {
   }
 }
 
+TEST(VariationsStudyFilteringTest, CheckStudyEndDate) {
+  const base::Time now = base::Time::Now();
+  const base::TimeDelta delta = base::TimeDelta::FromHours(1);
+  const struct {
+    const base::Time end_date;
+    bool expected_result;
+  } start_test_cases[] = {
+      {now - delta, false}, {now + delta, true},
+  };
+
+  Study_Filter filter;
+
+  // End date not set should result in true.
+  EXPECT_TRUE(internal::CheckStudyEndDate(filter, now));
+
+  for (size_t i = 0; i < arraysize(start_test_cases); ++i) {
+    filter.set_end_date(TimeToProtoTime(start_test_cases[i].end_date));
+    const bool result = internal::CheckStudyEndDate(filter, now);
+    EXPECT_EQ(start_test_cases[i].expected_result, result) << "Case " << i
+                                                           << " failed!";
+  }
+}
+
 TEST(VariationsStudyFilteringTest, CheckStudyVersion) {
   const struct {
     const char* min_version;
@@ -573,8 +598,9 @@ TEST(VariationsStudyFilteringTest, ValidateStudy) {
   study.mutable_filter()->set_max_version("2.3.4");
   EXPECT_TRUE(processed_study.Init(&study, false));
 
+  // A blank default study is allowed.
   study.clear_default_experiment_name();
-  EXPECT_FALSE(processed_study.Init(&study, false));
+  EXPECT_TRUE(processed_study.Init(&study, false));
 
   study.set_default_experiment_name("xyz");
   EXPECT_FALSE(processed_study.Init(&study, false));

components/variations/variations_seed_processor.cc

@@ -274,7 +274,7 @@ void VariationsSeedProcessor::CreateTrialFromStudy(
   scoped_refptr<base::FieldTrial> trial(
       base::FieldTrialList::FactoryGetFieldTrialWithRandomizationSeed(
           study.name(), processed_study.total_probability(),
-          study.default_experiment_name(),
+          processed_study.GetDefaultExperimentName(),
           base::FieldTrialList::kNoExpirationYear, 1, 1, randomization_type,
           randomization_seed, NULL,
           ShouldStudyUseLowEntropy(study) ? low_entropy_provider : NULL));

components/variations/variations_seed_processor_unittest.cc

@@ -378,8 +378,9 @@ TEST_F(VariationsSeedProcessorTest, ValidateStudy) {
   study.mutable_filter()->set_max_version("2.3.4");
   EXPECT_TRUE(processed_study.Init(&study, false));
 
+  // A blank default study is allowed.
   study.clear_default_experiment_name();
-  EXPECT_FALSE(processed_study.Init(&study, false));
+  EXPECT_TRUE(processed_study.Init(&study, false));
 
   study.set_default_experiment_name("xyz");
   EXPECT_FALSE(processed_study.Init(&study, false));
@@ -845,6 +846,23 @@ TEST_F(VariationsSeedProcessorTest, FeaturesInExpiredStudies) {
   }
 }
 
+TEST_F(VariationsSeedProcessorTest, NoDefaultExperiment) {
+  base::FieldTrialList field_trial_list(nullptr);
+
+  Study study;
+  study.set_name("Study1");
+
+  AddExperiment("A", 1, &study);
+
+  EXPECT_TRUE(CreateTrialFromStudy(study));
+
+  base::FieldTrial* trial = base::FieldTrialList::Find("Study1");
+  trial->Disable();
+
+  EXPECT_EQ(ProcessedStudy::kGenericDefaultExperimentName,
+            base::FieldTrialList::FindFullName("Study1"));
+}
+
 TEST_F(VariationsSeedProcessorTest, LowEntropyStudyTest) {
   const std::string kTrial1Name = "A";
   const std::string kTrial2Name = "B";