Fix regression where hitting enter in the Omnibox would ignore recent editing.

Sadly, the only way I could find to fix this was a latest_result() accessor, because when the user hits enter, we really do need to use the very latest results.  This just highlights how more of this stuff should move to the AutocompleteController.

BUG=13428
TEST=Type "food.c" and wait for the popup to stabilize.  Then _very_ quickly type "om" and hit enter.  You should navigate to food.com instead of searching for "food.c".
Review URL: http://codereview.chromium.org/118398

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

4 files changed, 29 insertions, 17 deletions

diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc
index 3276b6e..4c94637 100644
--- a/chrome/browser/autocomplete/autocomplete.cc
+++ b/chrome/browser/autocomplete/autocomplete.cc
@@ -703,8 +703,7 @@ void AutocompleteController::Stop(bool clear_result) {
   update_pending_ = false;
   if (clear_result)
     result_.Reset();
-  latest_result_.CopyFrom(result_);  // Not strictly necessary, but keeps
-                                     // internal state consistent.
+  latest_result_.CopyFrom(result_);
   coalesce_timer_.Stop();
   max_delay_timer_.Stop();
 }
diff --git a/chrome/browser/autocomplete/autocomplete.h b/chrome/browser/autocomplete/autocomplete.h
index 44664fc..9d566fc 100644
--- a/chrome/browser/autocomplete/autocomplete.h
+++ b/chrome/browser/autocomplete/autocomplete.h
@@ -764,6 +764,9 @@ class AutocompleteController : public ACProviderListener {
   // Getters
   const AutocompleteInput& input() const { return input_; }
   const AutocompleteResult& result() const { return result_; }
+  // This next is temporary and should go away when
+  // AutocompletePopup::URLsForCurrentSelection() moves to the controller.
+  const AutocompleteResult& latest_result() const { return latest_result_; }
   const bool done() const { return done_; }
 
   // From AutocompleteProvider::Listener
@@ -777,6 +780,7 @@ class AutocompleteController : public ACProviderListener {
 
   // Copies |latest_result_| to |result_| and notifies observers of updates.
   void CommitResult();
+
   // Returns the matches from |provider| whose destination urls are not in
   // |latest_result_|.
   ACMatches GetMatchesNotInLatestResult(
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc
index 0964781..21e28c7 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.cc
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc
@@ -145,32 +145,38 @@ GURL AutocompletePopupModel::URLsForCurrentSelection(
     PageTransition::Type* transition,
     bool* is_history_what_you_typed_match,
     GURL* alternate_nav_url) const {
-  // We need to use the result on the controller, because if the popup is open,
-  // the user changes the contents of the edit, and then presses enter before
-  // any results have been displayed, results_ will be nonempty but wrong.  (In
-  // most other cases, the controller's results will match the popup's.)
-  // TODO(pkasting): If manually_selected_match_ moves to the controller, this
-  // can move to the edit.
-  if (controller_->result().empty())
-    return GURL();
-
-  const AutocompleteResult& result = controller_->result();
+  const AutocompleteResult* result;
   AutocompleteResult::const_iterator match;
   if (!controller_->done()) {
+    result = &controller_->latest_result();
+    // It's technically possible for |result| to be empty if no provider returns
+    // a synchronous result but the query has not completed synchronously;
+    // pratically, however, that should never actually happen.
+    if (result->empty())
+      return GURL();
     // The user cannot have manually selected a match, or the query would have
     // stopped.  So the default match must be the desired selection.
-    match = result.default_match();
+    match = result->default_match();
   } else {
-    // The query isn't running, so the popup can't possibly be out of date.
-    DCHECK(selected_line_ < result.size());
-    match = result.begin() + selected_line_;
+    // The query isn't running, so the standard result set can't possibly be out
+    // of date.
+    //
+    // NOTE: In practice, it should actually be safe to use
+    // controller_->latest_result() here too, since the controller keeps that
+    // up-to-date.  However we generally try to avoid referring to that.
+    result = &controller_->result();
+    // If there are no results, the popup is closed, so URLsForDefaultMatch()
+    // should have been called instead.
+    DCHECK(!result->empty());
+    DCHECK(selected_line_ < result->size());
+    match = result->begin() + selected_line_;
   }
   if (transition)
     *transition = match->transition;
   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.alternate_nav_url();
+    *alternate_nav_url = result->alternate_nav_url();
   return match->destination_url;
 }
 
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.h b/chrome/browser/autocomplete/autocomplete_popup_model.h
index 94772e6..c0c6185 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.h
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.h
@@ -86,6 +86,9 @@ class AutocompletePopupModel : public NotificationObserver {
   // If |alternate_nav_url| is non-NULL, it will be set to the alternate
   // navigation URL for |url| if one exists, or left unchanged otherwise.  See
   // comments on AutocompleteResult::GetAlternateNavURL().
+  //
+  // TODO(pkasting): When manually_selected_match_ moves to the controller, this
+  // can move too.
   GURL URLsForCurrentSelection(
       PageTransition::Type* transition,
       bool* is_history_what_you_typed_match,