GTK browser actions toolbar fiddling:

- make the toolbar size restore properly when the toolbar is created before the model is loaded
- allow the user to right click menu items in the overflow menu

BUG=40068 (sorta)
TEST=manual

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

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

9 files changed, 148 insertions, 64 deletions

diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc
index f99928a..cac48db 100644
--- a/chrome/browser/extensions/extension_toolbar_model.cc
+++ b/chrome/browser/extensions/extension_toolbar_model.cc
@@ -189,6 +189,7 @@ void ExtensionToolbarModel::InitializeExtensionList() {
   UpdatePrefs();
 
   extensions_initialized_ = true;
+  FOR_EACH_OBSERVER(Observer, observers_, ModelLoaded());
 }
 
 void ExtensionToolbarModel::UpdatePrefs() {
diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h
index a4e5c64..f2bbe9d 100644
--- a/chrome/browser/extensions/extension_toolbar_model.h
+++ b/chrome/browser/extensions/extension_toolbar_model.h
@@ -32,6 +32,9 @@ class ExtensionToolbarModel : public NotificationObserver {
 
     // The browser action button for |extension| has been moved to |index|.
     virtual void BrowserActionMoved(Extension* extension, int index) {}
+
+    // Called when the model has finished loading.
+    virtual void ModelLoaded() {}
   };
 
   // Functions called by the view.
diff --git a/chrome/browser/gtk/bookmark_menu_controller_gtk.cc b/chrome/browser/gtk/bookmark_menu_controller_gtk.cc
index 4f5c76f..485b440 100644
--- a/chrome/browser/gtk/bookmark_menu_controller_gtk.cc
+++ b/chrome/browser/gtk/bookmark_menu_controller_gtk.cc
@@ -56,26 +56,7 @@ void* AsVoid(const BookmarkNode* node) {
 // to whichever menu last had them. (Assuming that menu is still showing.)
 // The event mask in this function is taken from gtkmenu.c.
 void OnContextMenuHide(GtkWidget* context_menu, GtkWidget* grab_menu) {
-  guint time = gtk_get_current_event_time();
-
-  if (GTK_WIDGET_VISIBLE(grab_menu)) {
-    if (!gdk_pointer_grab(grab_menu->window, TRUE,
-                          GdkEventMask(
-                          GDK_BUTTON_PRESS_MASK | GDK_BUTTON_RELEASE_MASK |
-                          GDK_ENTER_NOTIFY_MASK | GDK_LEAVE_NOTIFY_MASK |
-                          GDK_POINTER_MOTION_MASK), NULL, NULL, time) == 0) {
-      g_object_unref(grab_menu);
-      return;
-    }
-    if (!gdk_keyboard_grab(grab_menu->window, TRUE, time) == 0) {
-      gdk_display_pointer_ungrab(gdk_drawable_get_display(grab_menu->window),
-                                 time);
-      g_object_unref(grab_menu);
-      return;
-    }
-    gtk_grab_add(grab_menu);
-  }
-
+  gtk_util::GrabAllInput(grab_menu);
   // Match the ref we took when connecting this signal.
   g_object_unref(grab_menu);
 }
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index 977bd2f..7f3ec625 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -48,6 +48,8 @@ const int kSeparatorPadding = 2;
 
 const char* kDragTarget = "application/x-chrome-browseraction";
 
+const char* kMenuItemKey = "__CHROME_OVERFLOW_MENU_ITEM_KEY__";
+
 GtkTargetEntry GetDragTargetEntry() {
   static std::string drag_target_string(kDragTarget);
   GtkTargetEntry drag_target;
@@ -179,10 +181,30 @@ class BrowserActionButton : public NotificationObserver,
     }
   }
 
+  MenuGtk* GetContextMenu() {
+    context_menu_model_.reset(
+        new ExtensionContextMenuModel(extension_, toolbar_->browser(), this));
+    context_menu_.reset(
+        new MenuGtk(this, context_menu_model_.get()));
+    return context_menu_.get();
+  }
+
  private:
   // MenuGtk::Delegate implementation.
   virtual void StoppedShowing() {
     gtk_chrome_button_unset_paint_state(GTK_CHROME_BUTTON(button_.get()));
+
+    // If the context menu was showing for the overflow menu, re-assert the
+    // grab that was shadowed.
+    if (toolbar_->overflow_menu_.get())
+      gtk_util::GrabAllInput(toolbar_->overflow_menu_->widget());
+  }
+
+  virtual void CommandWasExecuted() {
+    // If the context menu was showing for the overflow menu, and a command
+    // is executed, then stop showing the overflow menu.
+    if (toolbar_->overflow_menu_.get())
+      toolbar_->overflow_menu_->Cancel();
   }
 
   // Returns true to prevent further processing of the event that caused us to
@@ -224,16 +246,9 @@ class BrowserActionButton : public NotificationObserver,
     if (event->button.button != 3)
       return FALSE;
 
-    action->context_menu_model_.reset(
-        new ExtensionContextMenuModel(
-            action->extension_,
-            action->toolbar_->browser(),
-            action));
-    action->context_menu_.reset(
-        new MenuGtk(action, action->context_menu_model_.get()));
     gtk_chrome_button_set_paint_state(GTK_CHROME_BUTTON(action->button_.get()),
                                       GTK_STATE_PRELIGHT);
-    action->context_menu_->Popup(widget, event);
+    action->GetContextMenu()->Popup(widget, event);
 
     return TRUE;
   }
