Omnibox: Make Bookmarks Set Inline_Autocompletion

chrome/browser/autocomplete/bookmark_provider.cc

Index: chrome/browser/autocomplete/bookmark_provider.cc
diff --git a/chrome/browser/autocomplete/bookmark_provider.cc b/chrome/browser/autocomplete/bookmark_provider.cc
index 73d17370919e2185c555b1a12dc9a25a1baf6e71..b2983a3844043f1dddb29b12c16ff89fba90db1e 100644
--- a/chrome/browser/autocomplete/bookmark_provider.cc
+++ b/chrome/browser/autocomplete/bookmark_provider.cc
@@ -9,7 +9,10 @@
 #include <vector>
 
 #include "base/prefs/pref_service.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/autocomplete/autocomplete_result.h"
+#include "chrome/browser/autocomplete/history_provider.h"
+#include "chrome/browser/autocomplete/url_prefix.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/bookmark_title_match.h"
@@ -39,14 +42,9 @@ void BookmarkProvider::Start(const AutocompleteInput& input,
     return;
   matches_.clear();
 
-  // Short-circuit any matching when inline autocompletion is disabled and
-  // we're looking for BEST_MATCH because none of the BookmarkProvider's
-  // matches can score high enough to qualify.
   if (input.text().empty() ||
       ((input.type() != AutocompleteInput::UNKNOWN) &&
-       (input.type() != AutocompleteInput::QUERY)) ||
-      ((input.matches_requested() == AutocompleteInput::BEST_MATCH) &&

[Peter Kasting, 2014/04/10 00:22:35]

Man.  Are there legitimate uses of BEST_MATCH left?  It seems like you're
gradually removing all of them.  Maybe we can simplify something.

[Mark P, 2014/04/10 17:10:02]

Interesting question.

The only legitimate uses I see are in the AutocompleteClassifier and here in
keyword provider:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/aut...
, where it's merely for efficiency when dealing with extension providers.

Think we can get rid of BEST_MATCHES (and replace it with SYNCHRONOUS_MATCHES)? 
I think so.  We could even change the MatchesRequested enum and replace it with
a simple bool (want_asychronous_matches).

I think keeping the distinction between SYNCHRONOUS_MATCHES and ALL_MATCHES is
important.  The AutocompleteClassifier gets called all over the place, many
times, and I don't want it to hammer the history system or the suggest server
harder because it keeps sending history DB on-disk requests or suggest server
requests.

[Peter Kasting, 2014/04/14 22:56:13]

I believe if we tried to remove this distinction entirely, we'd get infinite
recursion -- I think at least one of the autocomplete providers uses the
AutocompleteClassifier, and that call stack would blow up.  So yeah, I'm sure we
still have to keep some distinction around, but maybe we could simplify it a
bit.

[Mark P, 2014/04/14 23:28:15]

I gave you the two possible ways to simplify it.  What do you think of them?  Or
we can leave it as-is.

-       input.prevent_inline_autocomplete()))
+       (input.type() != AutocompleteInput::QUERY)))
     return;
 
   DoAutocomplete(input,
@@ -94,11 +92,13 @@ void BookmarkProvider::DoAutocomplete(const AutocompleteInput& input,
                                                   &matches);
   if (matches.empty())
     return;  // There were no matches.
+  AutocompleteInput fixed_up_input(input);
+  FixupUserInput(&fixed_up_input);
   for (TitleMatches::const_iterator i = matches.begin(); i != matches.end();
        ++i) {
     // Create and score the AutocompleteMatch. If its score is 0 then the
     // match is discarded.
-    AutocompleteMatch match(TitleMatchToACMatch(*i));
+    AutocompleteMatch match(TitleMatchToACMatch(input, fixed_up_input, *i));
     if (match.relevance > 0)
       matches_.push_back(match);
   }
@@ -153,6 +153,8 @@ class ScoringFunctor {
 }  // namespace
 
 AutocompleteMatch BookmarkProvider::TitleMatchToACMatch(
+    const AutocompleteInput& input,
+    const AutocompleteInput& fixed_up_input,
     const BookmarkTitleMatch& title_match) {
   // The AutocompleteMatch we construct is non-deletable because the only
   // way to support this would be to delete the underlying bookmark, which is
@@ -161,16 +163,35 @@ AutocompleteMatch BookmarkProvider::TitleMatchToACMatch(
                           AutocompleteMatchType::BOOKMARK_TITLE);
   const base::string16& title(title_match.node->GetTitle());
   DCHECK(!title.empty());
+
   const GURL& url(title_match.node->url());
+  const base::string16& url_utf16 = base::UTF8ToUTF16(url.spec());
+  size_t match_start, inline_autocomplete_offset;
+  URLPrefix::ComputeMatchStartAndInlineAutocompleteOffset(input, fixed_up_input,
+      false, url_utf16, &match_start, &inline_autocomplete_offset);

[Peter Kasting, 2014/04/10 00:22:35]

Nit: All lines of args should begin at the same position

[Mark P, 2014/04/10 17:10:02]

Done.

   match.destination_url = url;
+  const bool trim_http = !AutocompleteInput::HasHTTPScheme(input.text()) &&
+      ((match_start == base::string16::npos) || (match_start != 0));
   match.contents = net::FormatUrl(url, languages_,
-      net::kFormatUrlOmitAll & net::kFormatUrlOmitHTTP,
-      net::UnescapeRule::SPACES, NULL, NULL, NULL);
+      net::kFormatUrlOmitAll & ~(trim_http ? 0 : net::kFormatUrlOmitHTTP),
+      net::UnescapeRule::SPACES, NULL, NULL, &inline_autocomplete_offset);
   match.contents_class.push_back(
       ACMatchClassification(0, ACMatchClassification::URL));
   match.fill_into_edit =
       AutocompleteInput::FormattedStringWithEquivalentMeaning(url,
                                                               match.contents);
+  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.allowed_to_be_default_match = match.inline_autocompletion.empty() ||
+        !HistoryProvider::PreventInlineAutocomplete(input);
+  }
   match.description = title;
   match.description_class =
       ClassificationsFromMatch(title_match.match_positions,