Parametrize Suggest polling strategy and delay. Also add a parameter

components/omnibox/omnibox_field_trial_unittest.cc

Index: components/omnibox/omnibox_field_trial_unittest.cc
diff --git a/components/omnibox/omnibox_field_trial_unittest.cc b/components/omnibox/omnibox_field_trial_unittest.cc
index 9a78a67b014ad17703968931a7edc7e658a9f7b0..f77bb51d0f4e4ebfbdfc665b2146e6582006d1db 100644
--- a/components/omnibox/omnibox_field_trial_unittest.cc
+++ b/components/omnibox/omnibox_field_trial_unittest.cc
@@ -65,6 +65,15 @@ class OmniboxFieldTrialTest : public testing::Test {
       const std::string& rule,
       OmniboxEventProto::PageClassification page_classification);
 
+  // EXPECTS that OmniboxFieldTrial::GetSuggestPollingStrategy returns
+  // |expected_from_last_keystroke| and |expected_delay_ms| for the given
+  // experiment params.
+  void VerifySuggestPollingStrategy(
+      const std::string& from_last_keystroke_rule_value,
+      const std::string& polling_delay_ms_rule_value,
+      bool expected_from_last_keystroke,
+      int expected_delay_ms);
+
  private:
   scoped_ptr<base::FieldTrialList> field_trial_list_;
 
@@ -92,6 +101,31 @@ void OmniboxFieldTrialTest::ExpectRuleValue(
                 rule, page_classification));
 }
 
+void OmniboxFieldTrialTest::VerifySuggestPollingStrategy(
+    const std::string& from_last_keystroke_rule_value,
+    const std::string& polling_delay_ms_rule_value,
+    bool expected_from_last_keystroke,

[Mark P, 2014/10/22 23:03:15]

this and below might as well be const

[Bart N., 2014/10/23 01:42:54]

We don't typically put const for params passed by value. See examples above and
elsewhere. If you really insist, I can do it, but please provide a good reason
of doing so.

+    int expected_delay_ms) {
+  ResetFieldTrialList();
+  std::map<std::string, std::string> params;
+  params[std::string(
+      OmniboxFieldTrial::kMeasureSuggestPollingDelayFromLastKeystrokeRule)] =
+      from_last_keystroke_rule_value;
+  params[std::string(
+      OmniboxFieldTrial::kSuggestPollingDelayMsRule)] =
+      polling_delay_ms_rule_value;
+  ASSERT_TRUE(variations::AssociateVariationParams(
+      OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+  base::FieldTrialList::CreateFieldTrial(
+      OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+
+  bool from_last_keystroke;
+  int delay_ms;
+  OmniboxFieldTrial::GetSuggestPollingStrategy(&from_last_keystroke, &delay_ms);
+  EXPECT_EQ(expected_from_last_keystroke, from_last_keystroke);
+  EXPECT_EQ(expected_delay_ms, delay_ms);
+}
+
 // Test if GetDisabledProviderTypes() properly parses various field trial
 // group names.
 TEST_F(OmniboxFieldTrialTest, GetDisabledProviderTypes) {
@@ -378,3 +412,34 @@ TEST_F(OmniboxFieldTrialTest, HalfLifeTimeDecay) {
   EXPECT_EQ(1.0, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(0)));
   EXPECT_EQ(1.0, buckets.HalfLifeTimeDecay(base::TimeDelta::FromDays(-1)));
 }
+
+TEST_F(OmniboxFieldTrialTest, DisableResultsCaching) {
+  EXPECT_FALSE(OmniboxFieldTrial::DisableResultsCaching());
+
+  {
+    std::map<std::string, std::string> params;
+    params[std::string(OmniboxFieldTrial::kDisableResultsCachingRule)] = "true";
+    ASSERT_TRUE(variations::AssociateVariationParams(
+        OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A", params));
+    base::FieldTrialList::CreateFieldTrial(
+        OmniboxFieldTrial::kBundledExperimentFieldTrialName, "A");
+
+    EXPECT_TRUE(OmniboxFieldTrial::DisableResultsCaching());
+  }
+}
+
+TEST_F(OmniboxFieldTrialTest, GetSuggestPollingStrategy) {

[Mark P, 2014/10/22 23:03:15]

The function this test uses VerifySuggestPollingStrategy() always sets both
parameters.  Can you revise this framework to allow testing that things work
right if only one of the polling strategy parameter fields is set?

[Bart N., 2014/10/23 01:42:53]

Done.

+  // Invalid params.
+  VerifySuggestPollingStrategy(
+      "", "", false,
+      OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs);
+  VerifySuggestPollingStrategy(
+      "foo", "-1", false,
+      OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs);
+  VerifySuggestPollingStrategy(
+      "TRUE", "xyz", false,
+      OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs);
+
+  // Valid params.
+  VerifySuggestPollingStrategy("true", "50", true, 50);
+}

[Mark P, 2014/10/22 23:03:15]

Please add a test for the default values of this polling strategy.

[Bart N., 2014/10/23 01:42:54]

Done.