@@ -352,20 +367,17 @@ BrowserActionsToolbarGtk::BrowserActionsToolbarGtk(Browser* browser)
   model_ = extension_service->toolbar_model();
   model_->AddObserver(this);
   SetupDrags();
-  CreateAllButtons();
+
+  if (model_->extensions_initialized()) {
+    CreateAllButtons();
+    SetContainerWidth();
+  }
 
   // We want to connect to "set-focus" on the toplevel window; we have to wait
   // until we are added to a toplevel window to do so.
   g_signal_connect(widget(), "hierarchy-changed",
                    G_CALLBACK(OnHierarchyChangedThunk), this);
 
-  int showing_actions = model_->GetVisibleIconCount();
-  if (showing_actions >= 0) {
-    SetButtonHBoxWidth(WidthForIconCount(showing_actions));
-    if (showing_actions < static_cast<int>(model_->size()))
-      gtk_widget_show(overflow_button_.widget());
-  }
-
   ViewIDUtil::SetID(button_hbox_, VIEW_ID_BROWSER_ACTION_TOOLBAR);
 }
 
@@ -415,6 +427,15 @@ void BrowserActionsToolbarGtk::CreateAllButtons() {
   }
 }
 
+void BrowserActionsToolbarGtk::SetContainerWidth() {
+  int showing_actions = model_->GetVisibleIconCount();
+  if (showing_actions >= 0) {
+    SetButtonHBoxWidth(WidthForIconCount(showing_actions));
+    if (showing_actions < static_cast<int>(model_->size()))
+      gtk_widget_show(overflow_button_.widget());
+  }
+}
+
 void BrowserActionsToolbarGtk::CreateButtonForExtension(Extension* extension,
                                                         int index) {
   if (!ShouldDisplayBrowserAction(extension))
@@ -488,6 +509,8 @@ void BrowserActionsToolbarGtk::AnimateToShowNIcons(int count) {
 
 void BrowserActionsToolbarGtk::BrowserActionAdded(Extension* extension,
                                                   int index) {
+  overflow_menu_.reset();
+
   CreateButtonForExtension(extension, index);
 
   // If we are still initializing the container, don't bother animating.
@@ -502,6 +525,8 @@ void BrowserActionsToolbarGtk::BrowserActionAdded(Extension* extension,
 }
 
 void BrowserActionsToolbarGtk::BrowserActionRemoved(Extension* extension) {
+  overflow_menu_.reset();
+
   if (drag_button_ != NULL) {
     // Break the current drag.
     gtk_grab_remove(button_hbox_);
@@ -536,6 +561,10 @@ void BrowserActionsToolbarGtk::BrowserActionMoved(Extension* extension,
   gtk_box_reorder_child(GTK_BOX(button_hbox_), button->widget(), index);
 }
 
+void BrowserActionsToolbarGtk::ModelLoaded() {
+  SetContainerWidth();
+}
+
 void BrowserActionsToolbarGtk::AnimationProgressed(const Animation* animation) {
   int width = start_width_ + (desired_width_ - start_width_) *
       animation->GetCurrentValue();
@@ -761,17 +790,38 @@ gboolean BrowserActionsToolbarGtk::OnOverflowButtonPress(
     Extension* extension = model_->GetExtensionByIndex(i);
     BrowserActionButton* button = extension_button_map_[extension->id()].get();
 
-    overflow_menu_->AppendMenuItemWithIcon(
+    GtkWidget* menu_item = overflow_menu_->AppendMenuItemWithIcon(
         i,
         extension->name(),
         button->GetIcon());
 
+    g_object_set_data(G_OBJECT(menu_item), kMenuItemKey, button);
+
     // TODO(estade): set the menu item's tooltip.
   }
 
+  g_signal_connect(overflow_menu_->widget(), "button-press-event",
+                   G_CALLBACK(OnOverflowMenuButtonPressThunk), this);
+
   gtk_chrome_button_set_paint_state(GTK_CHROME_BUTTON(overflow),
                                     GTK_STATE_ACTIVE);
   overflow_menu_->PopupAsFromKeyEvent(overflow);
 
   return FALSE;
 }
+
+gboolean BrowserActionsToolbarGtk::OnOverflowMenuButtonPress(
+    GtkWidget* overflow, GdkEventButton* event) {
+  if (event->button != 3)
+    return FALSE;
+
+  GtkWidget* menu_item = GTK_MENU_SHELL(overflow)->active_menu_item;
+  if (!menu_item)
+    return FALSE;
+
+  void* data = g_object_get_data(G_OBJECT(menu_item), kMenuItemKey);
+  DCHECK(data);
+  BrowserActionButton* button = static_cast<BrowserActionButton*>(data);
+  button->GetContextMenu()->PopupAsContext(event->time);
+  return TRUE;
+}
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.h b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
index 5d5f342..955f55e 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.h
@@ -63,6 +63,9 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
   // and create the UI for them.
   void CreateAllButtons();
 
+  // Sets the width of the container and overflow state according to the model.
+  void SetContainerWidth();
+
   // Create the UI for a single browser action. This will stick the button
   // at the end of the toolbar.
   void CreateButtonForExtension(Extension* extension, int index);
@@ -90,6 +93,7 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
   virtual void BrowserActionAdded(Extension* extension, int index);
   virtual void BrowserActionRemoved(Extension* extension);
   virtual void BrowserActionMoved(Extension* extension, int index);
+  virtual void ModelLoaded();
 
   // AnimationDelegate implementation.
   virtual void AnimationProgressed(const Animation* animation);
@@ -130,8 +134,12 @@ class BrowserActionsToolbarGtk : public ExtensionToolbarModel::Observer,
                        OnGripperButtonRelease, GdkEventButton*);
   CHROMEGTK_CALLBACK_1(BrowserActionsToolbarGtk, gboolean,
                        OnGripperButtonPress, GdkEventButton*);
+  // The overflow button is pressed.
   CHROMEGTK_CALLBACK_1(BrowserActionsToolbarGtk, gboolean,
                        OnOverflowButtonPress, GdkEventButton*);
+  // The user presses a mouse button over the popped up overflow menu.
+  CHROMEGTK_CALLBACK_1(BrowserActionsToolbarGtk, gboolean,
+                       OnOverflowMenuButtonPress, GdkEventButton*);
 
   Browser* browser_;
 
diff --git a/chrome/browser/gtk/gtk_util.cc b/chrome/browser/gtk/gtk_util.cc
index bb3725c..6157bda 100644
--- a/chrome/browser/gtk/gtk_util.cc
+++ b/chrome/browser/gtk/gtk_util.cc
@@ -814,4 +814,29 @@ WindowOpenDisposition DispositionForCurrentButtonPressEvent() {
   return event_utils::DispositionFromEventFlags(state);
 }
 
+bool GrabAllInput(GtkWidget* widget) {
+  guint time = gtk_get_current_event_time();
+
+  if (!GTK_WIDGET_VISIBLE(widget))
+    return false;
+
+  if (!gdk_pointer_grab(widget->window, TRUE,
+                        GdkEventMask(GDK_BUTTON_PRESS_MASK |
+                                     GDK_BUTTON_RELEASE_MASK |
+                                     GDK_ENTER_NOTIFY_MASK |
+                                     GDK_LEAVE_NOTIFY_MASK |
+                                     GDK_POINTER_MOTION_MASK),
+                        NULL, NULL, time) == 0) {
+    return false;
+  }
+
+  if (!gdk_keyboard_grab(widget->window, TRUE, time) == 0) {
+    gdk_display_pointer_ungrab(gdk_drawable_get_display(widget->window), time);
+    return false;
+  }
+
+  gtk_grab_add(widget);
+  return true;
+}
+
 }  // namespace gtk_util
diff --git a/chrome/browser/gtk/gtk_util.h b/chrome/browser/gtk/gtk_util.h
index 5d261ca..92164ad 100644
--- a/chrome/browser/gtk/gtk_util.h
+++ b/chrome/browser/gtk/gtk_util.h
@@ -247,6 +247,10 @@ void SuppressDefaultPainting(GtkWidget* container);
 // error to call it when gtk_get_current_event() won't return a GdkEventButton*.
 WindowOpenDisposition DispositionForCurrentButtonPressEvent();
 
+// Safely grabs all input (with X grabs and an application grab), returning true
+// for success.
+bool GrabAllInput(GtkWidget* widget);
+
 }  // namespace gtk_util
 
 #endif  // CHROME_BROWSER_GTK_GTK_UTIL_H_
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index dd6945d..31c2a55 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -158,49 +158,51 @@ void MenuGtk::ConnectSignalHandlers() {
   g_signal_connect(menu_, "hide", G_CALLBACK(OnMenuHidden), this);
 }
 
-void MenuGtk::AppendMenuItemWithLabel(int command_id,
-                                      const std::string& label) {
+GtkWidget* MenuGtk::AppendMenuItemWithLabel(int command_id,
+                                            const std::string& label) {
   std::string converted_label = ConvertAcceleratorsFromWindowsStyle(label);
   GtkWidget* menu_item =
       gtk_menu_item_new_with_mnemonic(converted_label.c_str());
-  AppendMenuItem(command_id, menu_item);
+  return AppendMenuItem(command_id, menu_item);
 }
 
-void MenuGtk::AppendMenuItemWithIcon(int command_id,
-                                     const std::string& label,
-                                     const SkBitmap& icon) {
+GtkWidget* MenuGtk::AppendMenuItemWithIcon(int command_id,
+                                           const std::string& label,
+                                           const SkBitmap& icon) {
   std::string converted_label = ConvertAcceleratorsFromWindowsStyle(label);
   GtkWidget* menu_item = BuildMenuItemWithImage(converted_label, icon);
-  AppendMenuItem(command_id, menu_item);
+  return AppendMenuItem(command_id, menu_item);
 }
 
-void MenuGtk::AppendCheckMenuItemWithLabel(int command_id,
-                                           const std::string& label) {
+GtkWidget* MenuGtk::AppendCheckMenuItemWithLabel(int command_id,
+                                                 const std::string& label) {
   std::string converted_label = ConvertAcceleratorsFromWindowsStyle(label);
   GtkWidget* menu_item =
       gtk_check_menu_item_new_with_mnemonic(converted_label.c_str());
-  AppendMenuItem(command_id, menu_item);
+  return AppendMenuItem(command_id, menu_item);
 }
 
-void MenuGtk::AppendSeparator() {
+GtkWidget* MenuGtk::AppendSeparator() {
   GtkWidget* menu_item = gtk_separator_menu_item_new();
   gtk_widget_show(menu_item);
   gtk_menu_shell_append(GTK_MENU_SHELL(menu_), menu_item);
+  return menu_item;
 }
 
-void MenuGtk::AppendMenuItem(int command_id, GtkWidget* menu_item) {
-  AppendMenuItemToMenu(command_id, menu_item, menu_);
+GtkWidget* MenuGtk::AppendMenuItem(int command_id, GtkWidget* menu_item) {
+  return AppendMenuItemToMenu(command_id, menu_item, menu_);
 }
 
-void MenuGtk::AppendMenuItemToMenu(int command_id,
-                                   GtkWidget* menu_item,
-                                   GtkWidget* menu) {
+GtkWidget* MenuGtk::AppendMenuItemToMenu(int command_id,
+                                         GtkWidget* menu_item,
+                                         GtkWidget* menu) {
   SetMenuItemID(menu_item, command_id);
   g_signal_connect(menu_item, "activate",
                    G_CALLBACK(OnMenuItemActivated), this);
 
   gtk_widget_show(menu_item);
   gtk_menu_shell_append(GTK_MENU_SHELL(menu), menu_item);
+  return menu_item;
 }
 
 void MenuGtk::Popup(GtkWidget* widget, GdkEvent* event) {
@@ -511,6 +513,9 @@ void MenuGtk::ExecuteCommand(menus::MenuModel* model, int id) {
     model->ActivatedAt(id);
   else
     delegate_->ExecuteCommandById(id);
+
+  if (delegate_)
+    delegate_->CommandWasExecuted();
 }
 
 // http://crbug.com/31365
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index 6f076c4..4dfbdcf 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -40,7 +40,12 @@ class MenuGtk {
     // Executes the command.
     virtual void ExecuteCommandById(int command_id) {}
 
-    // Called when the menu stops showing. This will be called along with
+    // Called after a command is executed. This exists for the case where a
+    // model is handling the actual execution of commands, but the delegate
+    // still needs to know that some command got executed.
+    virtual void CommandWasExecuted() {}
+
+    // Called when the menu stops showing. This will be called before
     // ExecuteCommand if the user clicks an item, but will also be called when
     // the user clicks away from the menu.
     virtual void StoppedShowing() {}
@@ -64,16 +69,18 @@ class MenuGtk {
   // Initialize GTK signal handlers.
   void ConnectSignalHandlers();
 
-  // These methods are used to build the menu dynamically.
-  void AppendMenuItemWithLabel(int command_id, const std::string& label);
-  void AppendMenuItemWithIcon(int command_id, const std::string& label,
-                              const SkBitmap& icon);
-  void AppendCheckMenuItemWithLabel(int command_id, const std::string& label);
-  void AppendSeparator();
-  void AppendMenuItem(int command_id, GtkWidget* menu_item);
-  void AppendMenuItemToMenu(int command_id,
-                            GtkWidget* menu_item,
-                            GtkWidget* menu);
+  // These methods are used to build the menu dynamically. The return value
+  // is the new menu item.
+  GtkWidget* AppendMenuItemWithLabel(int command_id, const std::string& label);
+  GtkWidget* AppendMenuItemWithIcon(int command_id, const std::string& label,
+                                    const SkBitmap& icon);
+  GtkWidget* AppendCheckMenuItemWithLabel(int command_id,
+                                          const std::string& label);
+  GtkWidget* AppendSeparator();
+  GtkWidget* AppendMenuItem(int command_id, GtkWidget* menu_item);
+  GtkWidget* AppendMenuItemToMenu(int command_id,
+                                  GtkWidget* menu_item,
+                                  GtkWidget* menu);
 
   // Displays the menu. |timestamp| is the time of activation. The popup is
   // statically positioned at |widget|.