Omnibox UI Experiments: Clean up naming for Vertical Margin

components/omnibox/browser/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/browser/omnibox_field_trial.h"
 
 #include <cmath>
 #include <string>
 
 #include "base/command_line.h"
@@ skipping 46 matching lines @@
 // example, loading per-user models into memory.  Having a per-user model
 // in memory allows the suggest server to respond more quickly with
 // personalized suggestions as the user types.
 const base::Feature kSearchProviderWarmUpOnFocus{
     "OmniboxWarmUpSearchProviderOnFocus", base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Feature used to enable the transmission of HTTPS URLs as part of the
 // context to the suggest server (assuming SearchProvider is permitted to
 // transmit URLs for context in the first place).
 const base::Feature kSearchProviderContextAllowHttpsUrls{
     "OmniboixSearchProviderContextAllowHttpsUrls",

[Peter Kasting, 2017/05/10 20:26:34]

Nit: I just noticed "omnibox" is misspelled here.

[tommycli, 2017/05/10 20:30:39]

Done.

[tommycli, 2017/05/10 20:35:54]

Gah, actually I can't change this, since a Finch experiment is already running
with the misspelled name. We can fix in a separate CL....

     base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Feature used for the Zero Suggest Redirect to Chrome Field Trial.
 const base::Feature kZeroSuggestRedirectToChrome{
     "ZeroSuggestRedirectToChrome", base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Feature used to swap the title and URL when providing zero suggest
 // suggestions.
 const base::Feature kZeroSuggestSwapTitleAndUrl{
     "ZeroSuggestSwapTitleAndUrl", base::FEATURE_DISABLED_BY_DEFAULT};
 
 // Feature used to display the title of the current URL match.
 const base::Feature kDisplayTitleForCurrentUrl{
     "OmniboxDisplayTitleForCurrentUrl", base::FEATURE_DISABLED_BY_DEFAULT};
 
-// Feature used for UI experiments.
-const base::Feature kUIExperiments{"OmniboxUIExperiments",
-                                   base::FEATURE_DISABLED_BY_DEFAULT};
+// Feature used for the Vertical Margin UI experiment.

[Peter Kasting, 2017/05/10 20:26:34]

Nit: Maybe wouldn't capitalize V, M?

[tommycli, 2017/05/10 20:30:39]

Done.

+const base::Feature kUIExperimentVerticalMargin{
+    "OmniboxUIExperimentVerticalMargin", base::FEATURE_DISABLED_BY_DEFAULT};

[Peter Kasting, 2017/05/10 20:26:34]

We're not very consistent about whether to put "Omnibox" in front of these
things.  It seems like we should be.  Can we just always remove it?

[tommycli, 2017/05/10 20:30:39]

I would probably advocate keeping it, since this string also appears in
enums.xml, where there's no context. If it just said "ExperimentVerticalMargin"
it might be confusing.

[Peter Kasting, 2017/05/10 20:31:42]

Should we add it to the things that don't have it then?  Or is my desire for
consistency misguided here?

[tommycli, 2017/05/10 20:35:54]

I think that would be a good thing. But would that mess up finch configs? It
would probably also mess up existing histogram data for experiments that have
already been running.

In either case probably a separate CL right?

[Peter Kasting, 2017/05/10 20:43:51]

Maybe?  I dunno.  My job as reviewer is to lob the hard questions at you, then
run.  :D

 
 }  // namespace omnibox
 
 namespace {
 
 typedef std::map<std::string, std::string> VariationParams;
 typedef HUPScoringParams::ScoreBuckets ScoreBuckets;
 
 // Field trial names.
 const char kStopTimerFieldTrialName[] = "OmniboxStopTimer";
@@ skipping 637 matching lines @@
     "PhysicalWebZeroSuggestBaseRelevance";
 const char OmniboxFieldTrial::kPhysicalWebAfterTypingBaseRelevanceParam[] =
     "PhysicalWebAfterTypingBaseRelevanceParam";
 
 const char OmniboxFieldTrial::kZeroSuggestRedirectToChromeServerAddressParam[] =
     "ZeroSuggestRedirectToChromeServerAddress";
 const char
     OmniboxFieldTrial::kZeroSuggestRedirectToChromeAdditionalFieldsParam[] =
         "ZeroSuggestRedirectToChromeAdditionalFields";
 
-const char OmniboxFieldTrial::kUIExperimentsVerticalMarginParam[] =
-    "UIExperimentsVerticalMargin";
+const char OmniboxFieldTrial::kUIVerticalMarginParam[] = "UIVerticalMargin";
 
 // static
 int OmniboxFieldTrial::kDefaultMinimumTimeBetweenSuggestQueriesMs = 100;
 
 // Background and implementation details:
 //
 // Each experiment group in any field trial can come with an optional set of
 // parameters (key-value pairs).  In the bundled omnibox experiment
 // (kBundledExperimentFieldTrialName), each experiment group comes with a
 // list of parameters in the form:
@@ skipping 46 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();
 }