Parametrize Suggest polling strategy and delay. Also add a parameter

components/omnibox/omnibox_field_trial.cc

 // Copyright 2014 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 "components/omnibox/omnibox_field_trial.h"
 
 #include <cmath>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 318 matching lines @@
       kBundledExperimentFieldTrialName,
       kAddUWYTMatchEvenIfPromotedURLsRule) == "true";
 }
 
 bool OmniboxFieldTrial::DisplayHintTextWhenPossible() {
   return variations::GetVariationParamValue(
       kBundledExperimentFieldTrialName,
       kDisplayHintTextWhenPossibleRule) == "true";
 }
 
+bool OmniboxFieldTrial::DisableResultsCaching() {
+  return variations::GetVariationParamValue(
+      kBundledExperimentFieldTrialName,
+      kDisableResultsCachingRule) == "true";
+}
+
+void OmniboxFieldTrial::GetSuggestPollingStrategy(bool* since_the_last_request,
+                                                  int* polling_delay_ms) {
+  *since_the_last_request = variations::GetVariationParamValue(
+      kBundledExperimentFieldTrialName,
+      kSuggestPollingDelaySinceTheLastRequestRule) == "true";

[H Fung, 2014/10/20 21:59:42]

I'm not sure if you want to have a flag to take in when to reset the counter,
such as "keystroke" and "request" instead of using true/false.

[Bart N., 2014/10/22 01:19:08]

I think I'd prefer to stay with the current rule, since we are not planning to
add any new type soon. If so, we can change it easily.

+
+  const std::string& polling_delay_string = variations::GetVariationParamValue(
+      kBundledExperimentFieldTrialName,
+      kSuggestPollingDelayMsRule);
+  if (polling_delay_string.empty() ||
+      !base::StringToInt(polling_delay_string, polling_delay_ms) ||
+      *polling_delay_ms <= 0) {

[Mark P, 2014/10/21 21:28:22]

nit: parans around binary operator <=

[Bart N., 2014/10/22 01:19:07]

Done.

+    // Status Quo (100ms since the last suggest request).

[Mark P, 2014/10/21 21:28:22]

This comment is helpful for reviewers.  For the code, I'd either suggest your
remove it or phrase it differently such as
// Use a default of 100ms since the last suggest request.

(once you see that phrasing, you'll see what I mean when you say it's not
necessary in the code)

[Bart N., 2014/10/22 01:19:08]

Done.

+    *polling_delay_ms = 100;
+  }
+}
+
 const char OmniboxFieldTrial::kBundledExperimentFieldTrialName[] =
     "OmniboxBundledExperimentV1";
 const char OmniboxFieldTrial::kShortcutsScoringMaxRelevanceRule[] =
     "ShortcutsScoringMaxRelevance";
 const char OmniboxFieldTrial::kSearchHistoryRule[] = "SearchHistory";
 const char OmniboxFieldTrial::kDemoteByTypeRule[] = "DemoteByType";
 const char OmniboxFieldTrial::kHQPBookmarkValueRule[] =
     "HQPBookmarkValue";
 const char OmniboxFieldTrial::kHQPAllowMatchInTLDRule[] = "HQPAllowMatchInTLD";
 const char OmniboxFieldTrial::kHQPAllowMatchInSchemeRule[] =
     "HQPAllowMatchInScheme";
 const char OmniboxFieldTrial::kZeroSuggestRule[] = "ZeroSuggest";
 const char OmniboxFieldTrial::kZeroSuggestVariantRule[] = "ZeroSuggestVariant";
 const char OmniboxFieldTrial::kDisableInliningRule[] = "DisableInlining";
 const char OmniboxFieldTrial::kAnswersInSuggestRule[] = "AnswersInSuggest";
 const char OmniboxFieldTrial::kAddUWYTMatchEvenIfPromotedURLsRule[] =
     "AddUWYTMatchEvenIfPromotedURLs";
 const char OmniboxFieldTrial::kDisplayHintTextWhenPossibleRule[] =
     "DisplayHintTextWhenPossible";
+const char OmniboxFieldTrial::kDisableResultsCachingRule[] =
+    "DisableResultsCaching";
+const char OmniboxFieldTrial::kSuggestPollingDelaySinceTheLastRequestRule[] =
+    "SuggestPollingDelaySinceTheLastRequest";

[Mark P, 2014/10/21 21:28:22]

If you change the name of the rule, please revise the string here too.

[Bart N., 2014/10/22 01:19:08]

Done.

+const char OmniboxFieldTrial::kSuggestPollingDelayMsRule[] =

[H Fung, 2014/10/20 21:59:42]

This is a param, not a rule?

[Mark P, 2014/10/21 21:28:22]

It's a "rule".  In this file, rule correspond to the key part of the key-value
field that make up the params structure.  The value part of the param variables.
 I know it's weird nomenclature.

[Bart N., 2014/10/22 01:19:08]

Acknowledged.

[Bart N., 2014/10/22 01:19:08]

Acknowledged.

+    "SuggestPollingDelayMs";
 
 const char OmniboxFieldTrial::kHUPNewScoringEnabledParam[] =
     "HUPExperimentalScoringEnabled";
 const char OmniboxFieldTrial::kHUPNewScoringTypedCountRelevanceCapParam[] =
     "TypedCountRelevanceCap";
 const char OmniboxFieldTrial::kHUPNewScoringTypedCountHalfLifeTimeParam[] =
     "TypedCountHalfLifeTime";
 const char OmniboxFieldTrial::kHUPNewScoringTypedCountScoreBucketsParam[] =
     "TypedCountScoreBuckets";
 const char OmniboxFieldTrial::kHUPNewScoringVisitedCountRelevanceCapParam[] =
@@ skipping 58 matching lines @@
   if (it != params.end())
     return it->second;
   // Fall back to the global instant extended context.
   it = params.find(rule + ":" + page_classification_str + ":*");
   if (it != params.end())
     return it->second;
   // Look up rule in the global context.
   it = params.find(rule + ":*:*");
   return (it != params.end()) ? it->second : std::string();
 }