Linux: Prevent omnibox autocomplete from stealing the primary X selection.

This is mostly accomplished by decoupling autocomplete (and the
auto-select-all when first clicking in the omnibox to focus it) from GTK's
clipboard code.  Before we update the selection marks, I unregister the
clipboard and block the signal from reaching my handler.  Afterwards, I
restore things.

This creates the possibly-odd effect that text can be highlighted both in
the omnibox and in Webkit, assuming that the omnibox text isn't actually
the primary selection.  I think that this is reasonable, but let me know if
you can think of a better way that it should be handled.

To test, I confirmed that all of the cases listed in
http://codereview.chromium.org/151006's description still work, along with
the following new ones:

1. Highlight text in an xterm to make it the primary selection.  Start
   typing an autocomplete-able URL into Chrome's omnibox.  Middle-click in
   the xterm and check that the xterm's text, rather than the autocompleted
   text from the omnibox, is pasted.

2. Now switch to a different tab and middle-click in the xterm again.  The
   xterm's text should still be pasted.

3. Switch to the original tab and check that the xterm's text is still the
   primary selection.

4. Highlight text in an xterm.  Click in Webkit to make sure
   that the omnibox doesn't have the focus.  Left-click in the omnibox.
   Its text should be highlighted but not made the primary selection
   (middle-clicking in the xterm should still paste the xterm's text).

   EDIT: I've changed this behavior -- clicking in the omnibox to focus it
   now sets its text as the primary selection.

5. Now triple-left-click in the omnibox to highlight all of its text.  This
   time, the URL should become the primary selection.

   EDIT: This is no longer relevant.

I noticed the following annoying behavior, but it's also present in the
official build without this change, so I don't think it's a regression
(there's probably something going on in GTK that I don't understand):

Highlight some text in the omnibox to make it the primary selection.
Now double-left-click to highlight a word in an xterm.  The word flashes in
the xterm, but Chrome automatically grabs the selection again.
Double-clicking in the xterm again lets it hang on to the selection.

Patch by Dan Erat <derat [at] google>
original review url: <http://codereview.chromium.org/159185>

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

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

2 files changed, 75 insertions, 31 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
index 509eaa2..b3c4f49 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.cc
@@ -85,7 +85,8 @@ AutocompleteEditViewGtk::AutocompleteEditViewGtk(
       command_updater_(command_updater),
       popup_window_mode_(false),  // TODO(deanm)
       scheme_security_level_(ToolbarModel::NORMAL),
-      selection_saved_(false) {
+      selection_saved_(false),
+      mark_set_handler_id_(0) {
   model_->set_popup_model(popup_view_->GetModel());
 }
 
@@ -177,8 +178,8 @@ void AutocompleteEditViewGtk::Init() {
                    G_CALLBACK(&HandleViewSizeRequestThunk), this);
   g_signal_connect(text_view_, "populate-popup",
                    G_CALLBACK(&HandlePopulatePopupThunk), this);
-  g_signal_connect(text_buffer_, "mark-set",
-                   G_CALLBACK(&HandleMarkSetThunk), this);
+  mark_set_handler_id_ = g_signal_connect(
+      text_buffer_, "mark-set", G_CALLBACK(&HandleMarkSetThunk), this);
 }
 
 void AutocompleteEditViewGtk::SetFocus() {
@@ -303,8 +304,9 @@ void AutocompleteEditViewGtk::SetForcedQuery() {
     GtkTextIter start, end;
     gtk_text_buffer_get_bounds(text_buffer_, &start, &end);
     gtk_text_buffer_get_iter_at_offset(text_buffer_, &start, 1);
-    gtk_text_buffer_place_cursor(text_buffer_, &start);
+    StartUpdatingHighlightedText();
     gtk_text_buffer_select_range(text_buffer_, &start, &end);
+    FinishUpdatingHighlightedText();
   }
 }
 
@@ -314,14 +316,12 @@ bool AutocompleteEditViewGtk::IsSelectAll() {
 }
 
 void AutocompleteEditViewGtk::SelectAll(bool reversed) {
-  GtkTextIter start, end;
-  if (reversed) {
-    gtk_text_buffer_get_bounds(text_buffer_, &end, &start);
-  } else {
-    gtk_text_buffer_get_bounds(text_buffer_, &start, &end);
-  }
-  gtk_text_buffer_place_cursor(text_buffer_, &start);
-  gtk_text_buffer_select_range(text_buffer_, &start, &end);
+  // SelectAll() is invoked as a side effect of other actions (e.g.  switching
+  // tabs or hitting Escape) in autocomplete_edit.cc, so we don't update the
+  // PRIMARY selection here.
+  // TODO(derat): But this is also called by LocationBarView::FocusLocation() --
+  // should the X selection be updated when the user hits Ctrl-L?
+  SelectAllInternal(reversed, false);
 }
 
 void AutocompleteEditViewGtk::RevertAll() {
@@ -359,17 +359,7 @@ bool AutocompleteEditViewGtk::OnInlineAutocompleteTextMaybeChanged(
   if (display_text == GetText())
     return false;
 
-  // We need to get the clipboard while it's attached to the toplevel.  The
-  // easiest thing to do is just to lazily pull the clipboard here.
-  GtkClipboard* clipboard =
-      gtk_widget_get_clipboard(text_view_, GDK_SELECTION_PRIMARY);
-  DCHECK(clipboard);
-  if (!clipboard)
-    return true;
-
-  // Remove the PRIMARY clipboard to avoid having "clipboard helpers" like
-  // klipper and glipper race with / remove our inline autocomplete selection.
-  gtk_text_buffer_remove_selection_clipboard(text_buffer_, clipboard);
+  StartUpdatingHighlightedText();
   SetWindowTextAndCaretPos(display_text, 0);
 
   // Select the part of the text that was inline autocompleted.
@@ -378,10 +368,8 @@ bool AutocompleteEditViewGtk::OnInlineAutocompleteTextMaybeChanged(
   gtk_text_buffer_get_iter_at_offset(text_buffer_, &insert, user_text_length);
   gtk_text_buffer_select_range(text_buffer_, &insert, &bound);
 
+  FinishUpdatingHighlightedText();
   TextChanged();
-  // Put the PRIMARY clipboard back, so that selection still somewhat works.
-  gtk_text_buffer_add_selection_clipboard(text_buffer_, clipboard);
-
   return true;
 }
 
@@ -531,8 +519,9 @@ gboolean AutocompleteEditViewGtk::HandleViewButtonPress(GdkEventButton* event) {
   GtkWidgetClass* klass = GTK_WIDGET_GET_CLASS(text_view_);
   klass->button_press_event(text_view_, event);
 
-  // Select the full input when we get focus.
-  SelectAll(false);
+  // Select the full input and update the PRIMARY selection when we get focus.
+  SelectAllInternal(false, true);
+
   // So we told the buffer where the cursor should be, but make sure to tell
   // the view so it can scroll it to be visible if needed.
   // NOTE: This function doesn't seem to like a count of 0, looking at the
@@ -570,7 +559,7 @@ void AutocompleteEditViewGtk::HandleViewMoveCursor(
   else if (step != GTK_MOVEMENT_DISPLAY_LINES)
     return;  // Propagate into GtkTextView
   model_->OnUpOrDownKeyPressed(move_amount);
-  // move-cursor doesn't use a signal accumulaqtor on the return value (it
+  // move-cursor doesn't use a signal accumulator on the return value (it
   // just ignores then), so we have to stop the propagation.
   g_signal_stop_emission_by_name(text_view_, "move-cursor");
 }
@@ -660,6 +649,43 @@ void AutocompleteEditViewGtk::HandleMarkSet(GtkTextBuffer* buffer,
   }
 }
 
+void AutocompleteEditViewGtk::SelectAllInternal(bool reversed,
+                                                bool update_primary_selection) {
+  GtkTextIter start, end;
+  if (reversed) {
+    gtk_text_buffer_get_bounds(text_buffer_, &end, &start);
+  } else {
+    gtk_text_buffer_get_bounds(text_buffer_, &start, &end);
+  }
+  if (!update_primary_selection)
+    StartUpdatingHighlightedText();
+  gtk_text_buffer_select_range(text_buffer_, &start, &end);
+  if (!update_primary_selection)
+    FinishUpdatingHighlightedText();
+}
+
+void AutocompleteEditViewGtk::StartUpdatingHighlightedText() {
+  if (GTK_WIDGET_REALIZED(text_view_)) {
+    GtkClipboard* clipboard =
+        gtk_widget_get_clipboard(text_view_, GDK_SELECTION_PRIMARY);
+    DCHECK(clipboard);
+    if (clipboard)
+      gtk_text_buffer_remove_selection_clipboard(text_buffer_, clipboard);
+  }
+  g_signal_handler_block(text_buffer_, mark_set_handler_id_);
+}
+
+void AutocompleteEditViewGtk::FinishUpdatingHighlightedText() {
+  if (GTK_WIDGET_REALIZED(text_view_)) {
+    GtkClipboard* clipboard =
+        gtk_widget_get_clipboard(text_view_, GDK_SELECTION_PRIMARY);
+    DCHECK(clipboard);
+    if (clipboard)
+      gtk_text_buffer_add_selection_clipboard(text_buffer_, clipboard);
+  }
+  g_signal_handler_unblock(text_buffer_, mark_set_handler_id_);
+}
+
 AutocompleteEditViewGtk::CharRange AutocompleteEditViewGtk::GetSelection() {
   // You can not just use get_selection_bounds here, since the order will be
   // ascending, and you don't know where the user's start and end of the
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
index 97708d0..e0370ee 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_gtk.h
@@ -212,6 +212,21 @@ class AutocompleteEditViewGtk : public AutocompleteEditView {
                      GtkTextIter* location,
                      GtkTextMark* mark);
 
+  // Actual implementation of SelectAll(), but also provides control over
+  // whether the PRIMARY selection is set to the selected text (in SelectAll(),
+  // it isn't, but we want set the selection when the user clicks in the entry).
+  void SelectAllInternal(bool reversed, bool update_primary_selection);
+
+  // Get ready to update |text_buffer_|'s highlighting without making changes to
+  // the PRIMARY selection.  Removes the clipboard from |text_buffer_| and
+  // blocks the "mark-set" signal handler.
+  void StartUpdatingHighlightedText();
+
+  // Finish updating |text_buffer_|'s highlighting such that future changes will
+  // automatically update the PRIMARY selection.  Undoes
+  // StartUpdatingHighlightedText()'s changes.
+  void FinishUpdatingHighlightedText();
+
   // Get the character indices of the current selection.  This honors
   // direction, cp_max is the insertion point, and cp_min is the bound.
   CharRange GetSelection();
@@ -230,7 +245,7 @@ class AutocompleteEditViewGtk : public AutocompleteEditView {
   // Internally invoked whenever the text changes in some way.
   void TextChanged();
 
-  // Save 'selected_text' as the PRIMARY X selection.
+  // Save |selected_text| as the PRIMARY X selection.
   void SavePrimarySelection(const std::string& selected_text);
 
   // The widget we expose, used for vertically centering the real text edit,
@@ -274,10 +289,13 @@ class AutocompleteEditViewGtk : public AutocompleteEditView {
   // it, we pass this string to SavePrimarySelection()).
   std::string selected_text_;
 
-  // Has the current value of 'selected_text_' been saved as the PRIMARY
+  // Has the current value of |selected_text_| been saved as the PRIMARY
   // selection?
   bool selection_saved_;
 
+  // ID of the signal handler for "mark-set" on |text_buffer_|.
+  gulong mark_set_handler_id_;
+
   DISALLOW_COPY_AND_ASSIGN(AutocompleteEditViewGtk);
 };