Fix a DCHECK failure in AutocompleteEditModel::OnPopupDataChanged()

chrome/browser/autocomplete/autocomplete_edit.cc

 // 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.
 
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/metrics/histogram.h"
@@ skipping 468 matching lines @@
   has_focus_ = false;
   control_key_state_ = UP;
   paste_state_ = NONE;
 }
 
 bool AutocompleteEditModel::OnEscapeKeyPressed() {
   if (has_temporary_text_) {
     AutocompleteMatch match;
     popup_->InfoForCurrentSelection(&match, NULL);
     if (match.destination_url != original_url_) {
-      // The user typed something, then selected a different item.  Restore the
-      // text they typed and change back to the default item.
-      // NOTE: This purposefully does not reset paste_state_.
-      just_deleted_text_ = false;
-      has_temporary_text_ = false;
-      popup_->ResetToDefaultMatch();
-      view_->OnRevertTemporaryText();
+      RevertTemporaryText(true);
       return true;
     }
   }
 
   // If the user wasn't editing, but merely had focus in the edit, allow <esc>
   // to be processed as an accelerator, so it can still be used to stop a load.
   // When the permanent text isn't all selected we still fall through to the
   // SelectAll() call below so users can arrow around in the text and then hit
   // <esc> to quickly replace all the text; this matches IE.
   if (!user_input_in_progress_ && view_->IsSelectAll())
@@ skipping 86 matching lines @@
       // user's selection gets screwy; and if we don't update the popup, and the
       // user reverts, then the selected item will be as if control is still
       // pressed, even though maybe it isn't any more.  There is no obvious
       // right answer here :(
     }
     view_->OnTemporaryTextMaybeChanged(DisplayTextFromUserText(text),
                                        save_original_selection);
     return;
   }
 
-  // All cases that can result in |has_temporary_text_| being set should have
-  // been handled by the conditional above.
-  DCHECK(!has_temporary_text_);
-
+  bool call_controller_onchanged = true;
   inline_autocomplete_text_ = text;
   if (view_->OnInlineAutocompleteTextMaybeChanged(
       DisplayTextFromUserText(user_text_ + inline_autocomplete_text_),
       DisplayTextFromUserText(user_text_).length()))
-    return;
+    call_controller_onchanged = false;
 
-  // All other code paths that return invoke OnChanged. We need to invoke
-  // OnChanged in case the destination url changed (as could happen when control
-  // is toggled).
-  controller_->OnChanged();
+  // If |has_temporary_text_| is true, then we previously had a manual selection
+  // but now don't (or |destination_for_temporary_text_change| would have been
+  // non-NULL). This can happen when deleting the selected item in the popup.
+  // In this case, we've already reverted the popup to the default match, so we
+  // need to revert ourselves as well.
+  if (has_temporary_text_) {
+    RevertTemporaryText(false);
+    call_controller_onchanged = false;
+  }
+
+  // We need to invoke OnChanged in case the destination url changed (as could
+  // happen when control is toggled).
+  if (call_controller_onchanged)
+    controller_->OnChanged();
 }
 
 bool AutocompleteEditModel::OnAfterPossibleChange(
     const string16& new_text,
     bool selection_differs,
     bool text_differs,
     bool just_deleted_text,
     bool allow_keyword_ui_change) {
   // Update the paste state as appropriate: if we're just finishing a paste
   // that replaced all the text, preserve that information; otherwise, if we've
@@ skipping 30 matching lines @@
     // Track when the user has deleted text so we won't allow inline
     // autocomplete.
     just_deleted_text_ = just_deleted_text;
   }
 
   view_->UpdatePopup();
 
   // Change to keyword mode if the user has typed a keyword name and is now
   // pressing space after the name. Accepting the keyword will update our
   // state, so in that case there's no need to also return true here.
-  if (text_differs && allow_keyword_ui_change && !just_deleted_text &&
-      MaybeAcceptKeywordBySpace(old_user_text, user_text_))
-    return false;
-
-  return true;
+  return !(text_differs && allow_keyword_ui_change && !just_deleted_text &&
+           MaybeAcceptKeywordBySpace(old_user_text, user_text_));
 }
 
 void AutocompleteEditModel::PopupBoundsChangedTo(const gfx::Rect& bounds) {
   controller_->OnPopupBoundsChanged(bounds);
 }
 
 void AutocompleteEditModel::OnPopupClosed() {
   // Accepts the temporary text as the user text, because it makes little
   // sense to have temporary text when the popup is closed.
   InternalSetUserText(UserTextFromDisplayText(view_->GetText()));
@@ skipping 93 matching lines @@
   GURL parsed_url;
   const AutocompleteInput::Type type = AutocompleteInput::Parse(
       UserTextFromDisplayText(text), string16(), NULL, NULL, &parsed_url);
   if (type != AutocompleteInput::URL)
     return false;
 
   *url = parsed_url;
   return true;
 }
 
+void AutocompleteEditModel::RevertTemporaryText(bool revert_popup) {
+  // The user typed something, then selected a different item.  Restore the
+  // text they typed and change back to the default item.
+  // NOTE: This purposefully does not reset paste_state_.
+  just_deleted_text_ = false;
+  has_temporary_text_ = false;
+  if (revert_popup)
+    popup_->ResetToDefaultMatch();
+  view_->OnRevertTemporaryText();
+}
+
 bool AutocompleteEditModel::MaybeAcceptKeywordBySpace(
     const string16& old_user_text,
     const string16& new_user_text) {
   return (paste_state_ == NONE) && is_keyword_hint_ && !keyword_.empty() &&
       inline_autocomplete_text_.empty() && new_user_text.length() >= 2 &&
       IsSpaceCharForAcceptingKeyword(*new_user_text.rbegin()) &&
       !IsWhitespace(*(new_user_text.rbegin() + 1)) &&
       (old_user_text.length() + 1 >= new_user_text.length()) &&
       !new_user_text.compare(0, new_user_text.length() - 1, old_user_text,
                              0, new_user_text.length() - 1) &&
       AcceptKeyword();
 }
 
 //  static
 bool AutocompleteEditModel::IsSpaceCharForAcceptingKeyword(wchar_t c) {
   switch (c) {
     case 0x0020:  // Space
     case 0x3000:  // Ideographic Space
       return true;
     default:
       return false;
   }
 }