Omnibox: Combine Two Input Type Enums into One

chrome/browser/autocomplete/search_provider.cc

Index: chrome/browser/autocomplete/search_provider.cc
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 22b8bb32f3d04a3dc0cf3bc81721ae30fa6a7bc8..96c484c1e3e16c764f0fdcd9c5d73475ee5cd8b7 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -39,6 +39,7 @@
 #include "chrome/browser/ui/search/instant_controller.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
+#include "components/metrics/proto/omnibox_input_type.pb.h"
 #include "content/public/browser/user_metrics.h"
 #include "grit/generated_resources.h"
 #include "net/base/escape.h"
@@ -176,7 +177,7 @@ int SearchProvider::CalculateRelevanceForKeywordVerbatim(
   // describe it, so it's clear why the functions diverge.
   if (prefer_keyword)
     return 1500;
-  return (type == AutocompleteInput::QUERY) ? 1450 : 1100;
+  return (type == metrics::OmniboxInputType::QUERY) ? 1450 : 1100;
 }
 
 void SearchProvider::Start(const AutocompleteInput& input,
@@ -191,7 +192,7 @@ void SearchProvider::Start(const AutocompleteInput& input,
   field_trial_triggered_ = false;
 
   // Can't return search/suggest results for bogus input or without a profile.
-  if (!profile_ || (input.type() == AutocompleteInput::INVALID)) {
+  if (!profile_ || (input.type() == metrics::OmniboxInputType::INVALID)) {
     Stop(true);
     return;
   }
@@ -549,7 +550,7 @@ bool SearchProvider::IsQuerySuitableForSuggest() const {
 
   // FORCED_QUERY means the user is explicitly asking us to search for this, so
   // we assume it isn't a URL and/or there isn't private data.
-  if (input_.type() == AutocompleteInput::FORCED_QUERY)
+  if (input_.type() == metrics::OmniboxInputType::FORCED_QUERY)
     return true;
 
   // Next we check the scheme.  If this is UNKNOWN/URL with a scheme that isn't
@@ -565,7 +566,7 @@ bool SearchProvider::IsQuerySuitableForSuggest() const {
   if (!LowerCaseEqualsASCII(input_.scheme(), url::kHttpScheme) &&
       !LowerCaseEqualsASCII(input_.scheme(), url::kHttpsScheme) &&
       !LowerCaseEqualsASCII(input_.scheme(), url::kFtpScheme))
-    return (input_.type() == AutocompleteInput::QUERY);
+    return (input_.type() == metrics::OmniboxInputType::QUERY);
 
   // Don't send URLs with usernames, queries or refs.  Some of these are
   // private, and the Suggest server is unlikely to have any useful results
@@ -579,7 +580,8 @@ bool SearchProvider::IsQuerySuitableForSuggest() const {
   const url::Parsed& parts = input_.parts();
   if (parts.username.is_nonempty() || parts.port.is_nonempty() ||
       parts.query.is_nonempty() ||
-      (parts.ref.is_nonempty() && (input_.type() == AutocompleteInput::URL)))
+      (parts.ref.is_nonempty() &&
+       (input_.type() == metrics::OmniboxInputType::URL)))
     return false;
 
   // Don't send anything for https except the hostname.  Hostnames are OK
@@ -812,7 +814,7 @@ bool SearchProvider::HasKeywordDefaultMatchInKeywordMode() const {
 
 bool SearchProvider::IsTopMatchSearchWithURLInput() const {
   ACMatches::const_iterator first_match = FindTopMatch();
-  return (input_.type() == AutocompleteInput::URL) &&
+  return (input_.type() == metrics::OmniboxInputType::URL) &&
       (first_match != matches_.end()) &&
       (first_match->relevance > CalculateRelevanceForVerbatim()) &&
       (first_match->type != AutocompleteMatchType::NAVSUGGEST) &&
@@ -841,7 +843,7 @@ void SearchProvider::AddHistoryResultsToMap(const HistoryResults& results,
 
   base::TimeTicks start_time(base::TimeTicks::Now());
   bool prevent_inline_autocomplete = input_.prevent_inline_autocomplete() ||
-      (input_.type() == AutocompleteInput::URL);
+      (input_.type() == metrics::OmniboxInputType::URL);
   const base::string16& input_text =
       is_keyword ? keyword_input_.text() : input_.text();
   bool input_multiple_words = HasMultipleWords(input_text);
@@ -987,18 +989,20 @@ int SearchProvider::CalculateRelevanceForVerbatim() const {
 int SearchProvider::
     CalculateRelevanceForVerbatimIgnoringKeywordModeState() const {
   switch (input_.type()) {
-    case AutocompleteInput::UNKNOWN:
-    case AutocompleteInput::QUERY:
-    case AutocompleteInput::FORCED_QUERY:
+    case metrics::OmniboxInputType::UNKNOWN:
+    case metrics::OmniboxInputType::QUERY:
+    case metrics::OmniboxInputType::FORCED_QUERY:
       return kNonURLVerbatimRelevance;
 
-    case AutocompleteInput::URL:
+    case metrics::OmniboxInputType::URL:
       return 850;
 
-    default:
-      NOTREACHED();
-      return 0;
+    case metrics::OmniboxInputType::INVALID:
+    case metrics::OmniboxInputType::DEPRECATED_REQUESTED_URL:

[Peter Kasting, 2014/06/06 20:42:33]

I'm actually going to push back and say that I disagree with listing all the
enum elements in this switch, and think the old code was better than either
this, or this + break/return.

The other place in this CL you did this was fine, but in this one, I'd prefer
how it was done before.

[Mark P, 2014/06/06 21:24:52]

I went back to the old way.

The reason the old way isn't so bad is that if one adds a type without adding
something to this case, things will crash the instant the user types a key into
the omnibox.  (This function gets run all the time.)  In the other function is
called much more rarely (I think only in debug code), which makes forgetting to
add a value there much less likely to be discovered.

+      ;

[Ilya Sherman, 2014/06/06 20:29:24]

nit: I'd have a break or a return.  Just a semicolon looks pretty odd.

[Mark P, 2014/06/06 21:24:52]

Took Peter's advice; see comment there.

   }
+  NOTREACHED();
+  return 0;
 }
 
 int SearchProvider::GetKeywordVerbatimRelevance(
@@ -1059,7 +1063,7 @@ int SearchProvider::CalculateRelevanceForHistory(
   // a different way.
   int base_score;
   if (is_primary_provider)
-    base_score = (input_.type() == AutocompleteInput::URL) ? 750 : 1050;
+    base_score = (input_.type() == metrics::OmniboxInputType::URL) ? 750 : 1050;
   else
     base_score = 200;
   return std::max(0, base_score - score_discount);
@@ -1099,7 +1103,7 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
                          &inline_autocomplete_offset));
   // Preserve the forced query '?' prefix in |match.fill_into_edit|.
   // Otherwise, user edits to a suggestion would show non-Search results.
-  if (input_.type() == AutocompleteInput::FORCED_QUERY) {
+  if (input_.type() == metrics::OmniboxInputType::FORCED_QUERY) {
     match.fill_into_edit.insert(0, base::ASCIIToUTF16("?"));
     if (inline_autocomplete_offset != base::string16::npos)
       ++inline_autocomplete_offset;