Update BuiltinProvider to use chrome:// URLs.

chrome/browser/autocomplete/builtin_provider.cc

Index: chrome/browser/autocomplete/builtin_provider.cc
diff --git a/chrome/browser/autocomplete/builtin_provider.cc b/chrome/browser/autocomplete/builtin_provider.cc
index 507d9ed775113e31a50a4a7c8ce55050018e04ca..13844a698e7c9288a1d7d5b905bfe0bd93fbc93b 100644
--- a/chrome/browser/autocomplete/builtin_provider.cc
+++ b/chrome/browser/autocomplete/builtin_provider.cc
@@ -6,9 +6,44 @@
 
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
-#include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/browser_about_handler.h"
 #include "chrome/browser/net/url_fixer_upper.h"
+#include "chrome/common/url_constants.h"
+
+namespace {
+
+const string16 kAbout = ASCIIToUTF16(chrome::kAboutScheme) +

[Peter Kasting, 2011/06/10 18:50:35]

Declaring global instances of strings violates the style guide.  These have to
be char* or non-global-scope.

I'd probably just create them within the functions that use them, there won't be
a discernable speed hit.

[msw, 2011/06/11 21:49:48]

Done.

+                        ASCIIToUTF16(chrome::kStandardSchemeSeparator);
+const string16 kChrome = ASCIIToUTF16(chrome::kChromeUIScheme) +
+                         ASCIIToUTF16(chrome::kStandardSchemeSeparator);
+const size_t kAboutLength = strlen(chrome::kAboutScheme);
+
+const int kUrl = ACMatchClassification::URL;
+const int kMatch = ACMatchClassification::URL | ACMatchClassification::MATCH;

[Peter Kasting, 2011/06/10 18:50:35]

Nit: I'd probably declare these in the function where they're used, too, to
scope them as narrowly as possible.

[msw, 2011/06/11 21:49:48]

Done.

+
+// This list should be kept in sync with chrome/common/url_constnats.h.

[Peter Kasting, 2011/06/10 18:50:35]

Nit: constnats -> constants.  Would it make sense to have this list over in that
header instead?

[msw, 2011/06/11 21:49:48]

Fixed constants, the list isn't used elsewhere, so I'll keep it here for now.

+const char *kChromeSettingsSubPages[] = {

[Peter Kasting, 2011/06/10 18:50:35]

Nit: * goes on type, not name

[msw, 2011/06/11 21:49:48]

Done. Also fixed browser_about_handler.cc.

+  chrome::kAdvancedOptionsSubPage,
+  chrome::kAutofillSubPage,
+  chrome::kBrowserOptionsSubPage,
+  chrome::kClearBrowserDataSubPage,
+  chrome::kContentSettingsSubPage,
+  chrome::kContentSettingsExceptionsSubPage,
+  chrome::kImportDataSubPage,
+  chrome::kInstantConfirmPage,
+  chrome::kLanguageOptionsSubPage,
+  chrome::kPersonalOptionsSubPage,
+  chrome::kPasswordManagerSubPage,
+  chrome::kSearchEnginesSubPage,
+  chrome::kSyncSetupSubPage,
+#if defined(OS_CHROMEOS)
+  chrome::kAboutOptionsSubPage,
+  chrome::kInternetOptionsSubPage,
+  chrome::kSystemOptionsSubPage,
+#endif
+};
+
+}  // namespace
 
 const int BuiltinProvider::kRelevance = 575;
 
