Makes autocompleting previous search terms work on word boundaries.

BUG=80057
TEST=covered by unit tests, but see bugs for details.
R=pkasting@chromium.org

Review URL: http://codereview.chromium.org/6893140

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

14 files changed, 108 insertions, 45 deletions

diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index 9ea708f..b942b1f 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -48,7 +48,6 @@ using base::TimeDelta;
 
 AutocompleteInput::AutocompleteInput()
   : type_(INVALID),
-    initial_prevent_inline_autocomplete_(false),
     prevent_inline_autocomplete_(false),
     prefer_keyword_(false),
     allow_exact_keyword_match_(true),
@@ -61,17 +60,14 @@ AutocompleteInput::AutocompleteInput(const string16& text,
                                      bool prefer_keyword,
                                      bool allow_exact_keyword_match,
                                      MatchesRequested matches_requested)
-    : original_text_(text),
-      desired_tld_(desired_tld),
-      initial_prevent_inline_autocomplete_(prevent_inline_autocomplete),
+    : desired_tld_(desired_tld),
       prevent_inline_autocomplete_(prevent_inline_autocomplete),
       prefer_keyword_(prefer_keyword),
       allow_exact_keyword_match_(allow_exact_keyword_match),
       matches_requested_(matches_requested) {
-  // Trim whitespace from edges of input; don't inline autocomplete if there
-  // was trailing whitespace.
-  if (TrimWhitespace(text, TRIM_ALL, &text_) & TRIM_TRAILING)
-    prevent_inline_autocomplete_ = true;
+  // None of the providers care about leading white space so we always trim it.
+  // Providers that care about trailing white space handle trimming themselves.
+  TrimWhitespace(text, TRIM_LEADING, &text_);
 
   GURL canonicalized_url;
   type_ = Parse(text_, desired_tld, &parts_, &scheme_, &canonicalized_url);
diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h
index 95b1ee35..eaf2318 100644
--- a/chrome/browser/autocomplete/autocomplete.h
+++ b/chrome/browser/autocomplete/autocomplete.h
@@ -268,10 +268,6 @@ class AutocompleteInput {
   // type/scheme/etc. should use this.
   void set_text(const string16& text) { text_ = text; }
 
-  // The text supplied to the constructor. This differs from |text| if the text
-  // supplied to the constructor had leading or trailing white space.
-  const string16& original_text() const { return original_text_; }
-
   // User's desired TLD, if one is not already present in the text to
   // autocomplete.  When this is non-empty, it also implies that "www." should
   // be prepended to the domain where possible.  This should not have a leading
@@ -296,13 +292,6 @@ class AutocompleteInput {
     return prevent_inline_autocomplete_;
   }
 
-  // Returns the value of |prevent_inline_autocomplete| supplied to the
-  // constructor. This differs from the value returned by
-  // |prevent_inline_autocomplete()| if the input contained trailing whitespace.
-  bool initial_prevent_inline_autocomplete() const {
-    return initial_prevent_inline_autocomplete_;
-  }
-
   // Returns whether, given an input string consisting solely of a substituting
   // keyword, we should score it like a non-substituting keyword.
   bool prefer_keyword() const { return prefer_keyword_; }
@@ -323,13 +312,11 @@ class AutocompleteInput {
 
  private:
   string16 text_;
-  string16 original_text_;
   string16 desired_tld_;
   Type type_;
   url_parse::Parsed parts_;
   string16 scheme_;
   GURL canonicalized_url_;
-  bool initial_prevent_inline_autocomplete_;
   bool prevent_inline_autocomplete_;
   bool prefer_keyword_;
   bool allow_exact_keyword_match_;
diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc
index 4fce90c..f733687 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit.cc
@@ -261,17 +261,17 @@ bool AutocompleteEditModel::UseVerbatimInstant() {
   // TODO(suzhe): Fix Mac port to display Instant suggest in a separated NSView,
   // so that we can display instant suggest along with composition text.
   const AutocompleteInput& input = autocomplete_controller_->input();
-  if (input.initial_prevent_inline_autocomplete())
+  if (input.prevent_inline_autocomplete())
     return true;
 #endif
 
-  // The value of input.initial_prevent_inline_autocomplete() is determined by
+  // The value of input.prevent_inline_autocomplete() is determined by
   // following conditions:
   // 1. If the caret is at the end of the text (checked below).
   // 2. If it's in IME composition mode.
   // As we use a separated widget for displaying the instant suggest, it won't
   // interfere with IME composition, so we don't need to care about the value of
-  // input.initial_prevent_inline_autocomplete() here.
+  // input.prevent_inline_autocomplete() here.
   if (view_->DeleteAtEndPressed() || (popup_->selected_line() != 0) ||
       just_deleted_text_)
     return true;
diff --git a/chrome/browser/autocomplete/history_provider.cc b/chrome/browser/autocomplete/history_provider.cc
index d0dc207..50fa80f 100644
--- a/chrome/browser/autocomplete/history_provider.cc
+++ b/chrome/browser/autocomplete/history_provider.cc
@@ -145,3 +145,11 @@ size_t HistoryProvider::TrimHttpPrefix(string16* url) {
   url->erase(scheme_pos, prefix_end - scheme_pos);
   return (scheme_pos == 0) ? prefix_end : 0;
 }
+
+// static
+bool HistoryProvider::PreventInlineAutocomplete(
+    const AutocompleteInput& input) {
+  return input.prevent_inline_autocomplete() ||
+        (!input.text().empty() &&
+         IsWhitespace(input.text()[input.text().size() - 1]));
+}
diff --git a/chrome/browser/autocomplete/history_provider.h b/chrome/browser/autocomplete/history_provider.h
index a304742..968cc8d 100644
--- a/chrome/browser/autocomplete/history_provider.h
+++ b/chrome/browser/autocomplete/history_provider.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -51,6 +51,13 @@ class HistoryProvider : public AutocompleteProvider {
   // NOTE: For a view-source: URL, this will trim from after "view-source:" and
   // return 0.
   static size_t TrimHttpPrefix(string16* url);
+
+  // Returns true if inline autocompletion should be prevented. Use this instead
+  // of |input.prevent_inline_autocomplete| if the input is passed through
+  // FixupUserInput(). This method returns true if
+  // |input.prevent_inline_autocomplete()| is true, or the input text contains
+  // trailing whitespace.
+  bool PreventInlineAutocomplete(const AutocompleteInput& input);
 };
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_HISTORY_PROVIDER_H_
diff --git a/chrome/browser/autocomplete/history_quick_provider.cc b/chrome/browser/autocomplete/history_quick_provider.cc
index c4b4f4e..1b0754f 100644
--- a/chrome/browser/autocomplete/history_quick_provider.cc
+++ b/chrome/browser/autocomplete/history_quick_provider.cc
@@ -109,8 +109,10 @@ void HistoryQuickProvider::DoAutocomplete() {
        match_iter != matches.end(); ++match_iter) {
     const ScoredHistoryMatch& history_match(*match_iter);
     if (history_match.raw_score > 0) {
-      AutocompleteMatch ac_match = QuickMatchToACMatch(history_match,
-                                                       &max_match_score);
+      AutocompleteMatch ac_match = QuickMatchToACMatch(
+          history_match,
+          PreventInlineAutocomplete(autocomplete_input_),
+          &max_match_score);
       matches_.push_back(ac_match);
     }
   }
@@ -118,6 +120,7 @@ void HistoryQuickProvider::DoAutocomplete() {
 
 AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
     const ScoredHistoryMatch& history_match,
+    bool prevent_inline_autocomplete,
     int* max_match_score) {
   DCHECK(max_match_score);
   const history::URLRow& info = history_match.url_info;
@@ -142,8 +145,7 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch(
       SpansFromTermMatch(new_matches, match.contents.size(), true);
   match.fill_into_edit = match.contents;
 
-  if (autocomplete_input_.prevent_inline_autocomplete() ||
-      !history_match.can_inline) {
+  if (prevent_inline_autocomplete || !history_match.can_inline) {
     match.inline_autocomplete_offset = string16::npos;
   } else {
     match.inline_autocomplete_offset =
diff --git a/chrome/browser/autocomplete/history_quick_provider.h b/chrome/browser/autocomplete/history_quick_provider.h
index 50e4001..69c7dc8 100644
--- a/chrome/browser/autocomplete/history_quick_provider.h
+++ b/chrome/browser/autocomplete/history_quick_provider.h
@@ -49,6 +49,7 @@ class HistoryQuickProvider : public HistoryProvider {
   // the maximum possible score for the match.
   AutocompleteMatch QuickMatchToACMatch(
       const history::ScoredHistoryMatch& history_match,
+      bool prevent_inline_autocomplete,
       int* max_match_score);
 
   // Determines the relevance score of |history_match|. The maximum allowed
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index 3b5354a..342efce 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -109,6 +109,7 @@ HistoryURLProviderParams::HistoryURLProviderParams(
     const std::string& languages)
     : message_loop(MessageLoop::current()),
       input(input),
+      prevent_inline_autocomplete(input.prevent_inline_autocomplete()),
       trim_http(trim_http),
       cancel(false),
       failed(false),
@@ -243,7 +244,7 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
     // regardless of the input type.
     exact_suggestion = 1;
     params->matches.push_back(what_you_typed_match);
-  } else if (params->input.prevent_inline_autocomplete() ||
+  } else if (params->prevent_inline_autocomplete ||
       history_matches.empty() ||
       !PromoteMatchForInlineAutocomplete(params, history_matches.front())) {
     // Failed to promote any URLs for inline autocompletion.  Use the What You
@@ -603,6 +604,9 @@ void HistoryURLProvider::RunAutocompletePasses(
   scoped_ptr<HistoryURLProviderParams> params(
       new HistoryURLProviderParams(input, trim_http, languages));
 
+  params->prevent_inline_autocomplete =
+      PreventInlineAutocomplete(input);
+
   if (fixup_input_and_run_pass_1) {
     // Do some fixup on the user input before matching against it, so we provide
     // good results for local file paths, input with spaces, etc.
@@ -792,7 +796,7 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
           net::FormatUrl(info.url(), languages, format_types,
                          UnescapeRule::SPACES, NULL, NULL,
                          &inline_autocomplete_offset));
-  if (!params->input.prevent_inline_autocomplete())
+  if (!params->prevent_inline_autocomplete)
     match.inline_autocomplete_offset = inline_autocomplete_offset;
   DCHECK((match.inline_autocomplete_offset == string16::npos) ||
          (match.inline_autocomplete_offset <= match.fill_into_edit.length()));
diff --git a/chrome/browser/autocomplete/history_url_provider.h b/chrome/browser/autocomplete/history_url_provider.h
index f9b9a9f..885990f 100644
--- a/chrome/browser/autocomplete/history_url_provider.h
+++ b/chrome/browser/autocomplete/history_url_provider.h
@@ -95,6 +95,11 @@ struct HistoryURLProviderParams {
   // live beyond the original query while it runs on the history thread.
   AutocompleteInput input;
 
+  // Should inline autocompletion be disabled? This is initalized from
+  // |input.prevent_inline_autocomplete()|, but set to false is the input
+  // contains trailing white space.
+  bool prevent_inline_autocomplete;
+
   // Set when "http://" should be trimmed from the beginning of the URLs.
   bool trim_http;
 
diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc
index 4d0de7f..67f3762 100644
--- a/chrome/browser/autocomplete/history_url_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc
@@ -493,3 +493,16 @@ TEST_F(HistoryURLProviderTestNoDB, NavigateWithoutDB) {
 
   RunTest(ASCIIToUTF16("this is a query"), string16(), false, NULL, 0);
 }
+
+TEST_F(HistoryURLProviderTest, DontAutocompleteOnTrailingWhitespace) {
+  AutocompleteInput input(ASCIIToUTF16("slash "), string16(), false,
+                          false, true, AutocompleteInput::ALL_MATCHES);
+  autocomplete_->Start(input, false);
+  if (!autocomplete_->done())
+    MessageLoop::current()->Run();
+
+  // None of the matches should attempt to autocomplete.
+  matches_ = autocomplete_->matches();
+  for (size_t i = 0; i < matches_.size(); ++i)
+    EXPECT_EQ(string16::npos, matches_[i].inline_autocomplete_offset);
+}
diff --git a/chrome/browser/autocomplete/keyword_provider.cc b/chrome/browser/autocomplete/keyword_provider.cc
index fab254f..69a64d8 100644
--- a/chrome/browser/autocomplete/keyword_provider.cc
+++ b/chrome/browser/autocomplete/keyword_provider.cc
@@ -280,8 +280,10 @@ bool KeywordProvider::ExtractKeywordFromInput(const AutocompleteInput& input,
       (input.type() == AutocompleteInput::FORCED_QUERY))
     return false;
 
+  string16 trimmed_input;
+  TrimWhitespace(input.text(), TRIM_TRAILING, &trimmed_input);
   *keyword = TemplateURLModel::CleanUserInputKeyword(
-      SplitKeywordFromInput(input.text(), true, remaining_input));
+      SplitKeywordFromInput(trimmed_input, true, remaining_input));
   return !keyword->empty();
 }
 
@@ -300,7 +302,7 @@ string16 KeywordProvider::SplitKeywordFromInput(
   // Set |remaining_input| to everything after the first token.
   DCHECK(remaining_input != NULL);
   const size_t remaining_start = trim_leading_whitespace ?
-    input.find_first_not_of(kWhitespaceUTF16, first_white) : first_white + 1;
+      input.find_first_not_of(kWhitespaceUTF16, first_white) : first_white + 1;
 
   if (remaining_start < input.length())
     remaining_input->assign(input.begin() + remaining_start, input.end());
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index 235341c..f82dd47 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -120,7 +120,7 @@ void SearchProvider::FinalizeInstantQuery(const string16& input_text,
                 CalculateRelevanceForWhatYouTyped() + 1,
                 AutocompleteMatch::SEARCH_SUGGEST,
                 did_not_accept_default_suggestion, false,
-                input_.initial_prevent_inline_autocomplete(), &match_map);
+                input_.prevent_inline_autocomplete(), &match_map);
   DCHECK_EQ(1u, match_map.size());
   matches_.push_back(match_map.begin()->second);
   // Sort the results so that UpdateFirstSearchDescription does the right thing.
@@ -173,9 +173,6 @@ void SearchProvider::Start(const AutocompleteInput& input,
       default_provider_suggest_text_.clear();
     else
       Stop();
-  } else if (minimal_changes &&
-             (input_.original_text() != input.original_text())) {
-    default_provider_suggest_text_.clear();
   }
 
   providers_.Set(default_provider, keyword_provider);
@@ -544,13 +541,13 @@ void SearchProvider::ConvertResultsToAutocompleteMatches() {
                   CalculateRelevanceForWhatYouTyped(),
                   AutocompleteMatch::SEARCH_WHAT_YOU_TYPED,
                   did_not_accept_default_suggestion, false,
-                  input_.initial_prevent_inline_autocomplete(), &map);
+                  input_.prevent_inline_autocomplete(), &map);
     if (!default_provider_suggest_text_.empty()) {
       AddMatchToMap(input_.text() + default_provider_suggest_text_,
                     input_.text(), CalculateRelevanceForWhatYouTyped() + 1,
                     AutocompleteMatch::SEARCH_SUGGEST,
                     did_not_accept_default_suggestion, false,
-                    input_.initial_prevent_inline_autocomplete(), &map);
+                    input_.prevent_inline_autocomplete(), &map);
     }
   }
 
@@ -646,7 +643,7 @@ void SearchProvider::AddHistoryResultsToMap(const HistoryResults& results,
                   is_keyword ? keyword_input_text_ : input_.text(),
                   relevance,
                   AutocompleteMatch::SEARCH_HISTORY, did_not_accept_suggestion,
-                  is_keyword, input_.initial_prevent_inline_autocomplete(),
+                  is_keyword, input_.prevent_inline_autocomplete(),
                   map);
   }
 }
@@ -663,7 +660,7 @@ void SearchProvider::AddSuggestResultsToMap(
                                                   is_keyword),
                   AutocompleteMatch::SEARCH_SUGGEST,
                   static_cast<int>(i), is_keyword,
-                  input_.initial_prevent_inline_autocomplete(), map);
+                  input_.prevent_inline_autocomplete(), map);
   }
 }
 
diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc
index 8156b63..68842b6 100644
--- a/chrome/browser/autocomplete/search_provider_unittest.cc
+++ b/chrome/browser/autocomplete/search_provider_unittest.cc
@@ -442,14 +442,13 @@ TEST_F(SearchProviderTest, DifferingText) {
   // Finalize the instant query immediately.
   provider_->FinalizeInstantQuery(ASCIIToUTF16("foo"), ASCIIToUTF16("bar"));
 
-  // Query with input that ends up getting trimmed to be the same as was
-  // originally supplied.
-  QueryForInput(ASCIIToUTF16("foo "), false, true);
+  // Query with the same input text, but trailing whitespace.
+  QueryForInput(ASCIIToUTF16("foo "), false, false);
 
   // There should only one match, for what you typed.
   EXPECT_EQ(1u, provider_->matches().size());
   GURL instant_url = GURL(default_t_url_->url()->ReplaceSearchTerms(
-      *default_t_url_, ASCIIToUTF16("foo"), 0, string16()));
+      *default_t_url_, ASCIIToUTF16("foo "), 0, string16()));
   AutocompleteMatch instant_match = FindMatchWithDestination(instant_url);
   EXPECT_FALSE(instant_match.destination_url.is_empty());
 }
@@ -492,3 +491,43 @@ TEST_F(SearchProviderTest, DontAutocompleteURLLikeTerms) {
       FindMatchWithDestination(what_you_typed_url);
   EXPECT_GT(what_you_typed_match.relevance, term_match.relevance);
 }
