Omnibox: Don't Promote Intranet Hostnames Pound Sign Space Whatever ("c# foo")

Suppose you have an intranet hostname "c" that you've previously visited.

After this change, the behavior will be as follows:
input  ->  inline suggestion (what happens what you hit return)
c -> navigate to URL
c/ -> navigate to URL
c#foo -> search (this behavior differs from before this change)
c# foo -> search (this behavior differs from before this change)
c#  foo -> search (this behavior differs from before this change)
c# -> search (this behavior differs from before this change)
c#  -> search (this behavior differs from before this change) (notice the extra spaces the URL parser drops them)
c/#foo -> navigate to URL

BUG=169449

Review URL: https://chromiumcodereview.appspot.com/13190020

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@192615 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/autocomplete/history_url_provider.cc

@@ -741,16 +741,42 @@ bool HistoryURLProvider::FixupExactSuggestion(
       break;
   }
 
+  const GURL& url = match->destination_url;
+  const url_parse::Parsed& parsed = url.parsed_for_possibly_invalid_spec();
+  // If the what-you-typed result looks like a single word (which can be
+  // interpreted as an intranet address) followed by a pound sign ("#"),
+  // leave the score for the url-what-you-typed result as is.  It will be
+  // outscored by a search query from the SearchProvider. This test fixes
+  // cases such as "c#" and "c# foo" where the user has visited an intranet
+  // site "c".  We want the search-what-you-typed score to beat the
+  // URL-what-you-typed score in this case.  Most of the below test tries to
+  // make sure that this code does not trigger if the user did anything to
+  // indicate the desired match is a URL.  For instance, "c/# foo" will not
+  // pass the test because that will be classified as input type URL.  The
+  // parsed.CountCharactersBefore() in the test looks for the presence of a
+  // reference fragment in the URL by checking whether the position differs
+  // included the delimiter (pound sign) versus not including the delimiter.
+  // (One cannot simply check url.ref() because it will not distinguish
+  // between the input "c" and the input "c#", both of which will have empty
+  // reference fragments.)
+  if ((type == UNVISITED_INTRANET) &&
+      (input.type() != AutocompleteInput::URL) &&
+      url.username().empty() && url.password().empty() && url.port().empty() &&
+      (url.path() == "/") && url.query().empty() &&
+      (parsed.CountCharactersBefore(url_parse::Parsed::REF, true) !=
+       parsed.CountCharactersBefore(url_parse::Parsed::REF, false))) {
+    return false;
+  }
+
   match->relevance = CalculateRelevance(type, 0);
 
-  if (type == UNVISITED_INTRANET && !matches->empty()) {
-    // If there are any other matches, then don't promote this match here, in
-    // hopes the caller will be able to inline autocomplete a better suggestion.
-    // DoAutocomplete() will fall back on this match if inline autocompletion
-    // fails.  This matches how we react to never-visited URL inputs in the non-
-    // intranet case.
+  // If there are any other matches, then don't promote this match here, in
+  // hopes the caller will be able to inline autocomplete a better suggestion.
+  // DoAutocomplete() will fall back on this match if inline autocompletion
+  // fails.  This matches how we react to never-visited URL inputs in the non-
+  // intranet case.
+  if (type == UNVISITED_INTRANET && !matches->empty())
     return false;
-  }
 
   // Put it on the front of the HistoryMatches for redirect culling.
   CreateOrPromoteMatch(classifier.url_row(), string16::npos, false, matches,

chrome/browser/autocomplete/history_url_provider_unittest.cc

@@ -165,12 +165,25 @@ class HistoryURLProviderTest : public testing::Test,
   void FillData();
 
   // Runs an autocomplete query on |text| and checks to see that the returned
-  // results' destination URLs match those provided.
+  // results' destination URLs match those provided.  Also allows checking
+  // that the input type was identified correctly.
   void RunTest(const string16 text,
                const string16& desired_tld,
                bool prevent_inline_autocomplete,
                const std::string* expected_urls,
-               size_t num_results);
+               size_t num_results,
+               AutocompleteInput::Type* identified_input_type);
+
+  // A version of the above without the final |type| output parameter.
+  void RunTest(const string16 text,
+               const string16& desired_tld,
+               bool prevent_inline_autocomplete,
+               const std::string* expected_urls,
+               size_t num_results) {
+    AutocompleteInput::Type type;
+    return RunTest(text, desired_tld, prevent_inline_autocomplete,
+                   expected_urls, num_results, &type);
+  }
 
   void RunAdjustOffsetTest(const string16 text, size_t expected_offset);
 
