GTK: hook up drag and drop of browser actions (for reordering).

The changes are propagated across open chrome windows, but are not persisted between sessions yet.

BUG=26990

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

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

10 files changed, 239 insertions, 54 deletions

diff --git a/chrome/browser/extensions/browser_action_apitest.cc b/chrome/browser/extensions/browser_action_apitest.cc
index b7a16ac..db9a9d2 100644
--- a/chrome/browser/extensions/browser_action_apitest.cc
+++ b/chrome/browser/extensions/browser_action_apitest.cc
@@ -193,7 +193,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionTest, MAYBE_DynamicBrowserAction) {
   ASSERT_TRUE(RunExtensionTest("browser_action_no_icon")) << message_;
 
   // Test that there is a browser action in the toolbar and that it has no icon.
-  ASSERT_EQ(1, NumberOfBrowserActions());
+  EXPECT_EQ(1, NumberOfBrowserActions());
   EXPECT_TRUE(IsIconNull(0));
 
   // Tell the extension to update the icon using setIcon({imageData:...}).
diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index 39856cf..5a9f2a8 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -33,6 +33,35 @@ void ExtensionToolbarModel::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void ExtensionToolbarModel::MoveBrowserAction(Extension* extension,
+                                              int index) {
+  ExtensionList::iterator pos = std::find(begin(), end(), extension);
+  if (pos == end()) {
+    NOTREACHED();
+    return;
+  }
+  toolitems_.erase(pos);
+
+  int i = 0;
+  bool inserted = false;
+  for (ExtensionList::iterator iter = begin(); iter != end(); ++iter, ++i) {
+    if (i == index) {
+      toolitems_.insert(pos, extension);
+      inserted = true;
+      break;
+    }
+  }
+
+  if (!inserted) {
+    DCHECK_EQ(index, static_cast<int>(toolitems_.size()));
+    index = toolitems_.size();
+
+    toolitems_.push_back(extension);
+  }
+
+  FOR_EACH_OBSERVER(Observer, observers_, BrowserActionMoved(extension, index));
+}
+
 void ExtensionToolbarModel::Observe(NotificationType type,
                                     const NotificationSource& source,
                                     const NotificationDetails& details) {
diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h
index d02dc0f..54977eb 100644
--- a/chrome/browser/extensions/extension_toolbar_model.h
+++ b/chrome/browser/extensions/extension_toolbar_model.h
@@ -28,10 +28,15 @@ class ExtensionToolbarModel : public NotificationObserver {
 
     // The browser action button for |extension| should no longer show.
     virtual void BrowserActionRemoved(Extension* extension) {}
+
+    // The browser action button for |extension| has been moved to |index|.
+    virtual void BrowserActionMoved(Extension* extension, int index) {}
   };
 
+  // Functions called by the view.
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
+  void MoveBrowserAction(Extension* extension, int index);
 
   ExtensionList::iterator begin() {
     return toolitems_.begin();
@@ -61,4 +66,5 @@ class ExtensionToolbarModel : public NotificationObserver {
 
   NotificationRegistrar registrar_;
 };
+
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_TOOLBAR_MODEL_H_
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 058907e..f8916d7 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -276,10 +276,9 @@ void ExtensionsService::EnableExtension(const std::string& extension_id) {
 
 void ExtensionsService::DisableExtension(const std::string& extension_id) {
   Extension* extension = GetExtensionByIdInternal(extension_id, true, false);
-  if (!extension) {
-    NOTREACHED() << "Trying to disable an extension that isn't enabled.";
+  // The extension may have been disabled already.
+  if (!extension)
     return;
-  }
 
   // Remember that we disabled it, unless it's temporary.
   if (extension->location() != Extension::LOAD)
diff --git a/chrome/browser/gtk/bookmark_bar_gtk.cc b/chrome/browser/gtk/bookmark_bar_gtk.cc
index 05db22b..cac9ed4 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.cc
+++ b/chrome/browser/gtk/bookmark_bar_gtk.cc
@@ -235,9 +235,7 @@ void BookmarkBarGtk::Init(Profile* profile) {
   bookmark_toolbar_.Own(gtk_toolbar_new());
   SetToolBarStyle();
   gtk_widget_set_name(bookmark_toolbar_.get(), "chrome-bookmark-toolbar");
-  gtk_widget_set_app_paintable(bookmark_toolbar_.get(), TRUE);
-  g_signal_connect(bookmark_toolbar_.get(), "expose-event",
-                   G_CALLBACK(&OnToolbarExpose), this);
+  gtk_util::SuppressDefaultPainting(bookmark_toolbar_.get());
   g_signal_connect(bookmark_toolbar_.get(), "size-allocate",
                    G_CALLBACK(&OnToolbarSizeAllocate), this);
   gtk_box_pack_start(GTK_BOX(bookmark_hbox_), bookmark_toolbar_.get(),
@@ -974,23 +972,6 @@ void BookmarkBarGtk::OnFolderClicked(GtkWidget* sender,
 }
 
 // static
-gboolean BookmarkBarGtk::OnToolbarExpose(GtkWidget* widget,
-                                         GdkEventExpose* event,
-                                         BookmarkBarGtk* bar) {
-  // A GtkToolbar's expose handler first draws a box. We don't want that so we
-  // need to propagate the expose event to all the container's children.
-  GList* children = gtk_container_get_children(GTK_CONTAINER(widget));
-  for (GList* item = children; item; item = item->next) {
-    gtk_container_propagate_expose(GTK_CONTAINER(widget),
-                                   GTK_WIDGET(item->data),
-                                   event);
-  }
-  g_list_free(children);
-
-  return TRUE;
-}
-
-// static
 gboolean BookmarkBarGtk::OnToolbarDragMotion(GtkToolbar* toolbar,
                                              GdkDragContext* context,
                                              gint x,
diff --git a/chrome/browser/gtk/bookmark_bar_gtk.h b/chrome/browser/gtk/bookmark_bar_gtk.h
index 571e43f..6a58e17 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.h
+++ b/chrome/browser/gtk/bookmark_bar_gtk.h
@@ -228,8 +228,6 @@ class BookmarkBarGtk : public AnimationDelegate,
                               BookmarkBarGtk* bar);
 
   // GtkToolbar callbacks.
-  static gboolean OnToolbarExpose(GtkWidget* widget, GdkEventExpose* event,
-                                  BookmarkBarGtk* window);
   static gboolean OnToolbarDragMotion(GtkToolbar* toolbar,
                                       GdkDragContext* context,
                                       gint x,
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index 91ea423..56424b3 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -4,7 +4,6 @@
 
 #include "chrome/browser/gtk/browser_actions_toolbar_gtk.h"
 
-#include <gtk/gtk.h>
 #include <vector>
 
 #include "app/gfx/canvas_paint.h"
@@ -26,13 +25,28 @@
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
 
+namespace {
+
 // The size of each button on the toolbar.
-static const int kButtonSize = 29;
+const int kButtonSize = 29;
 
 // The padding between browser action buttons. Visually, the actual number of
 // "empty" (non-drawing) pixels is this value + 2 when adjacent browser icons
 // use their maximum allowed size.
-static const int kBrowserActionButtonPadding = 3;
+const int kButtonPadding = 3;
+
+const char* kDragTarget = "application/x-chrome-browseraction";
+
+GtkTargetEntry GetDragTargetEntry() {
+  static std::string drag_target_string(kDragTarget);
+  GtkTargetEntry drag_target;
+  drag_target.target = const_cast<char*>(drag_target_string.c_str());
+  drag_target.flags = GTK_TARGET_SAME_APP;
+  drag_target.info = 0;
+  return drag_target;
+}
+
+}  // namespace
 
 class BrowserActionButton : public NotificationObserver,
                             public ImageLoadingTracker::Observer {
@@ -61,11 +75,12 @@ class BrowserActionButton : public NotificationObserver,
                     Extension::kBrowserActionIconMaxSize));
     }
 
-    // We need to hook up extension popups here. http://crbug.com/23897
     g_signal_connect(button_.get(), "clicked",
                      G_CALLBACK(OnButtonClicked), this);
     g_signal_connect_after(button_.get(), "expose-event",
                            G_CALLBACK(OnExposeEvent), this);
+    g_signal_connect(button_.get(), "drag-begin",
+                     G_CALLBACK(&OnDragBegin), this);
 
     registrar_.Add(this, NotificationType::EXTENSION_BROWSER_ACTION_UPDATED,
                    Source<ExtensionAction>(extension->browser_action()));
@@ -90,6 +105,9 @@ class BrowserActionButton : public NotificationObserver,
 
   GtkWidget* widget() { return button_.get(); }
 
+  Extension* extension() { return extension_; }
+
+  // NotificationObserver implementation.
   void Observe(NotificationType type,
                const NotificationSource& source,
                const NotificationDetails& details) {
@@ -148,16 +166,16 @@ class BrowserActionButton : public NotificationObserver,
   }
 
   static void OnButtonClicked(GtkWidget* widget, BrowserActionButton* action) {
-   if (action->extension_->browser_action()->has_popup()) {
-     ExtensionPopupGtk::Show(
-         action->extension_->browser_action()->popup_url(),
-         action->toolbar_->browser(),
-         gtk_util::GetWidgetRectRelativeToToplevel(widget));
-   } else {
-     ExtensionBrowserEventRouter::GetInstance()->BrowserActionExecuted(
-         action->toolbar_->browser()->profile(), action->extension_->id(),
-         action->toolbar_->browser());
-   }
+    if (action->extension_->browser_action()->has_popup()) {
+      ExtensionPopupGtk::Show(
+          action->extension_->browser_action()->popup_url(),
+          action->toolbar_->browser(),
+          gtk_util::GetWidgetRectRelativeToToplevel(widget));
+    } else {
+      ExtensionBrowserEventRouter::GetInstance()->BrowserActionExecuted(
+          action->toolbar_->browser()->profile(), action->extension_->id(),
+          action->toolbar_->browser());
+    }
   }
 
   static gboolean OnExposeEvent(GtkWidget* widget,
@@ -177,6 +195,15 @@ class BrowserActionButton : public NotificationObserver,
     return FALSE;
   }
 
+  static void OnDragBegin(GtkWidget* widget,
+                          GdkDragContext* drag_context,
+                          BrowserActionButton* button) {
+    // Simply pass along the notification to the toolbar. The point of this
+    // function is to tell the toolbar which BrowserActionButton initiated the
+    // drag.
+    button->toolbar_->DragStarted(button, drag_context);
+  }
+
   // The toolbar containing this button.
   BrowserActionsToolbarGtk* toolbar_;
 
@@ -204,14 +231,18 @@ BrowserActionsToolbarGtk::BrowserActionsToolbarGtk(Browser* browser)
     : browser_(browser),
       profile_(browser->profile()),
       model_(NULL),
-      hbox_(gtk_hbox_new(FALSE, kBrowserActionButtonPadding)) {
+      hbox_(gtk_hbox_new(FALSE, kButtonPadding)),
+      drag_button_(NULL),
+      drop_index_(-1) {
   ExtensionsService* extension_service = profile_->GetExtensionsService();
   // The |extension_service| can be NULL in Incognito.
-  if (extension_service) {
-    model_ = extension_service->toolbar_model();
-    model_->AddObserver(this);
-    CreateAllButtons();
-  }
+  if (!extension_service)
+    return;
+
+  model_ = extension_service->toolbar_model();
+  model_->AddObserver(this);
+  SetupDrags();
+  CreateAllButtons();
 }
 
 BrowserActionsToolbarGtk::~BrowserActionsToolbarGtk() {
@@ -235,21 +266,42 @@ void BrowserActionsToolbarGtk::Update() {
   }
 }
 
+void BrowserActionsToolbarGtk::SetupDrags() {
+  GtkTargetEntry drag_target = GetDragTargetEntry();
+  gtk_drag_dest_set(widget(), GTK_DEST_DEFAULT_DROP, &drag_target, 1,
+                    GDK_ACTION_MOVE);
+
+  g_signal_connect(widget(), "drag-motion",
+                   G_CALLBACK(OnDragMotionThunk), this);
+}
+
 void BrowserActionsToolbarGtk::CreateAllButtons() {
+  extension_button_map_.clear();
+
+  int i = 0;
   for (ExtensionList::iterator iter = model_->begin();
        iter != model_->end(); ++iter) {
-    CreateButtonForExtension(*iter);
+    CreateButtonForExtension(*iter, i++);
   }
 }
 
-void BrowserActionsToolbarGtk::CreateButtonForExtension(Extension* extension) {
+void BrowserActionsToolbarGtk::CreateButtonForExtension(Extension* extension,
+                                                        int index) {
   RemoveButtonForExtension(extension);
   linked_ptr<BrowserActionButton> button(
       new BrowserActionButton(this, extension));
-  gtk_box_pack_end(GTK_BOX(hbox_.get()), button->widget(), FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(hbox_.get()), button->widget(), FALSE, FALSE, 0);
+  gtk_box_reorder_child(GTK_BOX(hbox_.get()), button->widget(), index);
   gtk_widget_show(button->widget());
   extension_button_map_[extension->id()] = button;
 
+  GtkTargetEntry drag_target = GetDragTargetEntry();
+  gtk_drag_source_set(button->widget(), GDK_BUTTON1_MASK, &drag_target, 1,
+                      GDK_ACTION_MOVE);
+  // We ignore whether the drag was a "success" or "failure" in Gtk's opinion.
+  g_signal_connect(button->widget(), "drag-end",
+                   G_CALLBACK(&OnDragEndThunk), this);
+
   UpdateVisibility();
 }
 
@@ -267,10 +319,68 @@ void BrowserActionsToolbarGtk::UpdateVisibility() {
 
 void BrowserActionsToolbarGtk::BrowserActionAdded(Extension* extension,
                                                   int index) {
-  // TODO(estade): respect |index|.
-  CreateButtonForExtension(extension);
+  CreateButtonForExtension(extension, index);
 }
 
 void BrowserActionsToolbarGtk::BrowserActionRemoved(Extension* extension) {
+  if (drag_button_ != NULL) {
+    // Break the current drag.
+    gtk_grab_remove(widget());
+
+    // Re-generate the toolbar to clean up unfinished drag business (i.e., we
+    // may have re-ordered some buttons; this will put them back where they
+    // belong).
+    CreateAllButtons();
+  }
+
   RemoveButtonForExtension(extension);
 }
+
+void BrowserActionsToolbarGtk::BrowserActionMoved(Extension* extension,
+                                                  int index) {
+  // We initiated this move action, and have already moved the button.
+  if (drag_button_ != NULL)
+    return;
+
+  BrowserActionButton* button = extension_button_map_[extension->id()].get();
+  if (!button) {
+    NOTREACHED();
+    return;
+  }
+
+  gtk_box_reorder_child(GTK_BOX(hbox_.get()), button->widget(), index);
+}
+
+void BrowserActionsToolbarGtk::DragStarted(BrowserActionButton* button,
+                                           GdkDragContext* drag_context) {
+  // No representation of the widget following the cursor.
+  GdkPixbuf* pixbuf = gdk_pixbuf_new(GDK_COLORSPACE_RGB, TRUE, 8, 1, 1);
+  gtk_drag_set_icon_pixbuf(drag_context, pixbuf, 0, 0);
+  g_object_unref(pixbuf);
+
+  DCHECK(!drag_button_);
+  drag_button_ = button;
+}
+
+gboolean BrowserActionsToolbarGtk::OnDragMotion(GtkWidget* widget,
+                                                GdkDragContext* drag_context,
+                                                gint x, gint y, guint time) {
+  drop_index_ = x < kButtonSize ? 0 : x / (kButtonSize + kButtonPadding);
+  // We will go ahead and reorder the child in order to provide visual feedback
+  // to the user. We don't inform the model that it has moved until the drag
+  // ends.
+  gtk_box_reorder_child(GTK_BOX(hbox_.get()), drag_button_->widget(),
+                        drop_index_);
+
+  gdk_drag_status(drag_context, GDK_ACTION_MOVE, time);
+  return TRUE;
+}
+
+void BrowserActionsToolbarGtk::OnDragEnd(GtkWidget* button,
+                                         GdkDragContext* drag_context) {
+  if (drop_index_ != -1)
+    model_->MoveBrowserAction(drag_button_->extension(), drop_index_);
+
+  drag_button_ = NULL;
+  drop_index_ = -1;
+}
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.h b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
index ceee599b..d2c27e6 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_GTK_BROWSER_ACTIONS_TOOLBAR_GTK_H_
 #define CHROME_BROWSER_GTK_BROWSER_ACTIONS_TOOLBAR_GTK_H_
 
+#include <gtk/gtk.h>
+
 #include <map>
 #include <string>
 
@@ -39,15 +41,18 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer {
   void Update();
 
  private:
+  friend class BrowserActionButton;
+
+  // Initialize drag and drop.
+  void SetupDrags();
+
   // Query the extensions service for all extensions with browser actions,
   // and create the UI for them.
   void CreateAllButtons();
 
   // Create the UI for a single browser action. This will stick the button
   // at the end of the toolbar.
-  // TODO(estade): is this OK, or does it need to place it in a specific
-  // location on the toolbar?
-  void CreateButtonForExtension(Extension* extension);
+  void CreateButtonForExtension(Extension* extension, int index);
 
   // Delete resources associated with UI for a browser action.
   void RemoveButtonForExtension(Extension* extension);
@@ -59,6 +64,27 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer {
   // ExtensionToolbarModel::Observer implementation.
   virtual void BrowserActionAdded(Extension* extension, int index);
   virtual void BrowserActionRemoved(Extension* extension);
+  virtual void BrowserActionMoved(Extension* extension, int index);
+
+  // Called by the BrowserActionButton in response to drag-begin.
+  void DragStarted(BrowserActionButton* button, GdkDragContext* drag_context);
+
+  static gboolean OnDragMotionThunk(GtkWidget* widget,
+                                    GdkDragContext* drag_context,
+                                    gint x, gint y, guint time,
+                                    BrowserActionsToolbarGtk* toolbar) {
+    return toolbar->OnDragMotion(widget, drag_context, x, y, time);
+  }
+  gboolean OnDragMotion(GtkWidget* widget,
+                        GdkDragContext* drag_context,
+                        gint x, gint y, guint time);
+
+  static void OnDragEndThunk(GtkWidget* button,
+                             GdkDragContext* drag_context,
+                             BrowserActionsToolbarGtk* toolbar) {
+    toolbar->OnDragEnd(button, drag_context);
+  }
+  void OnDragEnd(GtkWidget* button, GdkDragContext* drag_context);
 
   Browser* browser_;
 
@@ -68,6 +94,12 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer {
 
   OwnedWidgetGtk hbox_;
 
+  // The button that is currently being dragged, or NULL.
+  BrowserActionButton* drag_button_;
+
+  // The new position of the button in the drag, or -1.
+  int drop_index_;
+
   // Map from extension ID to BrowserActionButton, which is a wrapper for
   // a chrome button and related functionality. There should be one entry
   // for every extension that has a browser action.
diff --git a/chrome/common/gtk_util.cc b/chrome/common/gtk_util.cc
index c6c08e4..c9fce55 100644
--- a/chrome/common/gtk_util.cc
+++ b/chrome/common/gtk_util.cc
@@ -96,6 +96,22 @@ class GdkCursorCache {
   std::map<GdkCursorType, GdkCursor*> cursor_cache_;
 };
 
+// Expose event handler for a container that simply suppresses the default
+// drawing and propagates the expose event to the container's children.
+gboolean PaintNoBackground(GtkWidget* widget,
+                           GdkEventExpose* event,
+                           gpointer unused) {
+  GList* children = gtk_container_get_children(GTK_CONTAINER(widget));
+  for (GList* item = children; item; item = item->next) {
+    gtk_container_propagate_expose(GTK_CONTAINER(widget),
+                                   GTK_WIDGET(item->data),
+                                   event);
+  }
+  g_list_free(children);
+
+  return TRUE;
+}
+
 }  // namespace
 
 namespace event_utils {
@@ -619,4 +635,9 @@ void ApplyMessageDialogQuirks(GtkWidget* dialog) {
   }
 }
 
+void SuppressDefaultPainting(GtkWidget* container) {
+  g_signal_connect(container, "expose-event",
+                   G_CALLBACK(PaintNoBackground), NULL);
+}
+
 }  // namespace gtk_util
diff --git a/chrome/common/gtk_util.h b/chrome/common/gtk_util.h
index 7c10ce3..7484f48 100644
--- a/chrome/common/gtk_util.h
+++ b/chrome/common/gtk_util.h
@@ -203,6 +203,15 @@ gfx::Rect GetWidgetRectRelativeToToplevel(GtkWidget* widget);
 // gtk_message_dialog_new.
 void ApplyMessageDialogQuirks(GtkWidget* dialog);
 
+// Don't allow the widget to paint anything, and instead propagate the expose
+// to its children. This is similar to calling
+//
+//   gtk_widget_set_app_paintable(container, TRUE);
+//
+// except that it will always work, and it should be called after any custom
+// expose events are connected.
+void SuppressDefaultPainting(GtkWidget* container);
+
 }  // namespace gtk_util
 
 #endif  // CHROME_COMMON_GTK_UTIL_H_