[omnibox] Fix input offset after URL formatting

If the user enters a URL with a slash e.g. 'url/', the formatter,
having no context, will reduce it to 'url' and assume that the cursor
is before the slash '/'. This CL clears the bit requesting such ellision,
preserving the slash.

Bug: 809858
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I241fb58a6ff4223818884adf61f07e801e86e851
Reviewed-on: https://chromium-review.googlesource.com/934640Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541807}

chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc

@@ -47,7 +47,8 @@ base::string16 ChromeToolbarModelDelegate::FormattedStringWithEquivalentMeaning(
     const GURL& url,
     const base::string16& formatted_url) const {
   return AutocompleteInput::FormattedStringWithEquivalentMeaning(
-      url, formatted_url, ChromeAutocompleteSchemeClassifier(GetProfile()));
+      url, formatted_url, ChromeAutocompleteSchemeClassifier(GetProfile()),
+      nullptr);
 }
 
 bool ChromeToolbarModelDelegate::GetURL(GURL* url) const {

components/omnibox/browser/autocomplete_input.cc

@@ -523,17 +523,21 @@ void AutocompleteInput::ParseForEmphasizeComponents(
 base::string16 AutocompleteInput::FormattedStringWithEquivalentMeaning(
     const GURL& url,
     const base::string16& formatted_url,
-    const AutocompleteSchemeClassifier& scheme_classifier) {
+    const AutocompleteSchemeClassifier& scheme_classifier,
+    size_t* offset) {
   if (!url_formatter::CanStripTrailingSlash(url))
     return formatted_url;
   const base::string16 url_with_path(formatted_url + base::char16('/'));
-  return (AutocompleteInput::Parse(
-              formatted_url, std::string(), scheme_classifier, nullptr, nullptr,
-              nullptr) == AutocompleteInput::Parse(url_with_path, std::string(),
-                                                   scheme_classifier, nullptr,
-                                                   nullptr, nullptr))
-             ? formatted_url
-             : url_with_path;
+  if (AutocompleteInput::Parse(formatted_url, std::string(), scheme_classifier,
+                               nullptr, nullptr, nullptr) ==
+      AutocompleteInput::Parse(url_with_path, std::string(), scheme_classifier,
+                               nullptr, nullptr, nullptr)) {
+    return formatted_url;
+  }
+  // If offset is past the addition, shift it.
+  if (offset && *offset == formatted_url.size())
+    ++(*offset);
+  return url_with_path;
 }
 
 // static

components/omnibox/browser/autocomplete_input.h

@@ -93,11 +93,13 @@ class AutocompleteInput {
   // formatted string with the same meaning as the original URL (i.e. it will
   // re-append a slash if necessary).  Because this uses Parse() under the hood
   // to determine the meaning of the different strings, callers need to supply a
-  // |scheme_classifier| to pass to Parse().
+  // |scheme_classifier| to pass to Parse(). If |offset| is non-null, it will
+  // be updated with any changes that shift it.
   static base::string16 FormattedStringWithEquivalentMeaning(
       const GURL& url,
       const base::string16& formatted_url,
-      const AutocompleteSchemeClassifier& scheme_classifier);
+      const AutocompleteSchemeClassifier& scheme_classifier,
+      size_t* offset);
 
   // Returns the number of non-empty components in |parts| besides the host.
   static int NumNonHostComponents(const url::Parsed& parts);

components/omnibox/browser/history_quick_provider.cc

@@ -229,18 +229,12 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
           url_formatter::FormatUrl(info.url(), fill_into_edit_format_types,
                                    net::UnescapeRule::SPACES, nullptr, nullptr,
                                    &inline_autocomplete_offset),
-          client()->GetSchemeClassifier());
+          client()->GetSchemeClassifier(), &inline_autocomplete_offset);
 
   // Set |inline_autocompletion| and |allowed_to_be_default_match| if possible.
   if (inline_autocomplete_offset != base::string16::npos) {
-    // |inline_autocomplete_offset| may be beyond the end of the
-    // |match.fill_into_edit| if the user has typed an URL with a scheme and the
-    // last character typed is a slash.  That slash is removed by the
-    // FormatUrlWithOffsets call above.
-    if (inline_autocomplete_offset < match.fill_into_edit.length()) {
-      match.inline_autocompletion =
-          match.fill_into_edit.substr(inline_autocomplete_offset);
-    }
+    match.inline_autocompletion =
+        match.fill_into_edit.substr(inline_autocomplete_offset);
     match.allowed_to_be_default_match = match.inline_autocompletion.empty() ||
         !PreventInlineAutocomplete(autocomplete_input_);
   }

