gtk: Use a GtkInvisible to track the tab dragging operation. Originally we were using the actual tab widget for this purpose, but this is unreliable becauase we hide and detach the tab widget, which disturbs the drag operation.

BUG=19725,20513
TEST=Extensive tab dragging.
Review URL: http://codereview.chromium.org/176034

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

3 files changed, 114 insertions, 81 deletions

diff --git a/chrome/browser/gtk/tabs/tab_gtk.cc b/chrome/browser/gtk/tabs/tab_gtk.cc
index 91ef22e..0909d74 100644
--- a/chrome/browser/gtk/tabs/tab_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_gtk.cc
@@ -118,35 +118,25 @@ TabGtk::TabGtk(TabDelegate* delegate)
     : TabRendererGtk(delegate->GetThemeProvider()),
       delegate_(delegate),
       closing_(false),
-      dragging_(false),
-      title_width_(0) {
+      last_mouse_down_(NULL),
+      drag_widget_(NULL),
+      title_width_(0),
+      ALLOW_THIS_IN_INITIALIZER_LIST(destroy_factory_(this)) {
   event_box_ = gtk_event_box_new();
   gtk_event_box_set_visible_window(GTK_EVENT_BOX(event_box_), FALSE);
-  gtk_drag_source_set(event_box_, GDK_BUTTON1_MASK,
-                      NULL, 0, GDK_ACTION_MOVE);
-  GtkDndUtil::SetSourceTargetListFromCodeMask(event_box_,
-                                              GtkDndUtil::CHROME_TAB);
   g_signal_connect(G_OBJECT(event_box_), "button-press-event",
-                   G_CALLBACK(OnMousePress), this);
+                   G_CALLBACK(OnButtonPressEvent), this);
   g_signal_connect(G_OBJECT(event_box_), "button-release-event",
-                   G_CALLBACK(OnMouseRelease), this);
+                   G_CALLBACK(OnButtonReleaseEvent), this);
   g_signal_connect(G_OBJECT(event_box_), "enter-notify-event",
                    G_CALLBACK(OnEnterNotifyEvent), this);
   g_signal_connect(G_OBJECT(event_box_), "leave-notify-event",
                    G_CALLBACK(OnLeaveNotifyEvent), this);
-  g_signal_connect_after(G_OBJECT(event_box_), "drag-begin",
-                           G_CALLBACK(OnDragBegin), this);
-  g_signal_connect_after(G_OBJECT(event_box_), "drag-end",
-                         G_CALLBACK(OnDragEnd), this);
-  g_signal_connect_after(G_OBJECT(event_box_), "drag-failed",
-                           G_CALLBACK(OnDragFailed), this);
   gtk_widget_add_events(event_box_,
         GDK_BUTTON_PRESS_MASK | GDK_BUTTON_RELEASE_MASK |
         GDK_LEAVE_NOTIFY_MASK | GDK_LEAVE_NOTIFY_MASK);
   gtk_container_add(GTK_CONTAINER(event_box_), TabRendererGtk::widget());
   gtk_widget_show_all(event_box_);
