The accidental search infobar was almost never appearing. Fix that regression. Now we don't rely on the presence of a "what you typed" match in the result set; instead the AutocompleteResult determines the alternate nav URL directly.

To do this I refactored some of the code from the history URL provider over to AutocompleteInput.

BUG=10808
Review URL: http://codereview.chromium.org/92140

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

6 files changed, 87 insertions, 103 deletions

diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index 8755834..137c923 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -53,6 +53,15 @@ AutocompleteInput::AutocompleteInput(const std::wstring& text,
   if (type_ == INVALID)
     return;
 
+  if ((type_ == UNKNOWN) || (type_ == REQUESTED_URL) || (type_ == URL)) {
+    GURL canonicalized_url(URLFixerUpper::FixupURL(WideToUTF8(text_),
+                                                   WideToUTF8(desired_tld_)));
+    if (canonicalized_url.is_valid() &&
+        (!canonicalized_url.IsStandard() || canonicalized_url.SchemeIsFile() ||
+         !canonicalized_url.host().empty()))
+      canonicalized_url_ = canonicalized_url;
+  }
+
   if (type_ == FORCED_QUERY && text_[0] == L'?')
     text_.erase(0, 1);
 }
@@ -521,11 +530,14 @@ void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) {
   // reconstruct them.
   default_match_ = (rhs.default_match_ == rhs.end()) ?
       end() : (begin() + (rhs.default_match_ - rhs.begin()));
+
+  alternate_nav_url_ = rhs.alternate_nav_url_;
 }
 
 void AutocompleteResult::AppendMatches(const ACMatches& matches) {
   std::copy(matches.begin(), matches.end(), std::back_inserter(matches_));
   default_match_ = end();
+  alternate_nav_url_ = GURL();
 }
 
 void AutocompleteResult::AddMatch(const AutocompleteMatch& match) {
@@ -540,7 +552,7 @@ void AutocompleteResult::AddMatch(const AutocompleteMatch& match) {
   default_match_ = begin() + default_offset;
 }
 
-void AutocompleteResult::SortAndCull() {
+void AutocompleteResult::SortAndCull(const AutocompleteInput& input) {
   // Remove duplicates.
   std::sort(matches_.begin(), matches_.end(),
             &AutocompleteMatch::DestinationSortFunc);
@@ -565,25 +577,18 @@ void AutocompleteResult::SortAndCull() {
       i->relevance = -i->relevance;
   }
 
-  // Now put the final result set in order.
+  // Put the final result set in order.
   std::sort(matches_.begin(), matches_.end(), &AutocompleteMatch::MoreRelevant);
   default_match_ = begin();
-}
 
-GURL AutocompleteResult::GetAlternateNavURL(
-    const AutocompleteInput& input,
-    const_iterator match) const {
+  // Set the alternate nav URL.
+  alternate_nav_url_ = GURL();
   if (((input.type() == AutocompleteInput::UNKNOWN) ||
        (input.type() == AutocompleteInput::REQUESTED_URL)) &&
-      (match->transition != PageTransition::TYPED)) {
-    for (const_iterator i(begin()); i != end(); ++i) {
-      if (i->is_history_what_you_typed_match) {
-        return (i->destination_url == match->destination_url) ?
-            GURL() : i->destination_url;
-      }
-    }
-  }
-  return GURL();
+      (default_match_ != end()) &&
+      (default_match_->transition != PageTransition::TYPED) &&
+      (input.canonicalized_url() != default_match_->destination_url))
+    alternate_nav_url_ = input.canonicalized_url();
 }
 
 #ifndef NDEBUG
@@ -726,7 +731,7 @@ void AutocompleteController::UpdateLatestResult(bool is_synchronous_pass) {
   }
 
   // Sort the matches and trim to a small number of "best" matches.
-  latest_result_.SortAndCull();
+  latest_result_.SortAndCull(input_);
 
   if (history_contents_provider_)
     AddHistoryContentsShortcut();
diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h
index 4c78aa1..cf40c1c 100644
--- a/chrome/browser/autocomplete/autocomplete.h
+++ b/chrome/browser/autocomplete/autocomplete.h
@@ -210,18 +210,24 @@ class AutocompleteInput {
   // type/scheme/etc. should use this.
   void set_text(const std::wstring& text) { text_ = 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
+  // '.' (use "com" instead of ".com").
+  const std::wstring& desired_tld() const { return desired_tld_; }
+
   // The type of input supplied.
   Type type() const { return type_; }
 
+  // Returns parsed URL components.
+  const url_parse::Parsed& parts() const { return parts_; }
+
   // The scheme parsed from the provided text; only meaningful when type_ is
   // URL.
   const std::wstring& scheme() const { return scheme_; }
 
-  // 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
-  // '.' (use "com" instead of ".com").
-  const std::wstring& desired_tld() const { return desired_tld_; }
+  // The input as an URL to navigate to, if possible.
+  const GURL& canonicalized_url() const { return canonicalized_url_; }
 
   // Returns whether inline autocompletion should be prevented.
   const bool prevent_inline_autocomplete() const {
@@ -244,15 +250,13 @@ class AutocompleteInput {
   // Resets all internal variables to the null-constructed state.
   void Clear();
 
-  // Returns parsed URL components.
-  const url_parse::Parsed& parts() const { return parts_; }
-
  private:
   std::wstring text_;
+  std::wstring desired_tld_;
   Type type_;
   url_parse::Parsed parts_;
   std::wstring scheme_;
-  std::wstring desired_tld_;
+  GURL canonicalized_url_;
   bool prevent_inline_autocomplete_;
   bool prefer_keyword_;
   bool synchronous_only_;
@@ -621,8 +625,9 @@ class AutocompleteResult {
   void AppendMatches(const ACMatches& matches);
 
   // Removes duplicates, puts the list in sorted order and culls to leave only
-  // the best kMaxMatches results.  Sets the default match to the best match.
-  void SortAndCull();
+  // the best kMaxMatches results.  Sets the default match to the best match
+  // and updates the alternate nav URL.
+  void SortAndCull(const AutocompleteInput& input);
 
   // Vector-style accessors/operators.
   size_t size() const { return matches_.size(); }
@@ -642,16 +647,7 @@ class AutocompleteResult {
   // end() if there is no default match.
   const_iterator default_match() const { return default_match_; }
 
-  // Given some input and a particular match in this result set, returns the
-  // "alternate navigation URL", if any, for that match.  This is a URL to try
-  // offering as a navigational option in case the user didn't actually mean to
-  // navigate to the URL of |match|.  For example, if the user's local intranet
-  // contains site "foo", and the user types "foo", we default to searching for
-  // "foo" when the user may have meant to navigate there.  In cases like this,
-  // |match| will point to the "search for 'foo'" result, and this function will
-  // return "http://foo/".
-  GURL GetAlternateNavURL(const AutocompleteInput& input,
-                          const_iterator match) const;
+  GURL alternate_nav_url() const { return alternate_nav_url_; }
 
   // Releases the resources associated with this object. Some callers may
   // want to perform several searches without creating new results each time.
@@ -671,9 +667,20 @@ class AutocompleteResult {
   // up showing an additional shortcut for Destinations->History, see
   // AddHistoryContentsShortcut.
   static size_t max_matches_;
+
   ACMatches matches_;
+
   const_iterator default_match_;
 
+  // The "alternate navigation URL", if any, for this result set.  This is a URL
+  // to try offering as a navigational option in case the user navigated to the
+  // URL of the default match but intended something else.  For example, if the
+  // user's local intranet contains site "foo", and the user types "foo", we
+  // default to searching for "foo" when the user may have meant to navigate
+  // there.  In cases like this, the default match will point to the "search for
+  // 'foo'" result, and this will contain "http://foo/".
+  GURL alternate_nav_url_;
+
   DISALLOW_EVIL_CONSTRUCTORS(AutocompleteResult);
 };
 
diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc
index 0065f36..98473ed 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit.cc
@@ -204,8 +204,7 @@ bool AutocompleteEditModel::CanPasteAndGo(const std::wstring& text) const {
   DCHECK(match != result.end());
   paste_and_go_url_ = match->destination_url;
   paste_and_go_transition_ = match->transition;
-  paste_and_go_alternate_nav_url_ =
-      result.GetAlternateNavURL(paste_and_go_controller->input(), match);
+  paste_and_go_alternate_nav_url_ = result.alternate_nav_url();
 
   return paste_and_go_url_.is_valid();
 }
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc
index 46bd01c..0186240 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.cc
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc
@@ -170,7 +170,7 @@ GURL AutocompletePopupModel::URLsForCurrentSelection(
   if (is_history_what_you_typed_match)
     *is_history_what_you_typed_match = match->is_history_what_you_typed_match;
   if (alternate_nav_url && manually_selected_match_.empty())
-    *alternate_nav_url = result.GetAlternateNavURL(controller_->input(), match);
+    *alternate_nav_url = result.alternate_nav_url();
   return match->destination_url;
 }
 
@@ -202,7 +202,7 @@ GURL AutocompletePopupModel::URLsForDefaultMatch(
   if (is_history_what_you_typed_match)
     *is_history_what_you_typed_match = match->is_history_what_you_typed_match;
   if (alternate_nav_url)
-    *alternate_nav_url = result.GetAlternateNavURL(controller_->input(), match);
+    *alternate_nav_url = result.alternate_nav_url();
   return match->destination_url;
 }
 
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index 4ddeefd..2e91e95 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -31,12 +31,10 @@ using base::TimeTicks;
 
 HistoryURLProviderParams::HistoryURLProviderParams(
     const AutocompleteInput& input,
-    const AutocompleteInput& original_input,
     bool trim_http,
     const std::wstring& languages)
     : message_loop(MessageLoop::current()),
       input(input),
-      original_input(original_input),
       trim_http(trim_http),
       cancel(false),
       languages(languages) {
@@ -102,7 +100,7 @@ void HistoryURLProvider::DeleteMatch(const AutocompleteMatch& match) {
   if (!done_) {
     // Copy params_->input to avoid a race condition where params_ gets deleted
     // out from under us on the other thread after we set params_->cancel here.
-    AutocompleteInput input(params_->original_input);
+    AutocompleteInput input(params_->input);
     params_->cancel = true;
     RunAutocompletePasses(input, false);
   }
@@ -135,15 +133,11 @@ void HistoryURLProvider::DoAutocomplete(history::HistoryBackend* backend,
                                         HistoryURLProviderParams* params) {
   // For non-UNKNOWN input, create a What You Typed match, which we'll need
   // below.
-  bool have_what_you_typed_match;
-  // This uses |original_input| because unlike the rest of the machinery here,
-  // it needs to take the user's desired_tld() into account; if it doesn't, it
-  // may convert "56" + ctrl into "0.0.0.56.com" instead of "56.com" like the
-  // user probably wanted.
-  AutocompleteMatch what_you_typed_match(SuggestExactInput(
-      params->original_input, params->trim_http, &have_what_you_typed_match));
-  have_what_you_typed_match &=
+  bool have_what_you_typed_match =
+      params->input.canonicalized_url().is_valid() &&
       (params->input.type() != AutocompleteInput::UNKNOWN);
+  AutocompleteMatch what_you_typed_match(SuggestExactInput(params->input,
+                                                           params->trim_http));
 
   // Get the matching URLs from the DB
   typedef std::vector<history::URLRow> URLRowVector;
@@ -237,25 +231,15 @@ void HistoryURLProvider::QueryComplete(
 
 AutocompleteMatch HistoryURLProvider::SuggestExactInput(
     const AutocompleteInput& input,
-    bool trim_http,
-    bool* valid) {
+    bool trim_http) {
   AutocompleteMatch match(this,
       CalculateRelevance(input.type(), WHAT_YOU_TYPED, 0), false,
       AutocompleteMatch::URL_WHAT_YOU_TYPED);
 
-  // Try to canonicalize the URL.  If this fails, don't create a What You Typed
-  // suggestion, since it can't be navigated to.  We also need this so other
-  // history suggestions don't duplicate the same effective URL as this.
-  GURL canonicalized_url(URLFixerUpper::FixupURL(WideToUTF8(input.text()),
-      WideToUTF8(input.desired_tld())));
-  if (!canonicalized_url.is_valid() ||
-      (canonicalized_url.IsStandard() &&
-       !canonicalized_url.SchemeIsFile() && canonicalized_url.host().empty())) {
-    *valid = false;
-  } else {
-    *valid = true;
-    match.destination_url = canonicalized_url;
-    match.fill_into_edit = StringForURLDisplay(canonicalized_url, false);
+  const GURL& url = input.canonicalized_url();
+  if (url.is_valid()) {
+    match.destination_url = url;
+    match.fill_into_edit = StringForURLDisplay(url, false);
     // NOTE: Don't set match.input_location (to allow inline autocompletion)
     // here, it's surprising and annoying.
     // Trim off "http://" if the user didn't type it.
@@ -310,9 +294,6 @@ bool HistoryURLProvider::FixupExactSuggestion(history::URLDatabase* db,
   if (!db->GetRowForURL(match.destination_url, &info)) {
     if (params->input.desired_tld().empty())
       return false;
-    // This code should match what SuggestExactInput() would do with no
-    // desired_tld().
-    // TODO(brettw) make autocomplete use GURL!
     GURL destination_url(
         URLFixerUpper::FixupURL(WideToUTF8(params->input.text()),
         std::string()));
@@ -632,8 +613,9 @@ void HistoryURLProvider::EnsureMatchPresent(
     matches->push_back(match);
 }
 
-void HistoryURLProvider::RunAutocompletePasses(const AutocompleteInput& input,
-                                               bool run_pass_1) {
+void HistoryURLProvider::RunAutocompletePasses(
+    const AutocompleteInput& input,
+    bool fixup_input_and_run_pass_1) {
   matches_.clear();
 
   if ((input.type() != AutocompleteInput::UNKNOWN) &&
@@ -646,10 +628,8 @@ void HistoryURLProvider::RunAutocompletePasses(const AutocompleteInput& input,
   // we'll run this again in DoAutocomplete() and use that result instead.
   const bool trim_http = !url_util::FindAndCompareScheme(
       WideToUTF8(input.text()), chrome::kHttpScheme, NULL);
-  bool valid;
-  AutocompleteMatch match(SuggestExactInput(input, trim_http, &valid));
-  if (valid)
-    matches_.push_back(match);
+  if (input.canonicalized_url().is_valid())
+    matches_.push_back(SuggestExactInput(input, trim_http));
 
   // We'll need the history service to run both passes, so try to obtain it.
   HistoryService* const history_service = profile_ ?
@@ -657,30 +637,29 @@ void HistoryURLProvider::RunAutocompletePasses(const AutocompleteInput& input,
   if (!history_service)
     return;
 
-  // Do some fixup on the user input before matching against it, so we provide
-  // good results for local file paths, input with spaces, etc.
-  // NOTE: This purposefully doesn't take input.desired_tld() into account; if
-  // it did, then holding "ctrl" would change all the results from the
-  // HistoryURLProvider provider, not just the What You Typed Result.
-  AutocompleteInput fixed_input(input);
-  const std::wstring fixed_text(FixupUserInput(input));
-  if (fixed_text.empty()) {
-    // Conceivably fixup could result in an empty string (although I don't
-    // have cases where this happens offhand).  We can't do anything with
-    // empty input, so just bail; otherwise we'd crash later.
-    return;
-  }
-  fixed_input.set_text(fixed_text);
-
   // Create the data structure for the autocomplete passes.  We'll save this off
   // onto the |params_| member for later deletion below if we need to run pass
   // 2.
   const std::wstring& languages = profile_ ?
       profile_->GetPrefs()->GetString(prefs::kAcceptLanguages) : std::wstring();
   scoped_ptr<HistoryURLProviderParams> params(
-      new HistoryURLProviderParams(fixed_input, input, trim_http, languages));
+      new HistoryURLProviderParams(input, trim_http, languages));
+
+  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.
+    // NOTE: This purposefully doesn't take input.desired_tld() into account; if
+    // it did, then holding "ctrl" would change all the results from the
+    // HistoryURLProvider provider, not just the What You Typed Result.
+    const std::wstring fixed_text(FixupUserInput(input));
+    if (fixed_text.empty()) {
+      // Conceivably fixup could result in an empty string (although I don't
+      // have cases where this happens offhand).  We can't do anything with
+      // empty input, so just bail; otherwise we'd crash later.
+      return;
+    }
+    params->input.set_text(fixed_text);
 
-  if (run_pass_1) {
     // Pass 1: Get the in-memory URL database, and use it to find and promote
     // the inline autocomplete match, if any.
     history::URLDatabase* url_db = history_service->in_memory_database();
diff --git a/chrome/browser/autocomplete/history_url_provider.h b/chrome/browser/autocomplete/history_url_provider.h
index 00e03da..3160796 100644
--- a/chrome/browser/autocomplete/history_url_provider.h
+++ b/chrome/browser/autocomplete/history_url_provider.h
@@ -85,7 +85,6 @@ class HistoryBackend;
 // service.
 struct HistoryURLProviderParams {
   HistoryURLProviderParams(const AutocompleteInput& input,
-                           const AutocompleteInput& original_input,
                            bool trim_http,
                            const std::wstring& languages);
 
@@ -95,10 +94,6 @@ struct HistoryURLProviderParams {
   // live beyond the original query while it runs on the history thread.
   AutocompleteInput input;
 
-  // The same as |input|, but without any fixup performed beforehand on the
-  // input text.  This is used when calling SuggestExactInput().
-  AutocompleteInput original_input;
-
   // Set when "http://" should be trimmed from the beginning of the URLs.
   bool trim_http;
 
@@ -317,7 +312,8 @@ class HistoryURLProvider : public AutocompleteProvider {
                                  bool promote);
 
   // Helper function that actually launches the two autocomplete passes.
-  void RunAutocompletePasses(const AutocompleteInput& input, bool run_pass_1);
+  void RunAutocompletePasses(const AutocompleteInput& input,
+                             bool fixup_input_and_run_pass_1);
 
   // Returns the best prefix that begins |text|.  "Best" means "greatest number
   // of components".  This may return NULL if no prefix begins |text|.
@@ -328,11 +324,9 @@ class HistoryURLProvider : public AutocompleteProvider {
   const Prefix* BestPrefix(const GURL& text,
                            const std::wstring& prefix_suffix) const;
 
-  // Returns a match corresponding to exactly what the user has typed.  This is
-  // only valid if |valid| is set to true.
+  // Returns a match corresponding to exactly what the user has typed.
   AutocompleteMatch SuggestExactInput(const AutocompleteInput& input,
-                                      bool trim_http,
-                                      bool* valid);
+                                      bool trim_http);
 
   // Assumes |params| contains the "what you typed" suggestion created by
   // SuggestExactInput().  Looks up its info in the DB.  If found, fills in the