Minor autocomplete fixes while trying to track down a topcrash:

* Turn some DCHECKs into CHECKs (I don't think any of these will get hit)
* Ensure that URLsForDefaultMatch() doesn't leave any results lying around on the controller, since the popup isn't open (and those two states should be locked in step)
* Eliminate some old code in Move() that shouldn't have actually done anything anymore (since result() should already be in sync with the popup, and SetSelectedLine() is going to stop the controller)
* Use a pre-existing helper function at two places in the Edit for readability

BUG=none
TEST=none
Review URL: http://codereview.chromium.org/180045

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

2 files changed, 32 insertions, 29 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc
index b842884..66cc59f 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit.cc
@@ -6,6 +6,7 @@
 
 #include "base/basictypes.h"
 #include "base/string_util.h"
+#include "base/time.h"  // TEMPORARY
 #include "chrome/browser/autocomplete/autocomplete_edit_view.h"
 #include "chrome/browser/autocomplete/autocomplete_popup_model.h"
 #include "chrome/browser/autocomplete/keyword_provider.h"
@@ -53,6 +54,9 @@ AutocompleteEditModel::AutocompleteEditModel(
     // we'll set this before each call to the controller.
     paste_and_go_controller = new AutocompleteController(NULL);
   }
+
+  temporary_timer_.Start(base::TimeDelta::FromMilliseconds(10), this,
+                         &AutocompleteEditModel::TemporaryTimerFired);
 }
 
 AutocompleteEditModel::~AutocompleteEditModel() {
@@ -384,7 +388,7 @@ void AutocompleteEditModel::OnUpOrDownKeyPressed(int count) {
   // NOTE: This purposefully don't trigger any code that resets paste_state_.
 
   if (!popup_->IsOpen()) {
-    if (popup_->autocomplete_controller()->done()) {
+    if (!query_in_progress()) {
       // The popup is neither open nor working on a query already.  So, start an
       // autocomplete query for the current text.  This also sets
       // user_input_in_progress_ to true, which we want: if the user has started
@@ -551,7 +555,7 @@ GURL AutocompleteEditModel::GetURLForCurrentText(
     PageTransition::Type* transition,
     bool* is_history_what_you_typed_match,
     GURL* alternate_nav_url) {
-  return (popup_->IsOpen() || !popup_->autocomplete_controller()->done()) ?
+  return (popup_->IsOpen() || query_in_progress()) ?
       popup_->URLsForCurrentSelection(transition,
                                       is_history_what_you_typed_match,
                                       alternate_nav_url) :
@@ -560,3 +564,7 @@ GURL AutocompleteEditModel::GetURLForCurrentText(
                                   is_history_what_you_typed_match,
                                   alternate_nav_url);
 }
+
+void AutocompleteEditModel::TemporaryTimerFired() {
+  GetURLForCurrentText(NULL, NULL, NULL);
+}
\ No newline at end of file
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc
index d3a6fe6..0e75699 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.cc
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc
@@ -150,6 +150,7 @@ GURL AutocompletePopupModel::URLsForCurrentSelection(
     PageTransition::Type* transition,
     bool* is_history_what_you_typed_match,
     GURL* alternate_nav_url) const {
+  CHECK(IsOpen());
   const AutocompleteResult* result;
   AutocompleteResult::const_iterator match;
   if (!controller_->done()) {
@@ -170,8 +171,9 @@ GURL AutocompletePopupModel::URLsForCurrentSelection(
     // 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.
+    // If there are no results, the popup should be closed (so we should have
+    // failed the CHECK above), and URLsForDefaultMatch() should have been
+    // called instead.
     CHECK(!result->empty());
     CHECK(selected_line_ < result->size());
     match = result->begin() + selected_line_;
@@ -193,28 +195,32 @@ GURL AutocompletePopupModel::URLsForDefaultMatch(
     GURL* alternate_nav_url) {
   // We had better not already be doing anything, or this call will blow it
   // away.
-  DCHECK(!IsOpen());
-  DCHECK(controller_->done());
+  CHECK(!IsOpen());
+  CHECK(controller_->done());
 
   // Run the new query and get only the synchronously available matches.
   inside_synchronous_query_ = true;  // Tell Observe() not to notify the edit or
                                      // update our appearance.
   controller_->Start(text, desired_tld, true, false, true);
   inside_synchronous_query_ = false;
-  DCHECK(controller_->done());
+  CHECK(controller_->done());
+
   const AutocompleteResult& result = controller_->result();
-  if (result.empty())
-    return GURL();
+  GURL destination_url;
+  if (!result.empty()) {
+    // Get the URLs for the default match.
+    const AutocompleteResult::const_iterator match = result.default_match();
+    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)
+      *alternate_nav_url = result.alternate_nav_url();
+    destination_url = match->destination_url;
+  }
 
-  // Get the URLs for the default match.
-  const AutocompleteResult::const_iterator match = result.default_match();
-  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)
-    *alternate_nav_url = result.alternate_nav_url();
-  return match->destination_url;
+  controller_->Stop(true);  // Keeps our state consistent.
+  return destination_url;
 }
 
 bool AutocompletePopupModel::GetKeywordForMatch(const AutocompleteMatch& match,
@@ -253,17 +259,6 @@ AutocompleteLog* AutocompletePopupModel::GetAutocompleteLog() {
 }
 
 void AutocompletePopupModel::Move(int count) {
-  // TODO(pkasting): Temporary hack.  If the query is running while the popup is
-  // open, we might be showing the results of the previous query still.  Force
-  // the popup to display the latest results so the popup and the controller
-  // aren't out of sync.  The better fix here is to roll the controller back to
-  // be in sync with what the popup is showing.
-  if (IsOpen() && !controller_->done()) {
-    Observe(NotificationType::AUTOCOMPLETE_CONTROLLER_RESULT_UPDATED,
-            Source<AutocompleteController>(controller_.get()),
-            Details<const AutocompleteResult>(&controller_->result()));
-  }
-
   const AutocompleteResult& result = controller_->result();
   if (result.empty())
     return;