Allow Search Keywords with ":" or "/"

Previously these keyword would not be recognized because we try to clean
up the user's input before checking to see if it matches a keyword.
This changelist makes use only try various cleaning of the user's input
until we get a match (or tried all the cleaning steps we have).  For
example if the input such as "wiki:" matches a keyword, we don't do any
cleanup.

Tested amply with unit tests.

Also tested interactively.  Some key interactive cases:
* typing an unusual keyword and pressing tab enters keyword mode.
* typing an unusual keyword and pressing space enters keyword mode.
* putting the cursor in the middle of a string and pressing space in
  a while that would make the left side into an unusual keyword puts
  the user into keyword mode
* backspacing out of keyword mode works correctly
* these suggestions for unusual keywords appear in the dropdown when
  the user types a prefix of them.
* when the user is in keyword mode for one of these unusual keywords,
  switching tabs and switching back keeps the user in keyword mode.
* when typing an unusual keyword with an additional thing that can
  be cleaned up (for example www.wiki: or http://wiki:), pressing
  space at the end of the keyword does NOT enter keyword mode.  This
  seems reasonable to me.  Pressing tab at the end of the keyword
  does enter keyword mode.

Yay for fixing a four-digit bug.  (It's one of the ~40 oldest existing
bugs in chromium.)

Bug: 2740
Change-Id: Ie53089421956acf5eb58fe1d1b53932e10169178
Reviewed-on: https://chromium-review.googlesource.com/664251
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502747}

chrome/browser/autocomplete/keyword_extensions_delegate_impl.cc

