Fix text and selection's save/restore issue of omnibox when displaying temporary text.

This CL fixes issue 21362: The original text and selection can't be reverted correctly when pressing escape key in omnibox if currently selected line is not the default match.

BUG=21362: The original text and selection can't be reverted correctly when pressing escape key in omnibox if currently selected line is not the default match.
TEST=Input something in omnibox and make sure inline autocomplete is triggered, then press down to select another line, then press escape to revert to the default match and see of the original text and selection was reverted correctly.

Review URL: http://codereview.chromium.org/194056

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

6 files changed, 44 insertions, 39 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit.cc b/chrome/browser/autocomplete/autocomplete_edit.cc
index 9a23bffe..ecde897 100644
--- a/chrome/browser/autocomplete/autocomplete_edit.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit.cc
@@ -456,6 +456,11 @@ void AutocompleteEditModel::OnPopupDataChanged(
     return;
   }
 
+  // TODO(suzhe): Instead of messing with |inline_autocomplete_text_| here,
+  // we should probably do it inside Observe(), and save/restore it around
+  // changes to the temporary text.  This will let us remove knowledge of
+  // inline autocompletions from the popup code.
+  //
   // Handle changes to inline autocomplete text.  Don't make changes if the user
   // is showing temporary text.  Making display changes would be obviously
   // wrong; making changes to the inline_autocomplete_text_ itself turns out to
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
index 708118e..2633037 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
@@ -338,26 +338,13 @@ void AutocompleteEditViewGtk::SetWindowTextAndCaretPos(const std::wstring& text,
   SetTextAndSelectedRange(text, range);
 }
 
-void AutocompleteEditViewGtk::SetTextAndSelectedRange(const std::wstring& text,
-                                                      const CharRange& range) {
-  std::string utf8 = WideToUTF8(text);
-  gtk_text_buffer_set_text(text_buffer_, utf8.data(), utf8.length());
-
-  GtkTextIter insert, bound;
-  ItersFromCharRange(range, &insert, &bound);
-  gtk_text_buffer_select_range(text_buffer_, &insert, &bound);
-}
-
 void AutocompleteEditViewGtk::SetForcedQuery() {
   const std::wstring current_text(GetText());
   if (current_text.empty() || (current_text[0] != '?')) {
     SetUserText(L"?");
   } else {
-    GtkTextIter start, end;
-    gtk_text_buffer_get_bounds(text_buffer_, &start, &end);
-    gtk_text_buffer_get_iter_at_offset(text_buffer_, &start, 1);
     StartUpdatingHighlightedText();
-    gtk_text_buffer_select_range(text_buffer_, &start, &end);
+    SetSelectedRange(CharRange(current_text.size(), 1));
     FinishUpdatingHighlightedText();
   }
 }
