Omnibox: Combine Two Input Type Enums into One

chrome/browser/autocomplete/autocomplete_input.h

Index: chrome/browser/autocomplete/autocomplete_input.h
diff --git a/chrome/browser/autocomplete/autocomplete_input.h b/chrome/browser/autocomplete/autocomplete_input.h
index b923e7b6542163ba51d00ce44c995eec3c0da602..8148baef106902ba1b227818d395026bc337073b 100644
--- a/chrome/browser/autocomplete/autocomplete_input.h
+++ b/chrome/browser/autocomplete/autocomplete_input.h
@@ -10,24 +10,13 @@
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/strings/string16.h"
+#include "components/metrics/proto/omnibox_input_type.pb.h"
 #include "url/gurl.h"
 #include "url/url_parse.h"
 
 // The user input for an autocomplete query.  Allows copying.
 class AutocompleteInput {
  public:
-  // Note that the type below may be misleading.  For example, "http:/" alone
-  // cannot be opened as a URL, so it is marked as a QUERY; yet the user
-  // probably intends to type more and have it eventually become a URL, so we
-  // need to make sure we still run it through inline autocomplete.
-  enum Type {
-    INVALID,        // Empty input
-    UNKNOWN,        // Valid input whose type cannot be determined
-    URL,            // Input autodetected as a URL
-    QUERY,          // Input autodetected as a query
-    FORCED_QUERY,   // Input forced to be a query by an initial '?'
-  };
-
   // The type of page currently displayed.
   // Note: when adding an element to this enum, please add it at the end
   // and update omnibox_event.proto::PageClassification and
@@ -129,22 +118,23 @@ class AutocompleteInput {
 
   // If type is |FORCED_QUERY| and |text| starts with '?', it is removed.
   // Returns number of leading characters removed.
-  static size_t RemoveForcedQueryStringIfNecessary(Type type,
-                                                   base::string16* text);
+  static size_t RemoveForcedQueryStringIfNecessary(
+      metrics::OmniboxInputType::Type type, base::string16* text);

[Ilya Sherman, 2014/06/06 05:01:01]

Optional nit: Within the scope of the class, you could "typedef
metrics::OmniboxInputType::Type Type;" if you wanted to.

[Peter Kasting, 2014/06/06 18:13:07]

I think this would be a good idea.  It would eliminate most of the changes in
this file and reduce the impact of the changes in the .cc file.

This would also let you change other files that current do
"AutocompleteInput::FOO" to "AutocompleteInput::Type::FOO" instead of referring
to metrics::OmniboxInputType or pulling in any additional headers, which I think
would also be clearer.

[Mark P, 2014/06/06 20:22:51]

I hoped I would get such a sweet suggestion.  Thanks Ilya!  Done.  This
shortened a lot of code everywhere.
This change doesn't let me AutocompleteInput::Type::FOO.
"../../chrome/browser/autocomplete/search_provider.cc:180:38: error:
‘AutocompleteInput::Type’ is not a class or namespace"

Nor does it work without the Type:: blip.
../../chrome/browser/autocomplete/search_provider.cc:180:19: error: ‘QUERY’ is
not a member of ‘AutocompleteInput’

Know of a way to get the enum values to be visible without AutocompleteInput's
scope?

[Peter Kasting, 2014/06/06 20:37:08]

That's strange.  The following code compiles for me in MSVC 2013:

class A {
public:
  enum Type { ONE, TWO };
};

class B {
public:
  typedef A::Type Foo;
};

void foo() {
  B::Foo foo = B::Foo::ONE;
}

I wonder if this is non-standard, or C++11-only, or something?

I don't have any other ideas of how to avoid using the original, underlying
enum's name.  I found this page:
http://stackoverflow.com/questions/902095/c-typedef-another-classs-enum which
may not help much.

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

Judging by the stackoverflow question, it sounds like what we're trying to do is
non-standard.  And because we have our compiler warnings turned all the way up,
it's getting reported as an error.  I think we're stuck with the icky
metrics::OmniboxInputType:: everywhere.

 
   // Converts |type| to a string representation.  Used in logging.
-  static std::string TypeToString(Type type);
+  static std::string TypeToString(metrics::OmniboxInputType::Type type);
 
   // Parses |text| and returns the type of input this will be interpreted as.
   // The components of the input are stored in the output parameter |parts|, if
   // it is non-NULL. The scheme is stored in |scheme| if it is non-NULL. The
   // canonicalized URL is stored in |canonicalized_url|; however, this URL is
   // not guaranteed to be valid, especially if the parsed type is, e.g., QUERY.
-  static Type Parse(const base::string16& text,
-                    const base::string16& desired_tld,
-                    url::Parsed* parts,
-                    base::string16* scheme,
-                    GURL* canonicalized_url);
+  static metrics::OmniboxInputType::Type Parse(
+      const base::string16& text,
+      const base::string16& desired_tld,
+      url::Parsed* parts,
+      base::string16* scheme,
+      GURL* canonicalized_url);
 
   // Parses |text| and fill |scheme| and |host| by the positions of them.
   // The results are almost as same as the result of Parse(), but if the scheme
@@ -194,7 +184,7 @@ class AutocompleteInput {
   }
 
   // The type of input supplied.
-  Type type() const { return type_; }
+  metrics::OmniboxInputType::Type type() const { return type_; }
 
   // Returns parsed URL components.
   const url::Parsed& parts() const { return parts_; }
@@ -236,7 +226,7 @@ class AutocompleteInput {
   size_t cursor_position_;
   GURL current_url_;
   AutocompleteInput::PageClassification current_page_classification_;
-  Type type_;
+  metrics::OmniboxInputType::Type type_;
   url::Parsed parts_;
   base::string16 scheme_;
   GURL canonicalized_url_;