components/omnibox/browser/history_quick_provider.h

@@ -53,6 +53,8 @@ class HistoryQuickProvider : public HistoryProvider {
   FRIEND_TEST_ALL_PREFIXES(HistoryQuickProviderTest,
                            DontTrimHttpsSchemeIfInputHasScheme);
   FRIEND_TEST_ALL_PREFIXES(HistoryQuickProviderTest, DoTrimHttpsScheme);
+  FRIEND_TEST_ALL_PREFIXES(HistoryQuickProviderTest,
+                           CorrectAutocompleteWithTrailingSlash);
 
   ~HistoryQuickProvider() override;
 

components/omnibox/browser/history_quick_provider_unittest.cc

@@ -823,6 +823,22 @@ TEST_F(HistoryQuickProviderTest, DoTrimHttpsScheme) {
   EXPECT_EQ(ASCIIToUTF16("facebook.com"), match.contents);
 }
 
+TEST_F(HistoryQuickProviderTest, CorrectAutocompleteWithTrailingSlash) {
+  provider().autocomplete_input_ = AutocompleteInput(
+      base::ASCIIToUTF16("cr/"), metrics::OmniboxEventProto::OTHER,
+      TestSchemeClassifier());
+  RowWordStarts word_starts;
+  word_starts.url_word_starts_ = {0};
+  ScoredHistoryMatch sh_match(history::URLRow(GURL("http://cr/")),
+                              VisitInfoVector(), ASCIIToUTF16("cr/"),
+                              {ASCIIToUTF16("cr")}, {0}, word_starts, false, 0,
+                              base::Time());
+  AutocompleteMatch ac_match(provider().QuickMatchToACMatch(sh_match, 0));
+  EXPECT_EQ(base::ASCIIToUTF16("cr/"), ac_match.fill_into_edit);
+  EXPECT_EQ(base::ASCIIToUTF16(""), ac_match.inline_autocompletion);
+  EXPECT_TRUE(ac_match.allowed_to_be_default_match);
+}
+
 // HQPOrderingTest -------------------------------------------------------------
 
 class HQPOrderingTest : public HistoryQuickProviderTest {

components/omnibox/browser/history_url_provider.cc

@@ -624,7 +624,8 @@ AutocompleteMatch HistoryURLProvider::SuggestExactInput(
     const size_t offset = trim_http ? TrimHttpPrefix(&display_string) : 0;
     match.fill_into_edit =
         AutocompleteInput::FormattedStringWithEquivalentMeaning(
-            destination_url, display_string, client()->GetSchemeClassifier());
+            destination_url, display_string, client()->GetSchemeClassifier(),
+            nullptr);
     // The what-you-typed match is generally only allowed to be default for
     // URL inputs or when there is no default search provider.  (It's also
     // allowed to be default for UNKNOWN inputs where the destination is a known
@@ -1240,7 +1241,7 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
           url_formatter::FormatUrl(info.url(), fill_into_edit_format_types,
                                    net::UnescapeRule::SPACES, nullptr, nullptr,
                                    &inline_autocomplete_offset),
-          client()->GetSchemeClassifier());
+          client()->GetSchemeClassifier(), &inline_autocomplete_offset);
   // |inline_autocomplete_offset| was guaranteed not to be npos before the call
   // to FormatUrl().  If it is npos now, that means the represented location no
   // longer exists as such in the formatted string, e.g. if the offset pointed

components/omnibox/browser/physical_web_provider.cc