@@ -240,14 +253,17 @@ void HistoryURLProviderTest::FillData() {
       false, history::SOURCE_BROWSED);
 }
 
-void HistoryURLProviderTest::RunTest(const string16 text,
-                                     const string16& desired_tld,
-                                     bool prevent_inline_autocomplete,
-                                     const std::string* expected_urls,
-                                     size_t num_results) {
+void HistoryURLProviderTest::RunTest(
+    const string16 text,
+    const string16& desired_tld,
+    bool prevent_inline_autocomplete,
+    const std::string* expected_urls,
+    size_t num_results,
+    AutocompleteInput::Type* identified_input_type) {
   AutocompleteInput input(text, string16::npos, desired_tld, GURL(),
                           prevent_inline_autocomplete, false, true,
                           AutocompleteInput::ALL_MATCHES);
+  *identified_input_type = input.type();
   autocomplete_->Start(input, false);
   if (!autocomplete_->done())
     MessageLoop::current()->Run();
@@ -610,6 +626,44 @@ TEST_F(HistoryURLProviderTest, IntranetURLsWithPaths) {
   }
 }
 
+TEST_F(HistoryURLProviderTest, IntranetURLsWithRefs) {
+  struct TestCase {
+    const char* input;
+    int relevance;
+    AutocompleteInput::Type type;
+  } test_cases[] = {
+    { "gooey", 1410, AutocompleteInput::UNKNOWN },
+    { "gooey/", 1410, AutocompleteInput::URL },
+    { "gooey#", 1200, AutocompleteInput::UNKNOWN },
+    { "gooey/#", 1200, AutocompleteInput::URL },
+    { "gooey#foo", 1200, AutocompleteInput::UNKNOWN },
+    { "gooey/#foo", 1200, AutocompleteInput::URL },
+    { "gooey# foo", 1200, AutocompleteInput::UNKNOWN },
+    { "gooey/# foo", 1200, AutocompleteInput::URL },
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    SCOPED_TRACE(test_cases[i].input);
+    const std::string output[] = {
+      URLFixerUpper::FixupURL(test_cases[i].input, std::string()).spec()
+    };
+    AutocompleteInput::Type type;
+    ASSERT_NO_FATAL_FAILURE(
+        RunTest(ASCIIToUTF16(test_cases[i].input),
+                string16(), false, output, arraysize(output), &type));
+    // Actual relevance should be at least what test_cases expects and
+    // and no more than 10 more.
+    EXPECT_LE(test_cases[i].relevance, matches_[0].relevance);
+    EXPECT_LT(matches_[0].relevance, test_cases[i].relevance + 10);
+    // Input type should be what we expect.  This is important because
+    // this provider counts on SearchProvider to give queries a relevance
+    // score >1200 for UNKNOWN inputs and <1200 for URL inputs.  (That's
+    // already tested in search_provider_unittest.cc.)  For this test
+    // here to test that the user sees the correct behavior, it needs
+    // to check that the input type was identified correctly.
+    EXPECT_EQ(test_cases[i].type, type);
+  }
+}
+
 // Makes sure autocompletion happens for intranet sites that have been
 // previoulsy visited.
 TEST_F(HistoryURLProviderTest, IntranetURLCompletion) {