+
+// Verifies autocomplete of previously typed words works on word boundaries.
+TEST_F(SearchProviderTest, AutocompletePreviousSearchOnSpace) {
+  // Add an entry that corresponds to a search with two words.
+  string16 term(ASCIIToUTF16("two words"));
+  HistoryService* history =
+      profile_.GetHistoryService(Profile::EXPLICIT_ACCESS);
+  GURL term_url(default_t_url_->url()->ReplaceSearchTerms(
+      *default_t_url_, term, 0, string16()));
+  history->AddPageWithDetails(term_url, string16(), 1, 1,
+                              base::Time::Now(), false,
+                              history::SOURCE_BROWSED);
+  history->SetKeywordSearchTermsForURL(term_url, default_t_url_->id(), term);
+
+  profile_.BlockUntilHistoryProcessesPendingRequests();
+
+  QueryForInput(ASCIIToUTF16("two "), false, false);
+
+  // Wait until history and the suggest query complete.
+  profile_.BlockUntilHistoryProcessesPendingRequests();
+  ASSERT_NO_FATAL_FAILURE(FinishDefaultSuggestQuery());
+
+  // Provider should be done.
+  EXPECT_TRUE(provider_->done());
+
+  // There should be two matches, one for what you typed, the other for
+  // 'two words'.
+  ASSERT_EQ(2u, provider_->matches().size());
+  AutocompleteMatch term_match = FindMatchWithDestination(term_url);
+  EXPECT_FALSE(term_match.destination_url.is_empty());
+  GURL what_you_typed_url = GURL(default_t_url_->url()->ReplaceSearchTerms(
+      *default_t_url_, ASCIIToUTF16("two "), 0, string16()));
+  AutocompleteMatch what_you_typed_match =
+      FindMatchWithDestination(what_you_typed_url);
+  EXPECT_FALSE(what_you_typed_match.destination_url.is_empty());
+  // term_match should be autocompleted.
+  EXPECT_GT(term_match.relevance, what_you_typed_match.relevance);
+  // And the offset should be at 4.
+  EXPECT_EQ(4u, term_match.inline_autocomplete_offset);
+}
diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc
index 75886a3..e251da6 100644
--- a/chrome/browser/search_engines/template_url_model.cc
+++ b/chrome/browser/search_engines/template_url_model.cc
@@ -10,6 +10,7 @@
 #include "base/stl_util-inl.h"
 #include "base/string_number_conversions.h"
 #include "base/string_split.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/google/google_url_tracker.h"
@@ -155,6 +156,7 @@ string16 TemplateURLModel::GenerateKeyword(const GURL& url,
 string16 TemplateURLModel::CleanUserInputKeyword(const string16& keyword) {
   // Remove the scheme.
   string16 result(base::i18n::ToLower(keyword));
+  TrimWhitespace(result, TRIM_ALL, &result);
   url_parse::Component scheme_component;
   if (url_parse::ExtractScheme(UTF16ToUTF8(keyword).c_str(),
                                static_cast<int>(keyword.length()),