@@ -407,10 +394,8 @@ void AutocompleteEditViewGtk::ClosePopup() {
 void AutocompleteEditViewGtk::OnTemporaryTextMaybeChanged(
     const std::wstring& display_text,
     bool save_original_selection) {
-  if (save_original_selection) {
+  if (save_original_selection)
     saved_temporary_selection_ = GetSelection();
-    saved_temporary_text_ = GetText();
-  }
 
   StartUpdatingHighlightedText();
   SetWindowTextAndCaretPos(display_text, display_text.length());
@@ -425,14 +410,8 @@ bool AutocompleteEditViewGtk::OnInlineAutocompleteTextMaybeChanged(
     return false;
 
   StartUpdatingHighlightedText();
-  SetWindowTextAndCaretPos(display_text, 0);
-
-  // Select the part of the text that was inline autocompleted.
-  GtkTextIter bound, insert;
-  gtk_text_buffer_get_bounds(text_buffer_, &insert, &bound);
-  gtk_text_buffer_get_iter_at_offset(text_buffer_, &insert, user_text_length);
-  gtk_text_buffer_select_range(text_buffer_, &insert, &bound);
-
+  CharRange range(display_text.size(), user_text_length);
+  SetTextAndSelectedRange(display_text, range);
   FinishUpdatingHighlightedText();
   TextChanged();
   return true;
@@ -440,9 +419,8 @@ bool AutocompleteEditViewGtk::OnInlineAutocompleteTextMaybeChanged(
 
 void AutocompleteEditViewGtk::OnRevertTemporaryText() {
   StartUpdatingHighlightedText();
-  SetTextAndSelectedRange(saved_temporary_text_, saved_temporary_selection_);
+  SetSelectedRange(saved_temporary_selection_);
   FinishUpdatingHighlightedText();
-  saved_temporary_text_.clear();
   TextChanged();
 }
 
@@ -1175,3 +1153,16 @@ void AutocompleteEditViewGtk::SavePrimarySelection(
   gtk_clipboard_set_text(
       clipboard, selected_text.data(), selected_text.size());
 }
+
+void AutocompleteEditViewGtk::SetTextAndSelectedRange(const std::wstring& text,
+                                                      const CharRange& range) {
+  std::string utf8 = WideToUTF8(text);
+  gtk_text_buffer_set_text(text_buffer_, utf8.data(), utf8.length());
+  SetSelectedRange(range);
+}
+
+void AutocompleteEditViewGtk::SetSelectedRange(const CharRange& range) {
+  GtkTextIter insert, bound;
+  ItersFromCharRange(range, &bound, &insert);
+  gtk_text_buffer_select_range(text_buffer_, &insert, &bound);
+}
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
index 5ec6d43..112cdef 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
@@ -317,6 +317,9 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
   void SetTextAndSelectedRange(const std::wstring& text,
                                const CharRange& range);
 
+  // Set the selection to |range|.
+  void SetSelectedRange(const CharRange& range);
+
   // The widget we expose, used for vertically centering the real text edit,
   // since the height will change based on the font / font size, etc.
   OwnedWidgetGtk alignment_;
@@ -346,9 +349,8 @@ class AutocompleteEditViewGtk : public AutocompleteEditView,
 
   ToolbarModel::SecurityLevel scheme_security_level_;
 
-  // Text and selection at the point where the user started using the
+  // Selection at the point where the user started using the
   // arrows to move around in the popup.
-  std::wstring saved_temporary_text_;
   CharRange saved_temporary_selection_;
 
   // Tracking state before and after a possible change.
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
index dd71e19..f6179fb 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
@@ -177,10 +177,9 @@ class AutocompleteEditViewMac : public AutocompleteEditView {
   // Objective-C object to bridge field_ delegate calls to C++.
   scoped_nsobject<AutocompleteEditHelper> edit_helper_;
 
-  // Text and selection at the point where the user started using the
+  // Selection at the point where the user started using the
   // arrows to move around in the popup.
   NSRange saved_temporary_selection_;
-  std::wstring saved_temporary_text_;
 
   // Tracking state before and after a possible change for reporting
   // to model_.
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
index 94a9af6..dc70cc1 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
@@ -451,10 +451,8 @@ void AutocompleteEditViewMac::OnTemporaryTextMaybeChanged(
   // TODO(shess): I believe this is for when the user arrows around
   // the popup, will be restored if they hit escape.  Figure out if
   // that is for certain it.
-  if (save_original_selection) {
+  if (save_original_selection)
     saved_temporary_selection_ = GetSelectedRange();
-    saved_temporary_text_ = GetText();
-  }
 
   SetWindowTextAndCaretPos(display_text, display_text.size());
   controller_->OnChanged();
@@ -478,8 +476,7 @@ bool AutocompleteEditViewMac::OnInlineAutocompleteTextMaybeChanged(
 }
 
 void AutocompleteEditViewMac::OnRevertTemporaryText() {
-  SetTextAndSelectedRange(saved_temporary_text_, saved_temporary_selection_);
-  saved_temporary_text_.clear();
+  SetSelectedRange(saved_temporary_selection_);
 }
 
 bool AutocompleteEditViewMac::IsFirstResponder() const {
diff --git a/chrome/browser/autocomplete/autocomplete_popup_model.cc b/chrome/browser/autocomplete/autocomplete_popup_model.cc
index bce6ce9..8b0c9d1 100644
--- a/chrome/browser/autocomplete/autocomplete_popup_model.cc
+++ b/chrome/browser/autocomplete/autocomplete_popup_model.cc
@@ -109,9 +109,20 @@ void AutocompletePopupModel::SetSelectedLine(size_t line,
   // eliminated and just become a call to the observer on the edit.
   std::wstring keyword;
   const bool is_keyword_hint = GetKeywordForMatch(match, &keyword);
-  edit_model_->OnPopupDataChanged(
-      reset_to_default ? std::wstring() : match.fill_into_edit,
-      !reset_to_default, keyword, is_keyword_hint, match.type);
+
+  if (reset_to_default) {
+    std::wstring inline_autocomplete_text;
+    if ((match.inline_autocomplete_offset != std::wstring::npos) &&
+        (match.inline_autocomplete_offset < match.fill_into_edit.length())) {
+      inline_autocomplete_text =
+          match.fill_into_edit.substr(match.inline_autocomplete_offset);
+    }
+    edit_model_->OnPopupDataChanged(inline_autocomplete_text, false,
+                                    keyword, is_keyword_hint, match.type);
+  } else {
+    edit_model_->OnPopupDataChanged(match.fill_into_edit, true,
+                                    keyword, is_keyword_hint, match.type);
+  }
 
   // Repaint old and new selected lines immediately, so that the edit doesn't
   // appear to update [much] faster than the popup.  We must not update