Fix issue 20934: Omnibox keyboard behavior wrong for "See recent pages in his...

chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc

 // Copyright (c) 2009 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_view_gtk.h"
 
 #include <gtk/gtk.h>
 #include <gdk/gdkkeysyms.h>
 
 #include <algorithm>
@@ skipping 88 matching lines @@
       command_updater_(command_updater),
       popup_window_mode_(popup_window_mode),
       scheme_security_level_(ToolbarModel::NORMAL),
       mark_set_handler_id_(0),
       button_1_pressed_(false),
       text_selected_during_click_(false),
       text_view_focused_before_button_press_(false),
 #if !defined(TOOLKIT_VIEWS)
       theme_provider_(GtkThemeProvider::GetFrom(profile)),
 #endif
+      enter_was_pressed_(false),
       tab_was_pressed_(false),
       paste_clipboard_requested_(false) {
   model_->SetPopupModel(popup_view_->GetModel());
 }
 
 AutocompleteEditViewGtk::~AutocompleteEditViewGtk() {
   NotificationService::current()->Notify(
       NotificationType::AUTOCOMPLETE_EDIT_DESTROYED,
       Source<AutocompleteEditViewGtk>(this),
       NotificationService::NoDetails());
@@ skipping 336 matching lines @@
       model_->on_paste_replacing_all();
   }
 
   // Record our state.
   text_before_change_ = GetText();
   sel_before_change_ = GetSelection();
 }
 
 // TODO(deanm): This is mostly stolen from Windows, and will need some work.
 bool AutocompleteEditViewGtk::OnAfterPossibleChange() {
+  // If the change is caused by an Enter key press event, and the event was not
+  // handled by IME, then it's an unexpected change and shall be reverted here.
+  // {Start|Finish}UpdatingHighlightedText() are called here to prevent the
+  // PRIMARY selection from being changed.
+  if (enter_was_pressed_ &&
+      (char_inserted_ == '\n' || char_inserted_ == '\r')) {
+    StartUpdatingHighlightedText();
+    SetTextAndSelectedRange(text_before_change_, sel_before_change_);
+    FinishUpdatingHighlightedText();
+    return false;
+  }
+
   CharRange new_sel = GetSelection();
   int length = GetTextLength();
   bool selection_differs = (new_sel.cp_min != sel_before_change_.cp_min) ||
                            (new_sel.cp_max != sel_before_change_.cp_max);
   bool at_end_of_edit = (new_sel.cp_min == length && new_sel.cp_max == length);
 
   // See if the text or selection have changed since OnBeforePossibleChange().
   std::wstring new_text(GetText());
   bool text_differs = (new_text != text_before_change_);
 
@@ skipping 117 matching lines @@
   // - If there is only one character in inserted text, save it in
   //   char_inserted_.
   // - Filter out all new line and tab characters.
   //
   // So if |char_inserted_| equals '\n' after calling |text_view_|'s
   // default signal handler against an Enter key press event, then we know that
   // the Enter key press event was handled by GtkTextView rather than IME, and
   // can perform the special behavior for Enter key safely.
   //
   // Now the last thing is to prevent the content of omnibox from being changed
-  // by GtkTextView when Tab or Enter key is pressed. Because we can't prevent
-  // it, we use a backup and restore trick: If Enter is pressed, backup the
-  // content of omnibox before sending the key event to |text_view_|, and
-  // then restore it afterwards if IME did not handle the event.
+  // by GtkTextView when Enter key is pressed. As OnBeforePossibleChange() and
+  // OnAfterPossibleChange() will be called by GtkTextView before and after
+  // changing the content, and the content is already saved in
+  // OnBeforePossibleChange(), so if the Enter key press event was not handled
+  // by IME, it's easy to restore the content in OnAfterPossibleChange(), as if
+  // it's not changed at all.
 
   GtkWidgetClass* klass = GTK_WIDGET_GET_CLASS(widget);
 
-  bool enter_pressed = (event->keyval == GDK_Return ||
+  enter_was_pressed_ = (event->keyval == GDK_Return ||
                         event->keyval == GDK_ISO_Enter ||
                         event->keyval == GDK_KP_Enter);
 
-  gchar* original_text = NULL;
-
-  // Enter key will have special behavior if it's not handled by IME.
-  // We need save the original content of |text_buffer_| and restore it when
-  // necessary, because GtkTextView might alter the content.
-  if (enter_pressed) {
-    GtkTextIter start, end;
-    gtk_text_buffer_get_bounds(text_buffer_, &start, &end);
-    original_text = gtk_text_buffer_get_text(text_buffer_, &start, &end, FALSE);
-    // Reset |char_inserted_|, which may be set in the "insert-text" signal
-    // handler, so that we'll know if an Enter key event was handled by IME.
-    char_inserted_ = 0;
-  }
-
   // Set |tab_was_pressed_| to true if it's a Tab key press event, so that our
   // handler of "move-focus" signal can trigger Tab to search behavior when
   // necessary.
   tab_was_pressed_ = ((event->keyval == GDK_Tab ||
                        event->keyval == GDK_ISO_Left_Tab ||
                        event->keyval == GDK_KP_Tab) &&
                       !(event->state & GDK_CONTROL_MASK));
 
+  // Reset |char_inserted_|, which may be set in the "insert-text" signal
+  // handler, so that we'll know if an Enter key event was handled by IME.
+  char_inserted_ = 0;
+
   // Reset |paste_clipboard_requested_| to make sure we won't misinterpret this
   // key input action as a paste action.
   paste_clipboard_requested_ = false;
 
   // Call the default handler, so that IME can work as normal.
   // New line characters will be filtered out by our "insert-text"
   // signal handler attached to |text_buffer_| object.
   gboolean result = klass->key_press_event(widget, event);
 
   // Set |tab_was_pressed_| to false, to make sure Tab to search behavior can
   // only be triggered by pressing Tab key.
   tab_was_pressed_ = false;
 
-  if (enter_pressed && (char_inserted_ == '\n' || char_inserted_ == '\r')) {
+  if (enter_was_pressed_ &&
+      (char_inserted_ == '\n' || char_inserted_ == '\r')) {
     bool alt_held = (event->state & GDK_MOD1_MASK);
-    // Revert the original text in case the text has been changed.
-    // Call gtk_text_buffer_{begin|end}_user_action() to make sure |model_| will
-    // be updated correctly.
-    // Note: SetUserText() does not work here, it'll reset the keyword state.
-    DCHECK(original_text);
-    gtk_text_buffer_begin_user_action(text_buffer_);
-    gtk_text_buffer_set_text(text_buffer_, original_text, -1);
-    gtk_text_buffer_end_user_action(text_buffer_);
     model_->AcceptInput(alt_held ? NEW_FOREGROUND_TAB : CURRENT_TAB, false);
     result = TRUE;
   } else if (!result && event->keyval == GDK_Escape &&
              (event->state & gtk_accelerator_get_default_mod_mask()) == 0) {
     // We can handle the Escape key if |text_view_| did not handle it.
     // If it's not handled by us, then we need to propagate it up to the parent
     // widgets, so that Escape accelerator can still work.
     result = model_->OnEscapeKeyPressed();
   } else if (event->keyval == GDK_Control_L || event->keyval == GDK_Control_R) {
     // Omnibox2 can switch its contents while pressing a control key. To switch
     // the contents of omnibox2, we notify the AutocompleteEditModel class when
     // the control-key state is changed.
     model_->OnControlKeyChanged(true);
   }
 
-  if (original_text)
-    g_free(original_text);
+  // Set |enter_was_pressed_| to false, to make sure OnAfterPossibleChange() can
+  // act as normal for changes made by other events.
+  enter_was_pressed_ = false;
 
   // If the key event is not handled by |text_view_| or us, then we need to
   // propagate the key event up to parent widgets by returning FALSE.
   // In this case we need to stop the signal emission explicitly to prevent the
   // default "key-press-event" handler of |text_view_| from being called again.
   if (!result) {
     static guint signal_id =
         g_signal_lookup("key-press-event", GTK_TYPE_WIDGET);
     g_signal_stop_emission(widget, signal_id, 0);
   }
@@ skipping 484 matching lines @@
     const std::string& selected_text) {
   GtkClipboard* clipboard =
       gtk_widget_get_clipboard(text_view_, GDK_SELECTION_PRIMARY);
   DCHECK(clipboard);
   if (!clipboard)
     return;
 
   gtk_clipboard_set_text(
       clipboard, selected_text.data(), selected_text.size());
 }