Stop exposing manually_selected_match_ outside the AutocompletePopupModel. The main goal of this is to be a first step towards divorcing the popup and the edit from each other.

To do this, I changed the behavior of manual selections.  They now do not persist once the user types more characters, hits esc, etc.  Our old behavior, which Brett and I designed long ago, turns out to have been a mistake; users who arrowed to an item and then typed more weren't expecting "stickiness" on their previous choice, and it led to user mistakes.

This also required changing how we do the "keyword UI" persistence in the case where the user switches into keyword UI, but then deletes all his text.  Previously, we used manually_selected_match_ with a provider affinity to the keyword provider in order to accomplish this.  Now we stick another flag on the AutocompleteInput, which, when set, biases the keyword provider to return the best results.  The user-visible effect of this is that when in keyword UI mode with no query string, the selected entry in the popup will be the first, rather than third, entry.  This is a small win.

While here I fixed the bug where editing a string and transforming it into a keyword search would avoid switching into keyword UI (as expected), but also delete the keyword off the visible string (oops).  I also made us lock the popup once the user changes the manually_selected_match_, in order to give a little more stability to it.

I'm sorry this makes so many behavioral changes at once.  All this code is tangled together and untangling it is hard :(

The keyword-related variables in the AutocompleteEditModel seem a mess.  They are probably worse now than before this change; I think I need a followup change at some point to make them all more sane.  It seems like we have three variables and complex conditionals where two, and simpler ones, would do.

BUG=997976,1201974,1204173
Review URL: http://codereview.chromium.org/3172

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

1 files changed, 19 insertions, 50 deletions

diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index 27a3e7b..e6ab849 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -31,9 +31,11 @@
 
 AutocompleteInput::AutocompleteInput(const std::wstring& text,
                                      const std::wstring& desired_tld,
-                                     bool prevent_inline_autocomplete)
+                                     bool prevent_inline_autocomplete,
+                                     bool prefer_keyword)
     : desired_tld_(desired_tld),
-      prevent_inline_autocomplete_(prevent_inline_autocomplete) {
+      prevent_inline_autocomplete_(prevent_inline_autocomplete),
+      prefer_keyword_(prefer_keyword) {
   // Trim whitespace from edges of input; don't inline autocomplete if there
   // was trailing whitespace.
   if (TrimWhitespace(text, TRIM_ALL, &text_) & TRIM_TRAILING)
@@ -213,7 +215,8 @@ bool AutocompleteInput::Equals(const AutocompleteInput& other) const {
          (type_ == other.type_) &&
          (desired_tld_ == other.desired_tld_) &&
          (scheme_ == other.scheme_) &&
-         (prevent_inline_autocomplete_ == other.prevent_inline_autocomplete_);
+         (prevent_inline_autocomplete_ == other.prevent_inline_autocomplete_) &&
+         (prefer_keyword_ == other.prefer_keyword_);
 }
 
 void AutocompleteInput::Clear() {
@@ -223,6 +226,7 @@ void AutocompleteInput::Clear() {
   scheme_.clear();
   desired_tld_.clear();
   prevent_inline_autocomplete_ = false;
+  prefer_keyword_ = false;
 }
 
 // AutocompleteMatch ----------------------------------------------------------
@@ -436,19 +440,23 @@ void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) {
 }
 
 void AutocompleteResult::AppendMatches(const ACMatches& matches) {
-  default_match_ = end();
   std::copy(matches.begin(), matches.end(), std::back_inserter(matches_));
+  default_match_ = end();
 }
 
 void AutocompleteResult::AddMatch(const AutocompleteMatch& match) {
-  default_match_ = end();
-  matches_.insert(std::upper_bound(matches_.begin(), matches_.end(), match,
-                                   &AutocompleteMatch::MoreRelevant), match);
+  DCHECK(default_match_ != end());
+  ACMatches::iterator insertion_point =
+      std::upper_bound(begin(), end(), match, &AutocompleteMatch::MoreRelevant);
+  ACMatches::iterator::difference_type default_offset =
+      default_match_ - begin();
+  if ((insertion_point - begin()) <= default_offset)
+    ++default_offset;
+  matches_.insert(insertion_point, match);
+  default_match_ = begin() + default_offset;
 }
 
 void AutocompleteResult::SortAndCull() {
-  default_match_ = end();
-
   // Remove duplicates.
   std::sort(matches_.begin(), matches_.end(),
             &AutocompleteMatch::DestinationSortFunc);
@@ -475,45 +483,7 @@ void AutocompleteResult::SortAndCull() {
 
   // Now put the final result set in order.
   std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::MoreRelevant);
-}
-
-bool AutocompleteResult::SetDefaultMatch(const Selection& selection) {
-  default_match_ = end();
-
-  // Look for the best match.
-  for (const_iterator i(begin()); i != end(); ++i) {
-    // If we have an exact match, return immediately.
-    if (selection.is_history_what_you_typed_match ?
-        i->is_history_what_you_typed_match :
-        (!selection.destination_url.empty() &&
-         (i->destination_url == selection.destination_url))) {
-      default_match_ = i;
-      return true;
-    }
-
-    // Otherwise, see if this match is closer to the desired selection than the
-    // existing one.
-    if (default_match_ == end()) {
-      // No match at all yet, pick the first one we see.
-      default_match_ = i;
-    } else if (selection.provider_affinity == NULL) {
-      // No provider desired, choose solely based on relevance.
-      if (AutocompleteMatch::MoreRelevant(*i, *default_match_))
-        default_match_ = i;
-    } else {
-      // Desired provider trumps any undesired provider; otherwise choose based
-      // on relevance.
-      const bool providers_match =
-          (i->provider == selection.provider_affinity);
-      const bool default_provider_doesnt_match =
-          (default_match_->provider != selection.provider_affinity);
-      if ((providers_match && default_provider_doesnt_match) ||
-          ((providers_match || default_provider_doesnt_match) &&
-           AutocompleteMatch::MoreRelevant(*i, *default_match_)))
-        default_match_ = i;
-    }
-  }
-  return false;
+  default_match_ = begin();
 }
 
 std::wstring AutocompleteResult::GetAlternateNavURL(
@@ -548,8 +518,7 @@ AutocompleteController::AutocompleteController(ACControllerListener* listener,
     : listener_(listener) {
   providers_.push_back(new SearchProvider(this, profile));
   providers_.push_back(new HistoryURLProvider(this, profile));
-  keyword_provider_ = new KeywordProvider(this, profile);
-  providers_.push_back(keyword_provider_);
+  providers_.push_back(new KeywordProvider(this, profile));
   if (listener) {
     // These providers are async-only, so there's no need to create them when
     // we'll only be doing synchronous queries.