@@ -17,8 +52,12 @@ BuiltinProvider::BuiltinProvider(ACProviderListener* listener,
     : AutocompleteProvider(listener, profile, "Builtin") {
   std::vector<std::string> builtins(ChromePaths());
   for (std::vector<std::string>::iterator i(builtins.begin());
-       i != builtins.end(); ++i)
-    builtins_.push_back(ASCIIToUTF16("about:") + ASCIIToUTF16(*i));
+    i != builtins.end(); ++i)
+      builtins_.push_back(ASCIIToUTF16(*i));
+  string16 settings(ASCIIToUTF16(chrome::kChromeUISettingsHost));
+  settings.append(ASCIIToUTF16("/"));

[Peter Kasting, 2011/06/10 18:50:35]

Nit: Or you could just do "... + '/'" in the previous initializer, I think that
ought to work.

[msw, 2011/06/11 21:49:48]

Done (sorta, can't add ASCII char/string to string16/char[] afaik).

+  for (size_t i = 0; i < arraysize(kChromeSettingsSubPages); i++)
+    builtins_.push_back(settings + ASCIIToUTF16(kChromeSettingsSubPages[i]));
 }
 
 BuiltinProvider::~BuiltinProvider() {}
@@ -31,24 +70,58 @@ void BuiltinProvider::Start(const AutocompleteInput& input,
       (input.type() == AutocompleteInput::QUERY) ||
       (input.matches_requested() == AutocompleteInput::BEST_MATCH))
     return;
-  for (Builtins::const_iterator i(builtins_.begin());
-       (i != builtins_.end()) && (matches_.size() < kMaxMatches); ++i) {
-    if (StartsWith(*i, input.text(), false)) {
-      AutocompleteMatch match(this, kRelevance, false,
-                              AutocompleteMatch::NAVSUGGEST);
-      match.fill_into_edit = *i;
-      match.destination_url = GURL(*i);
-      match.contents = match.fill_into_edit;
-      match.contents_class.push_back(ACMatchClassification(0,
-          ACMatchClassification::MATCH | ACMatchClassification::URL));
-      if (match.contents.length() > input.text().length()) {
-        match.contents_class.push_back(
-            ACMatchClassification(input.text().length(),
-                                  ACMatchClassification::URL));
+
+  string16 text = input.text();
+  bool starting_chrome = StartsWith(kChrome, text, false);
+  if (starting_chrome || StartsWith(kAbout, text, false)) {
+    ACMatchClassifications styles;
+    // Highlight the matching input, or "chrome://" given input "about:".
+    bool highlight = starting_chrome || text.length() > kAboutLength;

[Peter Kasting, 2011/06/10 18:50:35]

How can text.length() be larger than |kAboutLength|, when |kAbout| starts with
|text|?

[msw, 2011/06/11 21:49:48]

If the user typed |text| "about:", "about:/", or "about://" then it's longer
than |chrome::kAboutScheme| "about", and I'll highlight the "chrome://". I
adjusted the comment for clarity.

+    styles.push_back(ACMatchClassification(0, highlight ? kMatch : kUrl));
+    size_t offset = starting_chrome ? text.length() : kChrome.length();
+    styles.push_back(ACMatchClassification(offset, kUrl));
+    // Add common URLs as the user types the "about://" or "chrome://" scheme.
+    AddMatch(text, ASCIIToUTF16(chrome::kChromeUIVersionHost), styles);
+    AddMatch(text, ASCIIToUTF16(chrome::kChromeUISettingsHost), styles);
+    AddMatch(text, ASCIIToUTF16(chrome::kChromeUIChromeURLsHost), styles);
+  } else {
+    // Match input about: or chrome: URL hosts against builtin chrome URL hosts.
+    GURL fixed_url = URLFixerUpper::FixupURL(UTF16ToUTF8(text), std::string());
+    if (fixed_url.SchemeIs(chrome::kChromeUIScheme) && fixed_url.has_host()) {
+      for (Builtins::const_iterator i(builtins_.begin());
+         (i != builtins_.end()) && (matches_.size() < kMaxMatches); ++i) {
+        // Include the path for sub-pages (e.g. "chrome://settings/browser").

[Peter Kasting, 2011/06/10 18:50:35]

Shouldn't this whole |host_and_path| construction be hoisted outside this inner
loop?

[msw, 2011/06/11 21:49:48]

Done.

+        string16 host_and_path = UTF8ToUTF16(fixed_url.host());
+        host_and_path.append(UTF8ToUTF16(fixed_url.path()));

[Peter Kasting, 2011/06/10 18:50:35]

Nit: I'd just combine these two lines using the + operator

[msw, 2011/06/11 21:49:48]

Done.

+        // Trim the trailing slash of the path string.
+        if (EndsWith(host_and_path, ASCIIToUTF16("/"), true))
+          host_and_path = host_and_path.substr(0, host_and_path.length()-1);

[Peter Kasting, 2011/06/10 18:50:35]

Nit: Spaces around operator

[msw, 2011/06/11 21:49:48]

Done.

+        if (StartsWith(*i, host_and_path, false)) {
+          ACMatchClassifications styles;
+          // Highlight the "chrome://" scheme, even for input "about:foo".
+          styles.push_back(ACMatchClassification(0, kMatch));
+          size_t match_length = kChrome.length() + host_and_path.length();
+          if ((kChrome.length() + i->length()) > match_length)
+            styles.push_back(ACMatchClassification(match_length, kUrl));
+          AddMatch(text, *i, styles);
+        }
       }
-      matches_.push_back(match);
     }
   }
+
   for (size_t i = 0; i < matches_.size(); ++i)
     matches_[i].relevance = kRelevance + matches_.size() - (i + 1);
 }
+
+void BuiltinProvider::AddMatch(const string16& input,
+                               const string16& match_string,
+                               const ACMatchClassifications& styles) {
+  AutocompleteMatch match(this, kRelevance, false,
+                          AutocompleteMatch::NAVSUGGEST);
+  match.fill_into_edit = kChrome + match_string;
+  match.destination_url = GURL(match.fill_into_edit);
+  match.contents = match.fill_into_edit;
+  match.contents_class = styles;
+  matches_.push_back(match);
+}
+