Omnibox: Combine Two Input Type Enums into One

chrome/browser/autocomplete/autocomplete_input.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_INPUT_H_
 #define CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_INPUT_H_
 
 #include <string>
 
 #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
   // omnibox_edit_model.cc::ClassifyPage() too.
   enum PageClassification {
     // An invalid URL; shouldn't happen.
     INVALID_SPEC = 0,
 
     // chrome://newtab/.  This can be either the built-in version or a
     // replacement new tab page from an extension.  Note that when Instant
@@ skipping 81 matching lines @@
                     const GURL& current_url,
                     PageClassification current_page_classification,
                     bool prevent_inline_autocomplete,
                     bool prefer_keyword,
                     bool allow_exact_keyword_match,
                     bool want_asynchronous_matches);
   ~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
   // is view-source, this function returns the positions of scheme and host
   // in the URL qualified by "view-source:" prefix.
   static void ParseForEmphasizeComponents(const base::string16& text,
                                           url::Component* scheme,
                                           url::Component* host);
 
   // Code that wants to format URLs with a format flag including
@@ skipping 29 matching lines @@
   // The current URL, or an invalid GURL if query refinement is not desired.
   const GURL& current_url() const { return current_url_; }
 
   // The type of page that is currently behind displayed and how it is
   // displayed (e.g., with search term replacement or without).
   AutocompleteInput::PageClassification current_page_classification() const {
     return current_page_classification_;
   }
 
   // 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_; }
 
   // The scheme parsed from the provided text; only meaningful when type_ is
   // URL.
   const base::string16& scheme() const { return scheme_; }
 
   // The input as an URL to navigate to, if possible.
   const GURL& canonicalized_url() const { return canonicalized_url_; }
@@ skipping 21 matching lines @@
 
  private:
   FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, GetDestinationURL);
 
   // NOTE: Whenever adding a new field here, please make sure to update Clear()
   // method.
   base::string16 text_;
   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_;
   bool prevent_inline_autocomplete_;
   bool prefer_keyword_;
   bool allow_exact_keyword_match_;
   bool want_asynchronous_matches_;
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_INPUT_H_