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();