GTK: Add Recently Closed tabs to the History menu in the global menu bar.

chrome/browser/ui/gtk/menu_gtk.h

Index: chrome/browser/ui/gtk/menu_gtk.h
diff --git a/chrome/browser/ui/gtk/menu_gtk.h b/chrome/browser/ui/gtk/menu_gtk.h
index 381993bbba5d367ffd4055807906588d3320b9c0..012c4a8decc298baf7cbd8b03ca26a24efe4c9ac 100644
--- a/chrome/browser/ui/gtk/menu_gtk.h
+++ b/chrome/browser/ui/gtk/menu_gtk.h
@@ -22,11 +22,70 @@ class ButtonMenuItemModel;
 class MenuModel;
 }
 
-class MenuGtk {
+// We have multiple places in the code that actually build menus and have
+// wildly different requirements.
+class MenuCreator {

[Evan Stade, 2011/04/20 22:52:10]

can we call it GtkMenuCreator to make it more explicit it's creating the
GtkMenu? Also, it merits its own file(s).

  public:
-  // Delegate class that lets another class control the status of the menu.
   class Delegate {

[Evan Stade, 2011/04/20 22:52:10]

this should not be called Delegate. I think GtkMenuCreator::ModelExtras perhaps
makes more sense? I'm open to hearing your opinion here.

Previously the functions herein were part of MenuGtk::Delegate, but that's
probably just because it was the easiest place to tack them on.

    public:
+    virtual ~Delegate() {}
+
+    // Return true if we should override the "gtk-menu-images" system setting
+    // when showing image menu items for this menu.
+    virtual bool AlwaysShowIconForCmd(int command_id) const { return false; }
+
+    // Returns an icon for the menu item, if available.
+    virtual GtkWidget* GetImageForCommandId(int command_id) const;
+
+    static GtkWidget* GetDefaultImageForCommandId(int command_id);

[Evan Stade, 2011/04/20 22:52:10]

I think this belongs on MenuCreator.

+  };
+
+ protected:
+  explicit MenuCreator(Delegate* delegate);
+  virtual ~MenuCreator();
+
+  // Creates the menu items specified in |model| and places them into |menu|.
+  void BuildSubmenuFromModel(ui::MenuModel* model, GtkWidget* menu);
+
+  // Builds a GtkImageMenuItem.
+  GtkWidget* BuildMenuItemWithImage(const std::string& label,
+                                    const SkBitmap& icon) const;
+
+  GtkWidget* BuildMenuItemWithImage(const std::string& label,
+                                    GtkWidget* image) const;
+
+  GtkWidget* BuildMenuItemWithLabel(const std::string& label,
+                                    int command_id) const;
+
+  // Called to build one of our custom menu button items. Some menus we
+  // construct can't support this type (due to tight integration with our
+  // GtkCustomMenu); in that case always return NULL.
+  virtual GtkWidget* BuildButtonMenuItem(ui::ButtonMenuItemModel* model,
+                                         GtkWidget* menu) = 0;
+
+  // Called after every |menu_item| created. It's this method's responsibility
+  // to add the |menu_item| to |menu|, to hook up signals, etc.
+  virtual GtkWidget* AppendMenuItemToMenu(int index,
+                                          ui::MenuModel* model,
+                                          GtkWidget* menu_item,
+                                          GtkWidget* menu,
+                                          bool connect_to_activate) = 0;
+
+ private:
+  Delegate* delegate_;
+
+  // For some menu items, we want to show the accelerator, but not actually
+  // explicitly handle it. To this end we connect those menu items' accelerators
+  // to this group, but don't attach this group to any top level window.
+  GtkAccelGroup* dummy_accel_group_;
+};
+
+// MenuGtk both creates the menu.
+class MenuGtk : public MenuCreator {
+ public:
+  // Delegate class that lets another class control the status of the menu.
+  class Delegate : public MenuCreator::Delegate {

[Evan Stade, 2011/04/20 22:52:10]

I don't think MenuGtk::Delegate needs to be a subclass of MenuCreator::Delegate
(or GtkMenuCreator::ModelExtras if you take my above suggestions). Any client
that actually wants to implement those things should implement both interfaces.

And yes, this means we need to add another constructor that takes a delegate,
model, and modelextras.

+   public:
     virtual ~Delegate() { }
 
     // Called before a command is executed. This exists for the case where a
@@ -41,17 +100,8 @@ class MenuGtk {
     // the user clicks away from the menu.
     virtual void StoppedShowing() {}
 
-    // Return true if we should override the "gtk-menu-images" system setting
-    // when showing image menu items for this menu.
-    virtual bool AlwaysShowIconForCmd(int command_id) const { return false; }
-
     // Returns a tinted image used in button in a menu.
     virtual GtkIconSet* GetIconSetForId(int idr) { return NULL; }

[Evan Stade, 2011/04/20 22:52:10]

also belongs in ModelExtras

-
-    // Returns an icon for the menu item, if available.
-    virtual GtkWidget* GetImageForCommandId(int command_id) const;
-
-    static GtkWidget* GetDefaultImageForCommandId(int command_id);
   };
 
   MenuGtk(MenuGtk::Delegate* delegate, ui::MenuModel* model);
@@ -69,11 +119,6 @@ class MenuGtk {
                                           const std::string& label);
   GtkWidget* AppendSeparator();
   GtkWidget* AppendMenuItem(int command_id, GtkWidget* menu_item);
-  GtkWidget* AppendMenuItemToMenu(int index,
-                                  ui::MenuModel* model,
-                                  GtkWidget* menu_item,
-                                  GtkWidget* menu,
-                                  bool connect_to_activate);
 
   // Displays the menu near a widget, as if the widget were a menu bar.
   // Example: the wrench menu button.
@@ -123,26 +168,20 @@ class MenuGtk {
   void UpdateMenu();
 
  private:
-  // Builds a GtkImageMenuItem.
-  GtkWidget* BuildMenuItemWithImage(const std::string& label,
-                                    const SkBitmap& icon);
-
-  GtkWidget* BuildMenuItemWithImage(const std::string& label,
-                                    GtkWidget* image);
-
-  GtkWidget* BuildMenuItemWithLabel(const std::string& label,
-                                    int command_id);
-
   // A function that creates a GtkMenu from |model_|.
   void BuildMenuFromModel();
-  // Implementation of the above; called recursively.
-  void BuildSubmenuFromModel(ui::MenuModel* model, GtkWidget* menu);
-  // Builds a menu item with buttons in it from the data in the model.
-  GtkWidget* BuildButtonMenuItem(ui::ButtonMenuItemModel* model,
-                                 GtkWidget* menu);
 
   void ExecuteCommand(ui::MenuModel* model, int id);
 
+  // MenuCreator:
+  virtual GtkWidget* AppendMenuItemToMenu(int index,
+                                          ui::MenuModel* model,
+                                          GtkWidget* menu_item,
+                                          GtkWidget* menu,
+                                          bool connect_to_activate);
+  virtual GtkWidget* BuildButtonMenuItem(ui::ButtonMenuItemModel* model,
+                                         GtkWidget* menu);
+
   // Callback for when a menu item is clicked.
   CHROMEGTK_CALLBACK_0(MenuGtk, void, OnMenuItemActivated);
 
@@ -173,11 +212,6 @@ class MenuGtk {
   // menu (overriding the delegate as a controller).
   ui::MenuModel* model_;
 
-  // For some menu items, we want to show the accelerator, but not actually
-  // explicitly handle it. To this end we connect those menu items' accelerators
-  // to this group, but don't attach this group to any top level window.
-  GtkAccelGroup* dummy_accel_group_;
-
   // gtk_menu_popup() does not appear to take ownership of popup menus, so
   // MenuGtk explicitly manages the lifetime of the menu.
   GtkWidget* menu_;