Fix a DCHECK failure in AutocompleteEditModel::OnPopupDataChanged()

chrome/browser/autocomplete/autocomplete_popup_model.cc

Index: chrome/browser/autocomplete/autocomplete_popup_model.cc
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc
index 1dc8a95b55d90d770a8e3e251f54a359dfa46521..5192b1e019f7e161c973dda33d827cdb8917fc41 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.cc
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc
@@ -90,7 +90,8 @@ void AutocompletePopupModel::SetHoveredLine(size_t line) {
 }
 
 void AutocompletePopupModel::SetSelectedLine(size_t line,
-                                             bool reset_to_default) {
+                                             bool reset_to_default,
+                                             bool force) {
   // We should at least be dealing with the results of the current query.  Note
   // that even if |line| was valid on entry, this may make it invalid.  We clamp
   // it below.
@@ -115,7 +116,7 @@ void AutocompletePopupModel::SetSelectedLine(size_t line,
         match.is_history_what_you_typed_match;
   }
 
-  if (line == selected_line_)
+  if (line == selected_line_ && !force)
     return;  // Nothing else to do.
 
   // We need to update |selected_line_| before calling OnPopupDataChanged(), so
@@ -158,7 +159,7 @@ void AutocompletePopupModel::SetSelectedLine(size_t line,
 void AutocompletePopupModel::ResetToDefaultMatch() {
   const AutocompleteResult& result = controller_->result();
   CHECK(!result.empty());
-  SetSelectedLine(result.default_match() - result.begin(), true);
+  SetSelectedLine(result.default_match() - result.begin(), true, false);
   view_->OnDragCanceled();
 }
 
@@ -267,7 +268,7 @@ void AutocompletePopupModel::Move(int count) {
   // Clamp the new line to [0, result_.count() - 1].
   const size_t new_line = selected_line_ + count;
   SetSelectedLine(((count < 0) && (new_line >= selected_line_)) ? 0 : new_line,
-                  false);
+                  false, false);
 }
 
 void AutocompletePopupModel::TryDeletingCurrentItem() {
@@ -284,17 +285,34 @@ void AutocompletePopupModel::TryDeletingCurrentItem() {
       controller_->result().match_at(selected_line_);
   if (match.deletable) {
     const size_t selected_line = selected_line_;
-    controller_->DeleteMatch(match);  // This may synchronously notify us that
-                                      // the results have changed.
+    const bool was_temporary_text = !manually_selected_match_.empty();
+    GURL old_url;
+
+    // If the user tries to delete a match, which is not manually selected,
+    // then we should not re-select it if it's not deleted at all.
+    // Otherwise it will be selected as temporary text unexpectedly.
+    // We can check the url of the default match before and after calling
+    // |controller->DeleteMatch()| to see if it's actually deleted or not.
+    // But if the match is manually selected, then we should always re-select
+    // the selected line manually no matter if the match is actually deleted
+    // or not, to make sure we stay in the temporary text mode.
+    if (!was_temporary_text)
+      old_url = controller_->result().match_at(selected_line).destination_url;
+
+    // This may synchronously notify us that the results have changed. And may

[Peter Kasting, 2011/01/25 21:56:44]

Nit: Given the DCHECK in DeleteMatch(), it would probably be more accurate to
say:

    // This will synchronously notify both the edit and us that the results have
    // changed, causing both to revert to the default match.

[James Su, 2011/01/25 22:32:37]

Done.

+    // cause edit model to revert the temporary text automatically.
+    controller_->DeleteMatch(match);
     const AutocompleteResult& result = controller_->result();
-    if (!result.empty()) {
+    if (!result.empty() &&
+        (was_temporary_text || selected_line != selected_line_ ||
+         old_url != result.match_at(selected_line).destination_url)) {
       // Move the selection to the next choice after the deleted one.
       // SetSelectedLine() will clamp to take care of the case where we deleted
       // the last item.
       // TODO(pkasting): Eventually the controller should take care of this
       // before notifying us, reducing flicker.  At that point the check for
       // deletability can move there too.
-      SetSelectedLine(selected_line, false);
+      SetSelectedLine(selected_line, false, true);
     }
   }
 }
@@ -311,6 +329,7 @@ void AutocompletePopupModel::Observe(NotificationType type,
       kNoMatch : static_cast<size_t>(result->default_match() - result->begin());
   // There had better not be a nonempty result set with no default match.
   CHECK((selected_line_ != kNoMatch) || result->empty());
+  manually_selected_match_.Clear();
   // If we're going to trim the window size to no longer include the hovered
   // line, turn hover off.  Practically, this shouldn't happen, but it
   // doesn't hurt to be defensive.