@@ -143,8 +143,8 @@ void KeywordExtensionsDelegateImpl::Observe(
       // session.
       base::string16 keyword, remaining_input;
       if (matches()->empty() || current_keyword_extension_id_.empty() ||
-          !KeywordProvider::ExtractKeywordFromInput(
-              input, &keyword, &remaining_input))
+          !KeywordProvider::ExtractKeywordFromInput(input, model, &keyword,
+                                                    &remaining_input))
         return;
 
       const TemplateURL* template_url(
@@ -166,7 +166,7 @@ void KeywordExtensionsDelegateImpl::Observe(
       // direct calls from the development console, outside the normal flow of
       // user input.
       base::string16 keyword, remaining_input;
-      if (!KeywordProvider::ExtractKeywordFromInput(input, &keyword,
+      if (!KeywordProvider::ExtractKeywordFromInput(input, model, &keyword,
                                                     &remaining_input))
         return;
       const TemplateURL* template_url =

components/omnibox/browser/keyword_provider.cc

@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <vector>
 
+#include "base/i18n/case_conversion.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
@@ -22,6 +23,7 @@
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/strings/grit/components_strings.h"
+#include "components/url_formatter/url_formatter.h"
 #include "net/base/escape.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -132,7 +134,7 @@ const TemplateURL* KeywordProvider::GetSubstitutingTemplateURLForInput(
     return NULL;
 
   base::string16 keyword, remaining_input;
-  if (!ExtractKeywordFromInput(*input, &keyword, &remaining_input))
+  if (!ExtractKeywordFromInput(*input, model, &keyword, &remaining_input))
     return NULL;
 
   DCHECK(model);
@@ -171,15 +173,15 @@ const TemplateURL* KeywordProvider::GetSubstitutingTemplateURLForInput(
 
 base::string16 KeywordProvider::GetKeywordForText(
     const base::string16& text) const {
-  const base::string16 keyword(TemplateURLService::CleanUserInputKeyword(text));
-
-  if (keyword.empty())
-    return keyword;
-
   TemplateURLService* url_service = GetTemplateURLService();
   if (!url_service)
     return base::string16();
 
+  const base::string16 keyword(CleanUserInputKeyword(url_service, text));
+
+  if (keyword.empty())
+    return keyword;
+
   // Don't provide a keyword if it doesn't support replacement.
   const TemplateURL* const template_url =
       url_service->GetTemplateURLForKeyword(keyword);
@@ -242,7 +244,7 @@ void KeywordProvider::Start(const AutocompleteInput& input,
   // typed, if the user uses them enough and isn't obviously typing something
   // else.  In this case we'd consider all input here to be query input.
   base::string16 keyword, remaining_input;
-  if (!ExtractKeywordFromInput(input, &keyword, &remaining_input))
+  if (!ExtractKeywordFromInput(input, model_, &keyword, &remaining_input))
     return;
 
   // Get the best matches for this keyword.
@@ -356,13 +358,16 @@ void KeywordProvider::Stop(bool clear_cached_results,
 KeywordProvider::~KeywordProvider() {}
 
 // static
-bool KeywordProvider::ExtractKeywordFromInput(const AutocompleteInput& input,
-                                              base::string16* keyword,
-                                              base::string16* remaining_input) {
+bool KeywordProvider::ExtractKeywordFromInput(
+    const AutocompleteInput& input,
+    const TemplateURLService* template_url_service,
+    base::string16* keyword,
+    base::string16* remaining_input) {
   if ((input.type() == metrics::OmniboxInputType::INVALID))
     return false;
 
-  *keyword = TemplateURLService::CleanUserInputKeyword(
+  *keyword = CleanUserInputKeyword(
+      template_url_service,
       SplitKeywordFromInput(input.text(), true, remaining_input));
   return !keyword->empty();
 }
@@ -495,3 +500,49 @@ TemplateURLService* KeywordProvider::GetTemplateURLService() const {
   model_->Load();
   return model_;
 }
+
+// static
+base::string16 KeywordProvider::CleanUserInputKeyword(
+    const TemplateURLService* template_url_service,
+    const base::string16& keyword) {
+  base::string16 result(base::i18n::ToLower(keyword));
+  base::TrimWhitespace(result, base::TRIM_ALL, &result);
+  // If this keyword is found with no additional cleaning of input, return it.
+  if ((template_url_service != nullptr) &&
+      (template_url_service->GetTemplateURLForKeyword(result) != nullptr))
+    return result;
+
+  // If keyword is not found, try removing a "http" or "https" scheme if any.
+  url::Component scheme_component;
+  if (url::ExtractScheme(base::UTF16ToUTF8(result).c_str(),
+                         static_cast<int>(result.length()),
+                         &scheme_component) &&
+      (!result.compare(0, scheme_component.end(),
+                       base::ASCIIToUTF16(url::kHttpScheme)) ||
+       !result.compare(0, scheme_component.end(),
+                       base::ASCIIToUTF16(url::kHttpsScheme)))) {
+    // Remove the scheme and the trailing ':'.
+    result.erase(0, scheme_component.end() + 1);
+    if ((template_url_service != nullptr) &&
+        (template_url_service->GetTemplateURLForKeyword(result) != nullptr))
+      return result;
+    // Many schemes usually have "//" after them, so strip it too.
+    const base::string16 after_scheme(base::ASCIIToUTF16("//"));
+    if (result.compare(0, after_scheme.length(), after_scheme) == 0)
+      result.erase(0, after_scheme.length());
+    if ((template_url_service != nullptr) &&
+        (template_url_service->GetTemplateURLForKeyword(result) != nullptr))
+      return result;
+  }
+
+  // Remove leading "www.", if any, and again try to find a matching keyword.
+  result = url_formatter::StripWWW(result);
+  if ((template_url_service != nullptr) &&
+      (template_url_service->GetTemplateURLForKeyword(result) != nullptr))
+    return result;
+
+  // Remove trailing "/", if any.
+  if (!result.empty() && result.back() == '/')
+    result.pop_back();
+  return result;
+}

components/omnibox/browser/keyword_provider.h

@@ -104,13 +104,16 @@ class KeywordProvider : public AutocompleteProvider {
   // Extracts the keyword from |input| into |keyword|. Any remaining characters
   // after the keyword are placed in |remaining_input|. Returns true if |input|
   // is valid and has a keyword. This makes use of SplitKeywordFromInput to
-  // extract the keyword and remaining string, and uses
-  // TemplateURLService::CleanUserInputKeyword to remove unnecessary characters.
+  // extract the keyword and remaining string, and uses |template_url_service|
+  // to validate and clean up the extracted keyword (e.g., to remove unnecessary
+  // characters).
   // In general use this instead of SplitKeywordFromInput.
   // Leading whitespace in |*remaining_input| will be trimmed.
-  static bool ExtractKeywordFromInput(const AutocompleteInput& input,
-                                      base::string16* keyword,
-                                      base::string16* remaining_input);
+  static bool ExtractKeywordFromInput(
+      const AutocompleteInput& input,
+      const TemplateURLService* template_url_service,
+      base::string16* keyword,
+      base::string16* remaining_input);
 
   // Determines the relevance for some input, given its type, whether the user
   // typed the complete keyword (or close to it), and whether the user is in
@@ -143,6 +146,17 @@ class KeywordProvider : public AutocompleteProvider {
 
   TemplateURLService* GetTemplateURLService() const;
 
+  // Removes any unnecessary characters from a user input keyword, returning
+  // the resulting keyword.  Usually this means it does transformations such as
+  // removing any leading scheme, "www." and trailing slash and returning the
+  // resulting string regardless of whether it's a registered keyword.
+  // However, if a |template_url_service| is provided and the function finds a
+  // registered keyword at any point before finishing those transformations,
+  // it'll return that keyword.
+  static base::string16 CleanUserInputKeyword(
+      const TemplateURLService* template_url_service,
+      const base::string16& keyword);
+
   AutocompleteProviderListener* listener_;
 
   // Model for the keywords.

components/search_engines/template_url_service.cc

@@ -14,7 +14,6 @@
 #include "base/debug/crash_logging.h"
 #include "base/format_macros.h"
 #include "base/guid.h"
-#include "base/i18n/case_conversion.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_split.h"
@@ -40,7 +39,6 @@
 #include "components/sync/protocol/search_engine_specifics.pb.h"
 #include "components/sync/protocol/sync.pb.h"
 #include "components/url_formatter/url_fixer.h"
-#include "components/url_formatter/url_formatter.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "url/gurl.h"
 
@@ -304,41 +302,6 @@ void TemplateURLService::RegisterProfilePrefs(
   registry->RegisterBooleanPref(prefs::kDefaultSearchProviderEnabled, true);
 }
 
-// static
-base::string16 TemplateURLService::CleanUserInputKeyword(
-    const base::string16& keyword) {
-  // Remove the scheme.
-  base::string16 result(base::i18n::ToLower(keyword));
-  base::TrimWhitespace(result, base::TRIM_ALL, &result);
-  url::Component scheme_component;
-  if (url::ExtractScheme(base::UTF16ToUTF8(keyword).c_str(),
-                         static_cast<int>(keyword.length()),
-                         &scheme_component)) {
-    // If the scheme isn't "http" or "https", bail.  The user isn't trying to
-    // type a web address, but rather an FTP, file:, or other scheme URL, or a
-    // search query with some sort of initial operator (e.g. "site:").
-    if (result.compare(0, scheme_component.end(),
-                       base::ASCIIToUTF16(url::kHttpScheme)) &&
-        result.compare(0, scheme_component.end(),
-                       base::ASCIIToUTF16(url::kHttpsScheme)))
-      return base::string16();
-
-    // Include trailing ':'.
-    result.erase(0, scheme_component.end() + 1);
-    // Many schemes usually have "//" after them, so strip it too.
-    const base::string16 after_scheme(base::ASCIIToUTF16("//"));
-    if (result.compare(0, after_scheme.length(), after_scheme) == 0)
-      result.erase(0, after_scheme.length());
-  }
-
-  // Remove leading "www.".
-  result = url_formatter::StripWWW(result);
-
-  // Remove trailing "/".
-  return (result.empty() || result.back() != '/') ?
-      result : result.substr(0, result.length() - 1);
-}
-
 bool TemplateURLService::CanAddAutogeneratedKeyword(
     const base::string16& keyword,
     const GURL& url,

components/search_engines/template_url_service.h

@@ -118,10 +118,6 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Register Profile preferences in |registry|.
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
 
-  // Removes any unnecessary characters from a user input keyword.
-  // This removes the leading scheme, "www." and any trailing slash.
-  static base::string16 CleanUserInputKeyword(const base::string16& keyword);
-
   // Returns true if there is no TemplateURL that conflicts with the
   // keyword/url pair, or there is one but it can be replaced. If there is an
   // existing keyword that can be replaced and template_url_to_replace is