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
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 @@
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 @@
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 @@
// 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 @@
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..f8916d7f 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -276,10 +276,9 @@
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 @@
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 @@
}
// 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 571e43f7..6a58e17 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.h
+++ b/chrome/browser/gtk/bookmark_bar_gtk.h
@@ -228,8 +228,6 @@
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 @@
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 @@
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 @@
}
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 @@
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 @@
: 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::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::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 @@
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 @@
// 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 @@
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 @@
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 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 7c10ce33..7484f48 100644
--- a/chrome/common/gtk_util.h
+++ b/chrome/common/gtk_util.h
@@ -203,6 +203,15 @@
// 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_