-
-  SetEmptyDragIcon(event_box_);
 }
 
 TabGtk::~TabGtk() {
@@ -160,9 +150,11 @@ TabGtk::~TabGtk() {
 }
 
 // static
-gboolean TabGtk::OnMousePress(GtkWidget* widget, GdkEventButton* event,
-                              TabGtk* tab) {
-  if (event->button == 1) {
+gboolean TabGtk::OnButtonPressEvent(GtkWidget* widget, GdkEventButton* event,
+                                    TabGtk* tab) {
+  // Every button press ensures either a button-release-event or a drag-fail
+  // signal for |widget|.
+  if (event->button == 1 && event->type == GDK_BUTTON_PRESS) {
     // Store whether or not we were selected just now... we only want to be
     // able to drag foreground tabs, so we don't start dragging the tab if
     // it was in the background.
@@ -170,6 +162,12 @@ gboolean TabGtk::OnMousePress(GtkWidget* widget, GdkEventButton* event,
     if (just_selected) {
       tab->delegate_->SelectTab(tab);
     }
+
+    // Hook into the message loop to handle dragging.
+    MessageLoopForUI::current()->AddObserver(tab);
+
+    // Store the button press event, used to initiate a drag.
+    tab->last_mouse_down_ = gdk_event_copy(reinterpret_cast<GdkEvent*>(event));
   } else if (event->button == 3) {
     tab->ShowContextMenu();
   }
@@ -178,8 +176,17 @@ gboolean TabGtk::OnMousePress(GtkWidget* widget, GdkEventButton* event,
 }
 
 // static
-gboolean TabGtk::OnMouseRelease(GtkWidget* widget, GdkEventButton* event,
-                                TabGtk* tab) {
+gboolean TabGtk::OnButtonReleaseEvent(GtkWidget* widget, GdkEventButton* event,
+                                      TabGtk* tab) {
+  if (event->button == 1) {
+    MessageLoopForUI::current()->RemoveObserver(tab);
+
+    if (tab->last_mouse_down_) {
+      gdk_event_free(tab->last_mouse_down_);
+      tab->last_mouse_down_ = NULL;
+    }
+  }
+
   // Middle mouse up means close the tab, but only if the mouse is over it
   // (like a button).
   if (event->button == 2 &&
@@ -193,29 +200,19 @@ gboolean TabGtk::OnMouseRelease(GtkWidget* widget, GdkEventButton* event,
 }
 
 // static
-void TabGtk::OnDragBegin(GtkWidget* widget, GdkDragContext* context,
-                         TabGtk* tab) {
-  MessageLoopForUI::current()->AddObserver(tab);
-
-  int x, y;
-  gdk_window_get_pointer(tab->event_box_->window, &x, &y, NULL);
-
-  // Make the mouse coordinate relative to the tab.
-  x -= tab->bounds().x();
-  y -= tab->bounds().y();
-
-  tab->dragging_ = true;
-  tab->delegate_->MaybeStartDrag(tab, gfx::Point(x, y));
-}
-
-// static
 void TabGtk::OnDragEnd(GtkWidget* widget, GdkDragContext* context,
                        TabGtk* tab) {
-  // Release our grab on the pointer.
-  gdk_pointer_ungrab(GDK_CURRENT_TIME);
-  gtk_grab_remove(tab->widget());
+  // We must let gtk clean up after we handle the drag operation, otherwise
+  // there will be outstanding references to the drag widget when we try to
+  // destroy it.
+  MessageLoop::current()->PostTask(FROM_HERE,
+      tab->destroy_factory_.NewRunnableMethod(&TabGtk::DestroyDragWidget));
+
+  if (tab->last_mouse_down_) {
+    gdk_event_free(tab->last_mouse_down_);
+    tab->last_mouse_down_ = NULL;
+  }
 
-  tab->dragging_ = false;
   // Notify the drag helper that we're done with any potential drag operations.
   // Clean up the drag helper, which is re-created on the next mouse press.
   tab->delegate_->EndDrag(false);
@@ -242,25 +239,21 @@ void TabGtk::WillProcessEvent(GdkEvent* event) {
 }
 
 void TabGtk::DidProcessEvent(GdkEvent* event) {
-  switch (event->type) {
-    case GDK_MOTION_NOTIFY:
-      delegate_->ContinueDrag(NULL);
-      break;
-    case GDK_GRAB_BROKEN:
-      // If the user drags the mouse away from the dragged tab before the widget
-      // is created, gtk loses the grab used for the drag and we're stuck in a
-      // limbo where the drag is still active, but we don't get any
-      // motion-notify-event signals.  Adding the grab back doesn't keep the
-      // drag alive, but it does get us out of this bind by finishing the drag.
-      if (delegate_->IsTabDetached(this)) {
-        gdk_pointer_grab(widget()->window, FALSE, GDK_POINTER_MOTION_HINT_MASK,
-                         NULL, NULL, GDK_CURRENT_TIME);
-        gtk_grab_add(widget());
-      }
-      break;
-    default:
-      break;
+  if (event->type != GDK_MOTION_NOTIFY)
+    return;
+
+  if (drag_widget_) {
+    delegate_->ContinueDrag(NULL);
+    return;
   }
+
+  GdkEventMotion* motion = reinterpret_cast<GdkEventMotion*>(event);
+  GdkEventButton* button = reinterpret_cast<GdkEventButton*>(last_mouse_down_);
+  bool dragging = gtk_drag_check_threshold(widget(),
+                                           button->x, button->y,
+                                           motion->x, motion->y);
+  if (dragging)
+    StartDragging(gfx::Point(button->x, button->y));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -322,3 +315,29 @@ void TabGtk::UpdateTooltipState() {
     gtk_widget_set_has_tooltip(widget(), FALSE);
   }
 }
+
+void TabGtk::CreateDragWidget() {
+  drag_widget_ = gtk_invisible_new();
+  g_signal_connect(drag_widget_, "drag-failed",
+                   G_CALLBACK(OnDragFailed), this);
+  g_signal_connect(drag_widget_, "drag-end", G_CALLBACK(OnDragEnd), this);
+}
+
+void TabGtk::DestroyDragWidget() {
+  if (drag_widget_) {
+    gtk_widget_destroy(drag_widget_);
+    drag_widget_ = NULL;
+  }
+}
+
+void TabGtk::StartDragging(gfx::Point drag_offset) {
+  CreateDragWidget();
+
+  GtkTargetList* list = GtkDndUtil::GetTargetListFromCodeMask(
+      GtkDndUtil::CHROME_TAB);
+  gtk_drag_begin(drag_widget_, list, GDK_ACTION_COPY,
+                 1,  // Drags are always initiated by the left button.
+                 last_mouse_down_);
+
+  delegate_->MaybeStartDrag(this, drag_offset);
+}
diff --git a/chrome/browser/gtk/tabs/tab_gtk.h b/chrome/browser/gtk/tabs/tab_gtk.h
index dd8547b..8772d55 100644
--- a/chrome/browser/gtk/tabs/tab_gtk.h
+++ b/chrome/browser/gtk/tabs/tab_gtk.h
@@ -95,17 +95,21 @@ class TabGtk : public TabRendererGtk,
   virtual void UpdateData(TabContents* contents, bool loading_only);
   virtual void SetBounds(const gfx::Rect& bounds);
 
+ private:
+  class ContextMenuController;
+  friend class ContextMenuController;
+
+  // MessageLoop::Observer implementation:
+  virtual void WillProcessEvent(GdkEvent* event);
+  virtual void DidProcessEvent(GdkEvent* event);
+
   // button-press-event handler that handles mouse clicks.
-  static gboolean OnMousePress(GtkWidget* widget, GdkEventButton* event,
-                               TabGtk* tab);
+  static gboolean OnButtonPressEvent(GtkWidget* widget, GdkEventButton* event,
+                                     TabGtk* tab);
 
   // button-release-event handler that handles mouse click releases.
-  static gboolean OnMouseRelease(GtkWidget* widget, GdkEventButton* event,
-                                 TabGtk* tab);
-
-  // drag-begin handler that signals when a drag action begins.
-  static void OnDragBegin(GtkWidget* widget, GdkDragContext* context,
-                          TabGtk* tab);
+  static gboolean OnButtonReleaseEvent(GtkWidget* widget, GdkEventButton* event,
+                                       TabGtk* tab);
 
   // drag-end handler that signals when a drag action ends.
   static void OnDragEnd(GtkWidget* widget, GdkDragContext* context,
@@ -115,15 +119,6 @@ class TabGtk : public TabRendererGtk,
   static gboolean OnDragFailed(GtkWidget* widget, GdkDragContext* context,
                                GtkDragResult result, TabGtk* tab);
 
- protected:
-  // MessageLoop::Observer implementation:
-  virtual void WillProcessEvent(GdkEvent* event);
-  virtual void DidProcessEvent(GdkEvent* event);
-
- private:
-  class ContextMenuController;
-  friend class ContextMenuController;
-
   // Shows the context menu.
   void ShowContextMenu();
 
@@ -134,6 +129,16 @@ class TabGtk : public TabRendererGtk,
   // the tab.
   void UpdateTooltipState();
 
+  // Creates the drag widget used to track a drag operation.
+  void CreateDragWidget();
+
+  // Destroys the drag widget.
+  void DestroyDragWidget();
+
+  // Starts the dragging operation.  |drag_offset| is the offset inside the tab
+  // bounds where the grab occurred.
+  void StartDragging(gfx::Point drag_offset);
+
   // An instance of a delegate object that can perform various actions based on
   // user gestures.
   TabDelegate* delegate_;
@@ -152,13 +157,21 @@ class TabGtk : public TabRendererGtk,
   // DCHECK.
   GtkWidget* event_box_;
 
-  // True if this tab is being dragged.
-  bool dragging_;
+  // A copy of the last button press event, used to initiate a drag.
+  GdkEvent* last_mouse_down_;
+
+  // A GtkInivisible used to track the drag event.  GtkInvisibles are of the
+  // type GInitiallyUnowned, but the widget initialization code sinks the
+  // reference, so we can't used an OwnedWidgetGtk here.
+  GtkWidget* drag_widget_;
 
   // The cached width of the title in pixels, updated whenever the title
   // changes.
   int title_width_;
 
+  // Used to destroy the drag widget after a return to the message loop.
+  ScopedRunnableMethodFactory<TabGtk> destroy_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(TabGtk);
 };
 
diff --git a/chrome/browser/gtk/tabs/tab_strip_gtk.cc b/chrome/browser/gtk/tabs/tab_strip_gtk.cc
index 8d243ba..01be3db 100644
--- a/chrome/browser/gtk/tabs/tab_strip_gtk.cc
+++ b/chrome/browser/gtk/tabs/tab_strip_gtk.cc
@@ -843,9 +843,8 @@ void TabStripGtk::DestroyDraggedSourceTab(TabGtk* tab) {
   }
 
   gtk_container_remove(GTK_CONTAINER(tabstrip_.get()), tab->widget());
-  // If we delete the dragged source tab here, the gtk drag-n-drop API won't
-  // get a change to clean up and remove any references it's added to the tab
-  // widget, so we'll leak the widget.
+  // If we delete the dragged source tab here, the DestroyDragWidget posted
+  // task will be run after the tab is deleted, leading to a crash.
   MessageLoop::current()->DeleteSoon(FROM_HERE, tab);
 
   // Force a layout here, because if we've just quickly drag detached a Tab,
@@ -983,6 +982,8 @@ void TabStripGtk::TabSelectedAt(TabContents* old_contents,
     if (!IsAnimating() && (!resize_layout_scheduled_ || tiny_tabs))
       Layout();
 
+    GetTabAt(index)->SchedulePaint();
+
     int old_index = model_->GetIndexOfTabContents(old_contents);
     if (old_index >= 0)
       GetTabAt(old_index)->SchedulePaint();