@@ -248,7 +248,8 @@ void PhysicalWebProvider::ConstructZeroSuggestMatches(
 
     match.fill_into_edit =
         AutocompleteInput::FormattedStringWithEquivalentMeaning(
-            url, url_formatter::FormatUrl(url), client_->GetSchemeClassifier());
+            url, url_formatter::FormatUrl(url), client_->GetSchemeClassifier(),
+            nullptr);
 
     match.description = title;
     match.description_class.push_back(
@@ -318,7 +319,7 @@ void PhysicalWebProvider::AppendOverflowItem(int additional_url_count,
 
   match.fill_into_edit =
       AutocompleteInput::FormattedStringWithEquivalentMeaning(
-          url, match.contents, client_->GetSchemeClassifier());
+          url, match.contents, client_->GetSchemeClassifier(), nullptr);
 
   match.description =
       l10n_util::GetStringUTF16(IDS_PHYSICAL_WEB_OVERFLOW_DESCRIPTION);

components/omnibox/browser/search_provider.cc

@@ -1473,7 +1473,7 @@ AutocompleteMatch SearchProvider::NavigationToMatch(
           url_formatter::FormatUrl(navigation.url(), format_types,
                                    net::UnescapeRule::SPACES, nullptr, nullptr,
                                    &inline_autocomplete_offset),
-          client()->GetSchemeClassifier());
+          client()->GetSchemeClassifier(), &inline_autocomplete_offset);
   if (inline_autocomplete_offset != base::string16::npos) {
     DCHECK(inline_autocomplete_offset <= match.fill_into_edit.length());
     match.inline_autocompletion =

components/omnibox/browser/search_suggestion_parser.cc

@@ -255,7 +255,8 @@ SearchSuggestionParser::NavigationResult::NavigationResult(
                                    nullptr,
                                    nullptr,
                                    nullptr),
-          scheme_classifier)),
+          scheme_classifier,
+          nullptr)),
       description_(description) {
   DCHECK(url_.is_valid());
   CalculateAndClassifyMatchContents(true, input_text);

components/omnibox/browser/titled_url_match_utils.cc

@@ -105,16 +105,10 @@ AutocompleteMatch TitledUrlMatchToAutocompleteMatch(
           url_formatter::FormatUrl(url, fill_into_edit_format_types,
                                    net::UnescapeRule::SPACES, nullptr, nullptr,
                                    &inline_autocomplete_offset),
-          scheme_classifier);
+          scheme_classifier, &inline_autocomplete_offset);
   if (inline_autocomplete_offset != base::string16::npos) {
-    // |inline_autocomplete_offset| may be beyond the end of the
-    // |fill_into_edit| if the user has typed an URL with a scheme and the
-    // last character typed is a slash.  That slash is removed by the
-    // FormatURLWithOffsets call above.
-    if (inline_autocomplete_offset < match.fill_into_edit.length()) {
-      match.inline_autocompletion =
-          match.fill_into_edit.substr(inline_autocomplete_offset);
-    }
+    match.inline_autocompletion =
+        match.fill_into_edit.substr(inline_autocomplete_offset);
     match.allowed_to_be_default_match =
         match.inline_autocompletion.empty() ||
         !HistoryProvider::PreventInlineAutocomplete(input);

components/omnibox/browser/zero_suggest_provider.cc

@@ -375,7 +375,7 @@ AutocompleteMatch ZeroSuggestProvider::NavigationToMatch(
   match.fill_into_edit +=
       AutocompleteInput::FormattedStringWithEquivalentMeaning(
           navigation.url(), url_formatter::FormatUrl(navigation.url()),
-          client()->GetSchemeClassifier());
+          client()->GetSchemeClassifier(), nullptr);
 
   AutocompleteMatch::ClassifyLocationInString(base::string16::npos, 0,
       match.contents.length(), ACMatchClassification::URL,

ios/chrome/browser/ui/toolbar/toolbar_model_delegate_ios.mm

@@ -44,7 +44,7 @@ base::string16 ToolbarModelDelegateIOS::FormattedStringWithEquivalentMeaning(
     const GURL& url,
     const base::string16& formatted_url) const {
   return AutocompleteInput::FormattedStringWithEquivalentMeaning(
-      url, formatted_url, AutocompleteSchemeClassifierImpl());
+      url, formatted_url, AutocompleteSchemeClassifierImpl(), nullptr);
 }
 
 bool ToolbarModelDelegateIOS::GetURL(GURL* url) const {