Omnibox: Create Disable-Inlining Experiment

chrome/browser/autocomplete/autocomplete_result.cc

Index: chrome/browser/autocomplete/autocomplete_result.cc
diff --git a/chrome/browser/autocomplete/autocomplete_result.cc b/chrome/browser/autocomplete/autocomplete_result.cc
index b4637f67a0b9ec77c44a5a48db3156e87c99ecde..d3ddd7b5fcfff620dcd3b0ce5e9886a3832f23eb 100644
--- a/chrome/browser/autocomplete/autocomplete_result.cc
+++ b/chrome/browser/autocomplete/autocomplete_result.cc
@@ -8,6 +8,7 @@
 #include <iterator>
 
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_input.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
@@ -177,16 +178,41 @@ void AutocompleteResult::SortAndCull(const AutocompleteInput& input,
   DedupMatchesByDestination(input.current_page_classification(), true,
                             &matches_);
 
+  // If the result set has at least one legal default match without an
+  // inline autocompletion, then in the disable inlining experiment it's

[Peter Kasting, 2014/05/29 21:30:49]

Nit: it's -> it

[Mark P, 2014/05/30 00:03:58]

Done.  Also adjusted wrapping.

+  // will be okay to demote all matches with inline autocompletions.  On
+  // the other hand, if the experiment is active but there is no legal
+  // match without an inline autocompletion, then we'll pretend the
+  // experiment is not active and not demote the matches with an inline
+  // autocompletion.  In other words, an alternate name for this variable is
+  // allowed_to_demote_matches_with_inline_autocompletion.
+  bool has_legal_default_match_without_completion = false;
+  for (AutocompleteResult::iterator it = matches_.begin();
+       (it != matches_.end()) && !has_legal_default_match_without_completion;
+       ++it) {
+    if (it->allowed_to_be_default_match && it->inline_autocompletion.empty())
+      has_legal_default_match_without_completion = true;
+  }
+  UMA_HISTOGRAM_BOOLEAN("Omnibox.HasLegalDefaultMatchWithoutCompletion",
+                        has_legal_default_match_without_completion);
+
   // Sort and trim to the most relevant kMaxMatches matches.
   size_t max_num_matches = std::min(kMaxMatches, matches_.size());
   CompareWithDemoteByType comparing_object(input.current_page_classification());
   std::sort(matches_.begin(), matches_.end(), comparing_object);
-  if (!matches_.empty() && !matches_.begin()->allowed_to_be_default_match) {
+  if (!matches_.empty() &&
+      (!matches_.begin()->allowed_to_be_default_match ||

[Peter Kasting, 2014/05/29 21:30:49]

I might create a small helper functions which does:

  return it->allowed_to_be_default_match &&
      (!OmniboxFieldTrial::DisableInlining() ||
       !has_legal_default_match_without_completion ||
       it->inline_autocompletion.empty());

This could be used (negated) here and below.

[Mark P, 2014/05/30 00:03:58]

Done.  The function name is awkward though.  If you have a better suggestion I'm
all ears.

+       (OmniboxFieldTrial::DisableInlining() &&
+        has_legal_default_match_without_completion &&
+        !matches_.begin()->inline_autocompletion.empty()))) {
     // Top match is not allowed to be the default match.  Find the most
     // relevant legal match and shift it to the front.
     for (AutocompleteResult::iterator it = matches_.begin() + 1;
          it != matches_.end(); ++it) {
-      if (it->allowed_to_be_default_match) {
+      if (it->allowed_to_be_default_match &&
+          (!OmniboxFieldTrial::DisableInlining() ||
+           !has_legal_default_match_without_completion ||
+           it->inline_autocompletion.empty())) {
         std::rotate(matches_.begin(), it, it + 1);
         break;
       }