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

chrome/browser/ui/gtk/global_menu_bar.h

Index: chrome/browser/ui/gtk/global_menu_bar.h
diff --git a/chrome/browser/ui/gtk/global_menu_bar.h b/chrome/browser/ui/gtk/global_menu_bar.h
index 5ff88e7e068c46d27f3189682708f09693d12f2e..938816d8ffa74bfb48c419e96ac04954580e96fd 100644
--- a/chrome/browser/ui/gtk/global_menu_bar.h
+++ b/chrome/browser/ui/gtk/global_menu_bar.h
@@ -8,12 +8,15 @@
 #include <map>
 
 #include "chrome/browser/command_updater.h"
+#include "chrome/browser/ui/gtk/global_history_menu.h"
+#include "chrome/browser/ui/gtk/menu_gtk.h"
+#include "chrome/browser/ui/gtk/owned_widget_gtk.h"
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
+#include "ui/base/models/simple_menu_model.h"
 #include "ui/base/gtk/gtk_signal.h"
 
 class Browser;
-class BrowserWindowGtk;
 struct GlobalMenuBarCommand;
 
 typedef struct _GtkAccelGroup GtkAccelGroup;
@@ -28,20 +31,36 @@ typedef struct _GtkWidget GtkWidget;
 // bar itself is visible, so we insert a GtkMenuBar into the window hierarchy
 // and set it to be invisible.
 class GlobalMenuBar : public CommandUpdater::CommandObserver,
-                      public NotificationObserver {
+                      public NotificationObserver,
+                      public MenuCreator,

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

do you think it would work to make a separate class called GlobaMenuGtk : public
MenuCreator to encapsulate AppendMenuItemToMenu, BuildGtkMenuFrom, and
BuildMenuItem logic? The new class would be a cleaner parallel to MenuGtk.

It would fix the broken encapsulation of GlobalHistoryMenu, and also the fact
that this class is both a MenuCreator and MenuCreator::Delegate (which seems
very fishy).

[Elliot Glaysher, 2011/04/25 18:24:47]

At this point, we're close to looping back to how I did things in the first
place. I've addressed most of your comments just to show what this would look
like, but I disagree that this approach is better.

I've added a lot of complexity going with this OO approach and what was a very
simple, table driven piece of code is now really complex. :(

+                      public MenuCreator::Delegate,
+                      public ui::SimpleMenuModel::Delegate {
  public:
-  GlobalMenuBar(Browser* browser, BrowserWindowGtk* window);
+  // Command IDs under 4000 are reserved for us.
+  static const int TAG_NORMAL = 0;
+  static const int TAG_MOST_VISITED = 1;
+  static const int TAG_RECENTLY_CLOSED = 2;
+  static const int TAG_MOST_VISITED_HEADER = 3;
+  static const int TAG_RECENTLY_CLOSED_HEADER = 4;
+
+  explicit GlobalMenuBar(Browser* browser);
   virtual ~GlobalMenuBar();
 
-  GtkWidget* widget() { return menu_bar_; }
+  GtkWidget* widget() { return menu_bar_.get(); }
 
  private:
-  typedef std::map<int, GtkWidget*> IDMenuItemMap;
+  typedef std::map<int, GtkWidget*> CommandIDMenuItemMap;
 
   // Helper function that builds the data.
-  void BuildGtkMenuFrom(int menu_str_id,
-                        std::map<int, GtkWidget*>* id_to_menu_item,
-                        GlobalMenuBarCommand* commands);
+  GtkWidget* BuildGtkMenuFrom(int menu_str_id,
+                              ui::MenuModel* model);
+
+  // Builds an individual menu item.
+  GtkWidget* BuildMenuItem(int string_id,
+                           int command_id,
+                           int tag_id,
+                           std::map<int, GtkWidget*>* id_to_menu_item,
+                           GtkWidget* menu_to_add_to);
 
   // CommandUpdater::CommandObserver:
   virtual void EnabledStateChangedForCommand(int id, bool enabled);
@@ -51,24 +70,43 @@ class GlobalMenuBar : public CommandUpdater::CommandObserver,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
+  // MenuCreator:
+  virtual GtkWidget* BuildButtonMenuItem(ui::ButtonMenuItemModel* model,
+                                         GtkWidget* menu);
+  virtual GtkWidget* AppendMenuItemToMenu(int index,
+                                          ui::MenuModel* model,
+                                          GtkWidget* menu_item,
+                                          GtkWidget* menu,
+                                          bool connect_to_activate);
+
+  // SimpleMenuModel::Delegate:
+  //
+  // We only implement enough of this interface to allow us to get accelerators
+  // into MenuCreator. We don't rely on the command dispatch capabilities.
+  virtual bool IsCommandIdChecked(int command_id) const;
+  virtual bool IsCommandIdEnabled(int command_id) const;
+  virtual bool GetAcceleratorForCommandId(
+      int command_id,
+      ui::Accelerator* accelerator);
+  virtual void ExecuteCommand(int command_id);
+
   CHROMEGTK_CALLBACK_0(GlobalMenuBar, void, OnItemActivated);
 
   Browser* browser_;
-  BrowserWindowGtk* browser_window_;
+  Profile* profile_;
 
   NotificationRegistrar registrar_;
 
   // Our menu bar widget.
-  GtkWidget* menu_bar_;
+  OwnedWidgetGtk menu_bar_;
 
-  // 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_;
+  // Listens to the TabRestoreService and the HistoryService and keeps the
+  // history menu fresh.
+  GlobalHistoryMenu history_menu_;
 
   // A mapping from command ids to GtkMenuItem objects. We use this to update
-  // the enable state since we are a .
-  IDMenuItemMap id_to_menu_item_;
+  // the command enable state.
+  CommandIDMenuItemMap id_to_menu_item_;
 
   // gtk_check_menu_item_set_active() will call the "activate" signal. We need
   // to block this activation while we change the checked state.