Omnibox: Make Bookmarks Set Inline_Autocompletion

chrome/browser/autocomplete/bookmark_provider.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/autocomplete/bookmark_provider.h"
 
 #include <algorithm>
 #include <functional>
 #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/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"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "net/base/net_util.h"
 
 typedef std::vector<BookmarkTitleMatch> TitleMatches;
 
 // BookmarkProvider ------------------------------------------------------------
 
 BookmarkProvider::BookmarkProvider(
     AutocompleteProviderListener* listener,
     Profile* profile)
     : AutocompleteProvider(listener, profile,
                            AutocompleteProvider::TYPE_BOOKMARK),
       bookmark_model_(NULL) {
   if (profile) {
     bookmark_model_ = BookmarkModelFactory::GetForProfile(profile);
     languages_ = profile_->GetPrefs()->GetString(prefs::kAcceptLanguages);
   }
 }
 
 void BookmarkProvider::Start(const AutocompleteInput& input,
                              bool minimal_changes) {
   if (minimal_changes)
     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) &&
-       input.prevent_inline_autocomplete()))
+       (input.type() != AutocompleteInput::QUERY)))
     return;
 
   DoAutocomplete(input,
                  input.matches_requested() == AutocompleteInput::BEST_MATCH);
 }
 
 BookmarkProvider::~BookmarkProvider() {}
 
 void BookmarkProvider::DoAutocomplete(const AutocompleteInput& input,
                                       bool best_match) {
@@ skipping 27 matching lines @@
   // for all matched terms.
   //
   // Please refer to the code for BookmarkIndex::GetBookmarksWithTitlesMatching
   // for complete details of how title searches are performed against the user's
   // bookmarks.
   bookmark_model_->GetBookmarksWithTitlesMatching(input.text(),
                                                   kMaxBookmarkMatches,
                                                   &matches);
   if (matches.empty())
     return;  // There were no matches.
+  AutocompleteInput fixed_up_input;
+  // If the input fails to get fixed up, pretend the fixed up version is the
+  // original.
+  if (!FixupUserInput(&fixed_up_input))
+    fixed_up_input = 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);
   }
 
   // Sort and clip the resulting matches.
   size_t max_matches = best_match ? 1 : AutocompleteProvider::kMaxMatches;
   if (matches_.size() > max_matches) {
     std::partial_sort(matches_.begin(),
                       matches_.begin() + max_matches,
                       matches_.end(),
@@ skipping 34 matching lines @@
   double ScoringFactor() { return scoring_factor_; }
 
  private:
   double title_length_;
   double scoring_factor_;
 };
 
 }  // 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
   // unlikely to be what the user intends.
   AutocompleteMatch match(this, 0, false,
                           AutocompleteMatchType::BOOKMARK_TITLE);
   const base::string16& title(title_match.node->GetTitle());
   DCHECK(!title.empty());
+
+  // Determine |inline_autocomplete_offset|.

[Peter Kasting, 2014/04/09 00:53:32]

This whole block looks suspiciously similar to what other providers do (or at
least, need to do).  Is there any opportunity to hoist similar code to a common
location?

[Mark P, 2014/04/09 02:44:42]

I thought about this as I was transferred code here.  The regular providers
don't need to do the fixup case, so their logic is simply one BestURLPrefix call
and the variable declarations around it.  No sharing there.

Shortcuts does the fixup case.  But shortcuts doesn't use BestURLPrefix, it uses
something different BestURLPrefixWithWWWCase because the strings it's trying
match against may be missing the scheme but have a www that we want to drop.  I
guess I could share something with Shortcuts, but that code would be a bit odd
because we'd have to take an extra parameter want_www_case, and condition both
its best-url calls on that.  As those calls are really the only thing happening
here, I don't think it's worthwhile to share.  It'd be ugly.

The trim_http line is in common with history_url provider.  That's one line.  I
can replace it with a call that takes those three variables.  I don't think that
will help readability.  (Those three variables don't naturally go together; I
can't think a nice name.  TrimHttp (input, best_prefix,
inline_autocomplete_offset) doesn't look good to me.)

[Mark P, 2014/04/09 20:21:41]

I found a bug this code, causing me to change my mind.  I'm going to take your
advice and put it in a common place so I don't have to remember to make the same
fixes in multiple places.

   const GURL& url(title_match.node->url());
+  const base::string16& url_utf16 = base::UTF8ToUTF16(url.spec());
+  const URLPrefix* best_prefix = URLPrefix::BestURLPrefix(url_utf16,
+                                                          input.text());
+  base::string16 matching_string = input.text();
+  // If we failed to find a best_prefix initially, try again using a fixed-up
+  // version of the user input.  This is especially useful to get about: URLs
+  // to inline against chrome:// URLs.  (about: URLs are fixed up to the
+  // chrome:// scheme.)
+  if ((best_prefix == NULL) && !fixed_up_input.text().empty() &&
+      (fixed_up_input.text() != input.text())) {
+    best_prefix = URLPrefix::BestURLPrefix(url_utf16, fixed_up_input.text());
+    matching_string = fixed_up_input.text();
+  }
+  size_t inline_autocomplete_offset =
+      (best_prefix != NULL) ?
+      (best_prefix->prefix.length() + matching_string.length()) :
+      base::string16::npos;
+
   match.destination_url = url;
+  const bool trim_http = !AutocompleteInput::HasHTTPScheme(input.text()) &&
+      ((best_prefix != NULL) || (inline_autocomplete_offset != 0));

[Peter Kasting, 2014/04/09 00:53:32]

I don't think I understand this conditional.

Wouldn't this trim if the user text was "http" (since we'd get a non-null best
prefix of "")?  Also, if best_prefix is NULL, isn't inline_autocomplete_offset
== base::string16::npos, and thus nonzero, so the second line here always
evaluates to true?

[Mark P, 2014/04/09 02:44:42]

Oops, you're right.  It should be best_prefix == NULL, not best_prefix != NULL.
Done.

   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() ||
+        !input.prevent_inline_autocomplete();
+  }
   match.description = title;
   match.description_class =
       ClassificationsFromMatch(title_match.match_positions,
                                match.description.size());
   match.starred = true;
 
   // Summary on how a relevance score is determined for the match:
   //
   // For each term matching within the bookmark's title (as given by the set of
   // Snippet::MatchPositions) calculate a 'factor', sum up those factors, then
@@ skipping 81 matching lines @@
   for (Snippet::MatchPositions::const_iterator i = positions.begin();
        i != positions.end(); ++i) {
     AutocompleteMatch::ACMatchClassifications new_class;
     AutocompleteMatch::ClassifyLocationInString(i->first, i->second - i->first,
         text_length, 0, &new_class);
     classifications = AutocompleteMatch::MergeClassifications(
         classifications, new_class);
   }
   return classifications;
 }