summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/menus/accelerator.h14
-rw-r--r--chrome/browser/cocoa/toolbar_controller.h5
-rw-r--r--chrome/browser/cocoa/toolbar_controller.mm49
-rw-r--r--chrome/browser/cocoa/wrench_menu_controller.h7
-rw-r--r--chrome/browser/cocoa/wrench_menu_controller.mm37
-rw-r--r--chrome/browser/cocoa/wrench_menu_controller_unittest.mm2
-rw-r--r--chrome/browser/gtk/browser_toolbar_gtk.cc30
-rw-r--r--chrome/browser/gtk/browser_toolbar_gtk.h8
-rw-r--r--chrome/browser/views/toolbar_view.cc24
-rw-r--r--chrome/browser/views/toolbar_view.h12
-rw-r--r--chrome/browser/wrench_menu_model.cc172
-rw-r--r--chrome/browser/wrench_menu_model.h27
-rw-r--r--chrome/browser/wrench_menu_model_unittest.cc69
-rw-r--r--chrome/test/menu_model_test.h4
14 files changed, 217 insertions, 243 deletions
diff --git a/app/menus/accelerator.h b/app/menus/accelerator.h
index def6cf2..0509c14 100644
--- a/app/menus/accelerator.h
+++ b/app/menus/accelerator.h
@@ -68,6 +68,20 @@ class Accelerator {
int modifiers_;
};
+// Since acclerator code is one of the few things that can't be cross platform
+// in the chrome UI, separate out just the GetAcceleratorForCommandId() from
+// the menu delegates.
+class AcceleratorProvider {
+ public:
+ virtual ~AcceleratorProvider() {}
+
+ // Gets the accelerator for the specified command id. Returns true if the
+ // command id has a valid accelerator, false otherwise.
+ virtual bool GetAcceleratorForCommandId(
+ int command_id,
+ menus::Accelerator* accelerator) = 0;
+};
+
}
#endif // APP_MENUS_ACCELERATOR_H_
diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h
index 68b57bf..43420cc 100644
--- a/chrome/browser/cocoa/toolbar_controller.h
+++ b/chrome/browser/cocoa/toolbar_controller.h
@@ -28,7 +28,7 @@ class LocationBar;
class LocationBarViewMac;
@class MenuButton;
namespace ToolbarControllerInternal {
-class MenuDelegate;
+class WrenchAcceleratorDelegate;
class NotificationBridge;
} // namespace ToolbarControllerInternal
class Profile;
@@ -74,7 +74,8 @@ class WrenchMenuModel;
// Lazily-instantiated model and delegate for the menu on the
// wrench button. Once visible, it will be non-null, but will not
// reaped when the menu is hidden once it is initially shown.
- scoped_ptr<ToolbarControllerInternal::MenuDelegate> menuDelegate_;
+ scoped_ptr<ToolbarControllerInternal::WrenchAcceleratorDelegate>
+ acceleratorDelegate_;
scoped_ptr<WrenchMenuModel> wrenchMenuModel_;
// Used for monitoring the optional toolbar button prefs.
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index 57b0e38..b100b56 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -95,24 +95,9 @@ const CGFloat kWrenchMenuLeftPadding = 3.0;
namespace ToolbarControllerInternal {
-// A C++ delegate that handles enabling/disabling menu items and handling when
-// a menu command is chosen.
-class MenuDelegate : public menus::SimpleMenuModel::Delegate {
+// A C++ delegate that handles the accelerators in the wrench menu.
+class WrenchAcceleratorDelegate : public menus::AcceleratorProvider {
public:
- explicit MenuDelegate(Browser* browser)
- : browser_(browser) { }
-
- // Overridden from menus::SimpleMenuModel::Delegate
- virtual bool IsCommandIdChecked(int command_id) const {
- if (command_id == IDC_SHOW_BOOKMARK_BAR) {
- return browser_->profile()->GetPrefs()->GetBoolean(
- prefs::kShowBookmarkBar);
- }
- return false;
- }
- virtual bool IsCommandIdEnabled(int command_id) const {
- return browser_->command_updater()->IsCommandEnabled(command_id);
- }
virtual bool GetAcceleratorForCommandId(int command_id,
menus::Accelerator* accelerator_generic) {
// Downcast so that when the copy constructor is invoked below, the key
@@ -128,26 +113,6 @@ class MenuDelegate : public menus::SimpleMenuModel::Delegate {
}
return false;
}
- virtual void ExecuteCommand(int command_id) {
- browser_->ExecuteCommand(command_id);
- }
- virtual bool IsLabelForCommandIdDynamic(int command_id) const {
- // On Mac, switch between "Enter Full Screen" and "Exit Full Screen".
- return (command_id == IDC_FULLSCREEN);
- }
- virtual string16 GetLabelForCommandId(int command_id) const {
- if (command_id == IDC_FULLSCREEN) {
- int string_id = IDS_ENTER_FULLSCREEN_MAC; // Default to Enter.
- // Note: On startup, |window()| may be NULL.
- if (browser_->window() && browser_->window()->IsFullscreen())
- string_id = IDS_EXIT_FULLSCREEN_MAC;
- return l10n_util::GetStringUTF16(string_id);
- }
- return menus::SimpleMenuModel::Delegate::GetLabelForCommandId(command_id);
- }
-
- private:
- Browser* browser_;
};
// A class registered for C++ notifications. This is used to detect changes in
@@ -547,10 +512,12 @@ class NotificationBridge : public NotificationObserver {
- (void)installWrenchMenu {
if (wrenchMenuModel_.get())
return;
- menuDelegate_.reset(new ToolbarControllerInternal::MenuDelegate(browser_));
+ acceleratorDelegate_.reset(
+ new ToolbarControllerInternal::WrenchAcceleratorDelegate());
- wrenchMenuModel_.reset(new WrenchMenuModel(menuDelegate_.get(), browser_));
- [wrenchMenuController_ setModel:wrenchMenuModel_.get()];
+ wrenchMenuModel_.reset(new WrenchMenuModel(
+ acceleratorDelegate_.get(), browser_));
+ [wrenchMenuController_ setWrenchMenuModel:wrenchMenuModel_.get()];
[wrenchMenuController_ setUseWithPopUpButtonCell:YES];
[wrenchButton_ setAttachedMenu:[wrenchMenuController_ menu]];
}
@@ -578,8 +545,6 @@ class NotificationBridge : public NotificationObserver {
[overlayImage unlockFocus];
[[wrenchButton_ cell] setOverlayImage:overlayImage];
-
- [wrenchMenuController_ insertUpdateAvailableItem];
}
- (void)prefChanged:(std::string*)prefName {
diff --git a/chrome/browser/cocoa/wrench_menu_controller.h b/chrome/browser/cocoa/wrench_menu_controller.h
index 99f3743..65d32fc9 100644
--- a/chrome/browser/cocoa/wrench_menu_controller.h
+++ b/chrome/browser/cocoa/wrench_menu_controller.h
@@ -40,12 +40,16 @@ class ZoomLevelObserver;
IBOutlet NSButton* zoomMinus_;
IBOutlet NSButton* zoomFullScreen_;
+ WrenchMenuModel* wrench_model_;
+
scoped_ptr<WrenchMenuControllerInternal::ZoomLevelObserver> observer_;
}
// Designated initializer; called within the NIB.
- (id)init;
+- (void)setWrenchMenuModel:(WrenchMenuModel*)model;
+
// Used to dispatch commands from the Wrench menu. The custom items within the
// menu cannot be hooked up directly to First Responder because the window in
// which the controls reside is not the BrowserWindowController, but a
@@ -55,9 +59,6 @@ class ZoomLevelObserver;
// Returns the weak reference to the WrenchMenuModel.
- (WrenchMenuModel*)wrenchMenuModel;
-// Inserts the update available notification menu item.
-- (void)insertUpdateAvailableItem;
-
@end
////////////////////////////////////////////////////////////////////////////////
diff --git a/chrome/browser/cocoa/wrench_menu_controller.mm b/chrome/browser/cocoa/wrench_menu_controller.mm
index 4966157..11d40a2 100644
--- a/chrome/browser/cocoa/wrench_menu_controller.mm
+++ b/chrome/browser/cocoa/wrench_menu_controller.mm
@@ -63,6 +63,11 @@ class ZoomLevelObserver : public NotificationObserver {
return self;
}
+- (void)setWrenchMenuModel:(WrenchMenuModel*)model {
+ wrench_model_ = model;
+ [self setModel:model->menu_model()];
+}
+
- (void)addItemToMenu:(NSMenu*)menu
atIndex:(NSInteger)index
fromModel:(menus::MenuModel*)model
@@ -166,37 +171,7 @@ class ZoomLevelObserver : public NotificationObserver {
}
- (WrenchMenuModel*)wrenchMenuModel {
- return static_cast<WrenchMenuModel*>(model_);
-}
-
-// Inserts the update available notification menu item.
-- (void)insertUpdateAvailableItem {
- WrenchMenuModel* model = [self wrenchMenuModel];
- // Don't insert the item multiple times.
- if (!model || model->GetIndexOfCommandId(IDC_ABOUT) != -1)
- return;
-
- // Update the model manually because the model is static because other
- // platforms always have an About item.
- int index = model->GetIndexOfCommandId(IDC_OPTIONS) + 1;
- model->InsertItemAt(index, IDC_ABOUT,
- l10n_util::GetStringFUTF16(IDS_ABOUT,
- l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)));
-
- // The model does not broadcast change notifications to its delegate, so
- // insert the actual menu item ourselves.
- NSInteger menuIndex = [[self menu] indexOfItemWithTag:index];
- [self addItemToMenu:[self menu]
- atIndex:menuIndex
- fromModel:model
- modelIndex:index];
-
- // Since the tag of each menu item is the index within the model, they need
- // to be adjusted after insertion.
- for (NSInteger i = menuIndex + 1; i < [[self menu] numberOfItems]; ++i) {
- NSMenuItem* item = [[self menu] itemAtIndex:i];
- [item setTag:[item tag] + 1];
- }
+ return wrench_model_;
}
// Fit the localized strings into the Cut/Copy/Paste control, then resize the
diff --git a/chrome/browser/cocoa/wrench_menu_controller_unittest.mm b/chrome/browser/cocoa/wrench_menu_controller_unittest.mm
index 446bd80..d617afb 100644
--- a/chrome/browser/cocoa/wrench_menu_controller_unittest.mm
+++ b/chrome/browser/cocoa/wrench_menu_controller_unittest.mm
@@ -76,7 +76,7 @@ TEST_F(WrenchMenuControllerTest, DispatchSimple) {
// Set fake model to test dispatching.
EXPECT_CALL(fake_model_, ExecuteCommand(IDC_ZOOM_PLUS));
- [controller() setModel:&fake_model_];
+ [controller() setWrenchMenuModel:&fake_model_];
[controller() dispatchWrenchMenuCommand:button.get()];
}
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.cc b/chrome/browser/gtk/browser_toolbar_gtk.cc
index a28abed..2d8562a 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_toolbar_gtk.cc
@@ -227,7 +227,7 @@ void BrowserToolbarGtk::Init(Profile* profile,
gtk_container_add(GTK_CONTAINER(wrench_box), wrench_button);
gtk_box_pack_start(GTK_BOX(toolbar_), wrench_box, FALSE, FALSE, 4);
- wrench_menu_.reset(new MenuGtk(this, &wrench_menu_model_));
+ wrench_menu_.reset(new MenuGtk(this, wrench_menu_model_.menu_model()));
g_signal_connect(wrench_menu_->widget(), "show",
G_CALLBACK(OnWrenchMenuShowThunk), this);
@@ -333,33 +333,7 @@ GtkIconSet* BrowserToolbarGtk::GetIconSetForId(int idr) {
return theme_provider_->GetIconSetForId(idr);
}
-// menus::SimpleMenuModel::Delegate
-
-bool BrowserToolbarGtk::IsCommandIdEnabled(int id) const {
- return browser_->command_updater()->IsCommandEnabled(id);
-}
-
-bool BrowserToolbarGtk::IsCommandIdChecked(int id) const {
- if (!profile_)
- return false;
-
- EncodingMenuController controller;
- if (id == IDC_SHOW_BOOKMARK_BAR) {
- return profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
- } else if (controller.DoesCommandBelongToEncodingMenu(id)) {
- TabContents* tab_contents = browser_->GetSelectedTabContents();
- if (tab_contents) {
- return controller.IsItemChecked(profile_, tab_contents->encoding(),
- id);
- }
- }
-
- return false;
-}
-
-void BrowserToolbarGtk::ExecuteCommand(int id) {
- browser_->ExecuteCommand(id);
-}
+// menus::AcceleratorProvider
bool BrowserToolbarGtk::GetAcceleratorForCommandId(
int id,
diff --git a/chrome/browser/gtk/browser_toolbar_gtk.h b/chrome/browser/gtk/browser_toolbar_gtk.h
index 09f04f1..1018dc0 100644
--- a/chrome/browser/gtk/browser_toolbar_gtk.h
+++ b/chrome/browser/gtk/browser_toolbar_gtk.h
@@ -12,6 +12,7 @@
#include "app/active_window_watcher_x.h"
#include "app/gtk_signal.h"
#include "app/gtk_signal_registrar.h"
+#include "app/menus/accelerator.h"
#include "app/menus/simple_menu_model.h"
#include "app/throb_animation.h"
#include "base/scoped_ptr.h"
@@ -40,7 +41,7 @@ class ToolbarModel;
// View class that displays the GTK version of the toolbar and routes gtk
// events back to the Browser.
class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
- public menus::SimpleMenuModel::Delegate,
+ public menus::AcceleratorProvider,
public MenuGtk::Delegate,
public NotificationObserver,
public AnimationDelegate,
@@ -94,10 +95,7 @@ class BrowserToolbarGtk : public CommandUpdater::CommandObserver,
virtual void StoppedShowing();
virtual GtkIconSet* GetIconSetForId(int idr);
- // Overridden from menus::SimpleMenuModel::Delegate:
- virtual bool IsCommandIdEnabled(int id) const;
- virtual bool IsCommandIdChecked(int id) const;
- virtual void ExecuteCommand(int id);
+ // Overridden from menus::AcceleratorProvider:
virtual bool GetAcceleratorForCommandId(int id,
menus::Accelerator* accelerator);
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index 26f0768..252621f 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -110,7 +110,7 @@ void ToolbarView::Init(Profile* profile) {
browser_, BackForwardMenuModel::BACKWARD_MENU));
forward_menu_model_.reset(new BackForwardMenuModel(
browser_, BackForwardMenuModel::FORWARD_MENU));
- app_menu_model_.reset(new WrenchMenuModel(this, browser_));
+ wrench_menu_model_.reset(new WrenchMenuModel(this, browser_));
back_ = new views::ButtonDropDown(this, back_menu_model_.get());
back_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
@@ -248,7 +248,7 @@ void ToolbarView::RunMenu(views::View* source, const gfx::Point& /*pt*/) {
bool destroyed_flag = false;
destroyed_flag_ = &destroyed_flag;
wrench_menu_.reset(new WrenchMenu(browser_));
- wrench_menu_->Init(app_menu_model_.get());
+ wrench_menu_->Init(wrench_menu_model_->menu_model());
for (size_t i = 0; i < menu_listeners_.size(); ++i)
menu_listeners_[i]->OnMenuOpened();
@@ -344,21 +344,7 @@ void ToolbarView::Observe(NotificationType type,
}
////////////////////////////////////////////////////////////////////////////////
-// ToolbarView, menus::SimpleMenuModel::Delegate implementation:
-
-bool ToolbarView::IsCommandIdChecked(int command_id) const {
-#if defined(OS_CHROMEOS)
- if (command_id == IDC_TOGGLE_VERTICAL_TABS) {
- return browser_->UseVerticalTabs();
- }
-#endif
- return (command_id == IDC_SHOW_BOOKMARK_BAR) &&
- profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
-}
-
-bool ToolbarView::IsCommandIdEnabled(int command_id) const {
- return browser_->command_updater()->IsCommandEnabled(command_id);
-}
+// ToolbarView, menus::AcceleratorProvider implementation:
bool ToolbarView::GetAcceleratorForCommandId(int command_id,
menus::Accelerator* accelerator) {
@@ -380,10 +366,6 @@ bool ToolbarView::GetAcceleratorForCommandId(int command_id,
return GetWidget()->GetAccelerator(command_id, accelerator);
}
-void ToolbarView::ExecuteCommand(int command_id) {
- browser_->ExecuteCommand(command_id);
-}
-
////////////////////////////////////////////////////////////////////////////////
// ToolbarView, views::View overrides:
diff --git a/chrome/browser/views/toolbar_view.h b/chrome/browser/views/toolbar_view.h
index af73d60..08550c071 100644
--- a/chrome/browser/views/toolbar_view.h
+++ b/chrome/browser/views/toolbar_view.h
@@ -8,7 +8,7 @@
#include <vector>
-#include "app/menus/simple_menu_model.h"
+#include "app/menus/accelerator.h"
#include "app/slide_animation.h"
#include "base/scoped_ptr.h"
#include "chrome/browser/back_forward_menu_model.h"
@@ -27,11 +27,12 @@ class BrowserActionsContainer;
class Browser;
class Profile;
class WrenchMenu;
+class WrenchMenuModel;
// The Browser Window's toolbar.
class ToolbarView : public AccessibleToolbarView,
public views::ViewMenuDelegate,
- public menus::SimpleMenuModel::Delegate,
+ public menus::AcceleratorProvider,
public LocationBarView::Delegate,
public AnimationDelegate,
public NotificationObserver,
@@ -104,12 +105,9 @@ class ToolbarView : public AccessibleToolbarView,
const NotificationSource& source,
const NotificationDetails& details);
- // Overridden from menus::SimpleMenuModel::Delegate:
- virtual bool IsCommandIdChecked(int command_id) const;
- virtual bool IsCommandIdEnabled(int command_id) const;
+ // Overridden from menus::AcceleratorProvider:
virtual bool GetAcceleratorForCommandId(int command_id,
menus::Accelerator* accelerator);
- virtual void ExecuteCommand(int command_id);
// Overridden from views::View:
virtual gfx::Size GetPreferredSize();
@@ -185,7 +183,7 @@ class ToolbarView : public AccessibleToolbarView,
DisplayMode display_mode_;
// The contents of the app menu.
- scoped_ptr<menus::SimpleMenuModel> app_menu_model_;
+ scoped_ptr<WrenchMenuModel> wrench_menu_model_;
// Wrench menu.
scoped_ptr<WrenchMenu> wrench_menu_;
diff --git a/chrome/browser/wrench_menu_model.cc b/chrome/browser/wrench_menu_model.cc
index 031d0d9..b1d39d1 100644
--- a/chrome/browser/wrench_menu_model.cc
+++ b/chrome/browser/wrench_menu_model.cc
@@ -20,6 +20,7 @@
#include "chrome/browser/defaults.h"
#include "chrome/browser/encoding_menu_controller.h"
#include "chrome/browser/host_zoom_map.h"
+#include "chrome/browser/pref_service.h"
#include "chrome/browser/profile.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/sync_ui_util.h"
@@ -29,6 +30,7 @@
#include "chrome/common/notification_service.h"
#include "chrome/common/notification_source.h"
#include "chrome/common/notification_type.h"
+#include "chrome/common/pref_names.h"
#include "grit/chromium_strings.h"
#include "grit/generated_resources.h"
#include "grit/theme_resources.h"
@@ -37,6 +39,10 @@
#include <gtk/gtk.h>
#endif
+#if defined(OS_MACOSX)
+#include "chrome/browser/browser_window.h"
+#endif
+
////////////////////////////////////////////////////////////////////////////////
// EncodingMenuModel
@@ -163,10 +169,10 @@ void ToolsMenuModel::Build(Browser* browser) {
////////////////////////////////////////////////////////////////////////////////
// WrenchMenuModel
-WrenchMenuModel::WrenchMenuModel(menus::SimpleMenuModel::Delegate* delegate,
+WrenchMenuModel::WrenchMenuModel(menus::AcceleratorProvider* provider,
Browser* browser)
- : menus::SimpleMenuModel(delegate),
- delegate_(delegate),
+ : ALLOW_THIS_IN_INITIALIZER_LIST(model_(this)),
+ provider_(provider),
browser_(browser),
tabstrip_model_(browser_->tabstrip_model()) {
Build();
@@ -185,50 +191,64 @@ WrenchMenuModel::~WrenchMenuModel() {
tabstrip_model_->RemoveObserver(this);
}
-bool WrenchMenuModel::IsLabelDynamicAt(int index) const {
- return IsDynamicItem(index) || SimpleMenuModel::IsLabelDynamicAt(index);
+bool WrenchMenuModel::IsLabelForCommandIdDynamic(int command_id) const {
+ return command_id == IDC_ZOOM_PERCENT_DISPLAY ||
+ command_id == IDC_SYNC_BOOKMARKS ||
+#if defined(OS_MACOSX)
+ command_id == IDC_FULLSCREEN ||
+#endif
+ command_id == IDC_ABOUT;
}
-string16 WrenchMenuModel::GetLabelAt(int index) const {
- if (!IsDynamicItem(index))
- return SimpleMenuModel::GetLabelAt(index);
-
- int command_id = GetCommandIdAt(index);
-
+string16 WrenchMenuModel::GetLabelForCommandId(int command_id) const {
switch (command_id) {
case IDC_ABOUT:
return GetAboutEntryMenuLabel();
case IDC_SYNC_BOOKMARKS:
return GetSyncMenuLabel();
+ case IDC_ZOOM_PERCENT_DISPLAY:
+ return zoom_label_;
+#if defined(OS_MACOSX)
+ case IDC_FULLSCREEN: {
+ int string_id = IDS_ENTER_FULLSCREEN_MAC; // Default to Enter.
+ // Note: On startup, |window()| may be NULL.
+ if (browser_->window() && browser_->window()->IsFullscreen())
+ string_id = IDS_EXIT_FULLSCREEN_MAC;
+ return l10n_util::GetStringUTF16(string_id);
+ }
+#endif
default:
NOTREACHED();
return string16();
}
}
-bool WrenchMenuModel::GetIconAt(int index, SkBitmap* icon) const {
- if (GetCommandIdAt(index) == IDC_ABOUT &&
- Singleton<UpgradeDetector>::get()->notify_upgrade()) {
- // Show the exclamation point next to the menu item.
- ResourceBundle& rb = ResourceBundle::GetSharedInstance();
- *icon = *rb.GetBitmapNamed(IDR_UPDATE_AVAILABLE);
- return true;
- }
- return false;
+void WrenchMenuModel::ExecuteCommand(int command_id) {
+ browser_->ExecuteCommand(command_id);
}
-bool WrenchMenuModel::IsLabelForCommandIdDynamic(int command_id) const {
- return command_id == IDC_ZOOM_PERCENT_DISPLAY;
+bool WrenchMenuModel::IsCommandIdChecked(int command_id) const {
+#if defined(OS_CHROMEOS)
+ if (command_id == IDC_TOGGLE_VERTICAL_TABS) {
+ return browser_->UseVerticalTabs();
+ }
+#endif
+
+ if (command_id == IDC_SHOW_BOOKMARK_BAR) {
+ return browser_->profile()->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
+ }
+
+ return false;
}
-string16 WrenchMenuModel::GetLabelForCommandId(int command_id) const {
- DCHECK_EQ(IDC_ZOOM_PERCENT_DISPLAY, command_id);
- return zoom_label_;
+bool WrenchMenuModel::IsCommandIdEnabled(int command_id) const {
+ return browser_->command_updater()->IsCommandEnabled(command_id);
}
-void WrenchMenuModel::ExecuteCommand(int command_id) {
- if (delegate_)
- delegate_->ExecuteCommand(command_id);
+bool WrenchMenuModel::GetAcceleratorForCommandId(
+ int command_id,
+ menus::Accelerator* accelerator) {
+ return provider_->GetAcceleratorForCommandId(command_id, accelerator);
}
void WrenchMenuModel::TabSelectedAt(TabContents* old_contents,
@@ -258,12 +278,17 @@ void WrenchMenuModel::Observe(NotificationType type,
UpdateZoomControls();
}
+// For testing.
+WrenchMenuModel::WrenchMenuModel() : model_(NULL) {
+}
+
void WrenchMenuModel::Build() {
- AddItemWithStringId(IDC_NEW_TAB, IDS_NEW_TAB);
- AddItemWithStringId(IDC_NEW_WINDOW, IDS_NEW_WINDOW);
- AddItemWithStringId(IDC_NEW_INCOGNITO_WINDOW, IDS_NEW_INCOGNITO_WINDOW);
+ model_.AddItemWithStringId(IDC_NEW_TAB, IDS_NEW_TAB);
+ model_.AddItemWithStringId(IDC_NEW_WINDOW, IDS_NEW_WINDOW);
+ model_.AddItemWithStringId(IDC_NEW_INCOGNITO_WINDOW,
+ IDS_NEW_INCOGNITO_WINDOW);
- AddSeparator();
+ model_.AddSeparator();
#if defined(OS_MACOSX) || (defined(OS_LINUX) && !defined(TOOLKIT_VIEWS))
// WARNING: Mac does not use the ButtonMenuItemModel, but instead defines the
// layout for this menu item in Toolbar.xib. It does, however, use the
@@ -272,13 +297,13 @@ void WrenchMenuModel::Build() {
edit_menu_item_model_->AddGroupItemWithStringId(IDC_CUT, IDS_CUT);
edit_menu_item_model_->AddGroupItemWithStringId(IDC_COPY, IDS_COPY);
edit_menu_item_model_->AddGroupItemWithStringId(IDC_PASTE, IDS_PASTE);
- AddButtonItem(IDC_EDIT_MENU, edit_menu_item_model_.get());
+ model_.AddButtonItem(IDC_EDIT_MENU, edit_menu_item_model_.get());
#else
// TODO(port): Move to the above.
CreateCutCopyPaste();
#endif
- AddSeparator();
+ model_.AddSeparator();
#if defined(OS_MACOSX) || (defined(OS_LINUX) && !defined(TOOLKIT_VIEWS))
// WARNING: See above comment.
zoom_menu_item_model_.reset(
@@ -292,68 +317,79 @@ void WrenchMenuModel::Build() {
zoom_menu_item_model_->AddSpace();
zoom_menu_item_model_->AddItemWithImage(
IDC_FULLSCREEN, IDR_FULLSCREEN_MENU_BUTTON);
- AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
+ model_.AddButtonItem(IDC_ZOOM_MENU, zoom_menu_item_model_.get());
#else
// TODO(port): Move to the above.
CreateZoomFullscreen();
#endif
- AddSeparator();
- AddItemWithStringId(IDC_SAVE_PAGE, IDS_SAVE_PAGE);
- AddItemWithStringId(IDC_FIND, IDS_FIND);
- AddItemWithStringId(IDC_PRINT, IDS_PRINT);
+ model_.AddSeparator();
+ model_.AddItemWithStringId(IDC_SAVE_PAGE, IDS_SAVE_PAGE);
+ model_.AddItemWithStringId(IDC_FIND, IDS_FIND);
+ model_.AddItemWithStringId(IDC_PRINT, IDS_PRINT);
- tools_menu_model_.reset(new ToolsMenuModel(delegate(), browser_));
- AddSubMenuWithStringId(IDC_ZOOM_MENU, IDS_TOOLS_MENU,
+ tools_menu_model_.reset(new ToolsMenuModel(this, browser_));
+ model_.AddSubMenuWithStringId(IDC_ZOOM_MENU, IDS_TOOLS_MENU,
tools_menu_model_.get());
- AddSeparator();
+ model_.AddSeparator();
#if defined(ENABLE_REMOTING)
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableRemoting)) {
- AddItem(IDC_REMOTING_SETUP,
+ model_.AddItem(IDC_REMOTING_SETUP,
l10n_util::GetStringUTF16(IDS_REMOTING_SETUP_LABEL));
}
#endif
- AddItemWithStringId(IDC_SHOW_BOOKMARK_MANAGER, IDS_BOOKMARK_MANAGER);
- AddItemWithStringId(IDC_SHOW_HISTORY, IDS_SHOW_HISTORY);
- AddItemWithStringId(IDC_SHOW_DOWNLOADS, IDS_SHOW_DOWNLOADS);
- AddSeparator();
+ model_.AddItemWithStringId(IDC_SHOW_BOOKMARK_MANAGER, IDS_BOOKMARK_MANAGER);
+ model_.AddItemWithStringId(IDC_SHOW_HISTORY, IDS_SHOW_HISTORY);
+ model_.AddItemWithStringId(IDC_SHOW_DOWNLOADS, IDS_SHOW_DOWNLOADS);
+ model_.AddSeparator();
#if defined(OS_MACOSX)
- AddItemWithStringId(IDC_OPTIONS, IDS_PREFERENCES_MAC);
+ model_.AddItemWithStringId(IDC_OPTIONS, IDS_PREFERENCES_MAC);
#elif defined(OS_LINUX)
GtkStockItem stock_item;
if (gtk_stock_lookup(GTK_STOCK_PREFERENCES, &stock_item)) {
const char16 kUnderscore[] = { '_', 0 };
string16 preferences;
RemoveChars(UTF8ToUTF16(stock_item.label), kUnderscore, &preferences);
- AddItem(IDC_OPTIONS, preferences);
+ model_.AddItem(IDC_OPTIONS, preferences);
+ } else {
+ model_.AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
}
#else
- AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
+ model_.AddItemWithStringId(IDC_OPTIONS, IDS_OPTIONS);
#endif
#if defined(OS_CHROMEOS)
- AddCheckItemWithStringId(IDC_TOGGLE_VERTICAL_TABS,
+ model_.AddCheckItemWithStringId(IDC_TOGGLE_VERTICAL_TABS,
IDS_TAB_CXMENU_USE_VERTICAL_TABS);
#endif
+ // TODO(erg): This entire section needs to be reworked and is out of scope of
+ // the first cleanup patch I'm doing. Part 1 (crbug.com/47320) is moving most
+ // of the logic from each platform specific UI code into the model here. Part
+ // 2 (crbug.com/46221) is normalizing the about menu item/hidden update menu
+ // item behaviour across the three platforms.
+
// On Mac, there is no About item unless it is replaced with the update
// available notification.
if (browser_defaults::kShowAboutMenuItem ||
Singleton<UpgradeDetector>::get()->notify_upgrade()) {
- AddItem(IDC_ABOUT,
+ model_.AddItem(IDC_ABOUT,
l10n_util::GetStringFUTF16(
IDS_ABOUT,
l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)));
+ // ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+ // model_.SetIcon(model_.GetIndexOfCommandId(IDC_ABOUT),
+ // *rb.GetBitmapNamed(IDR_UPDATE_AVAILABLE));
}
- AddItemWithStringId(IDC_HELP_PAGE, IDS_HELP_PAGE);
+ model_.AddItemWithStringId(IDC_HELP_PAGE, IDS_HELP_PAGE);
if (browser_defaults::kShowExitMenuItem) {
- AddSeparator();
+ model_.AddSeparator();
#if defined(OS_CHROMEOS)
- AddItemWithStringId(IDC_EXIT, IDS_SIGN_OUT);
+ model_.AddItemWithStringId(IDC_EXIT, IDS_SIGN_OUT);
#else
- AddItemWithStringId(IDC_EXIT, IDS_EXIT);
+ model_.AddItemWithStringId(IDC_EXIT, IDS_EXIT);
#endif
}
}
@@ -361,17 +397,17 @@ void WrenchMenuModel::Build() {
void WrenchMenuModel::CreateCutCopyPaste() {
// WARNING: views/wrench_menu assumes these items are added in this order. If
// you change the order you'll need to update wrench_menu as well.
- AddItemWithStringId(IDC_CUT, IDS_CUT);
- AddItemWithStringId(IDC_COPY, IDS_COPY);
- AddItemWithStringId(IDC_PASTE, IDS_PASTE);
+ model_.AddItemWithStringId(IDC_CUT, IDS_CUT);
+ model_.AddItemWithStringId(IDC_COPY, IDS_COPY);
+ model_.AddItemWithStringId(IDC_PASTE, IDS_PASTE);
}
void WrenchMenuModel::CreateZoomFullscreen() {
// WARNING: views/wrench_menu assumes these items are added in this order. If
// you change the order you'll need to update wrench_menu as well.
- AddItemWithStringId(IDC_ZOOM_MINUS, IDS_ZOOM_MINUS);
- AddItemWithStringId(IDC_ZOOM_PLUS, IDS_ZOOM_PLUS);
- AddItemWithStringId(IDC_FULLSCREEN, IDS_FULLSCREEN);
+ model_.AddItemWithStringId(IDC_ZOOM_MINUS, IDS_ZOOM_MINUS);
+ model_.AddItemWithStringId(IDC_ZOOM_PLUS, IDS_ZOOM_PLUS);
+ model_.AddItemWithStringId(IDC_FULLSCREEN, IDS_FULLSCREEN);
}
void WrenchMenuModel::UpdateZoomControls() {
@@ -415,15 +451,3 @@ string16 WrenchMenuModel::GetAboutEntryMenuLabel() const {
return l10n_util::GetStringFUTF16(
IDS_ABOUT, l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
}
-
-bool WrenchMenuModel::IsDynamicItem(int index) const {
- int command_id = GetCommandIdAt(index);
- return command_id == IDC_SYNC_BOOKMARKS ||
- command_id == IDC_ABOUT;
-}
-
-bool WrenchMenuModel::IsCommandIdEnabled(int command_id) const {
- if (delegate_)
- return delegate_->IsCommandIdEnabled(command_id);
- return true;
-}
diff --git a/chrome/browser/wrench_menu_model.h b/chrome/browser/wrench_menu_model.h
index 3bf6c24..ba45a03 100644
--- a/chrome/browser/wrench_menu_model.h
+++ b/chrome/browser/wrench_menu_model.h
@@ -6,6 +6,7 @@
#define CHROME_BROWSER_WRENCH_MENU_MODEL_H_
#pragma once
+#include "app/menus/accelerator.h"
#include "app/menus/button_menu_item_model.h"
#include "app/menus/simple_menu_model.h"
#include "base/scoped_ptr.h"
@@ -67,26 +68,23 @@ class ToolsMenuModel : public menus::SimpleMenuModel {
};
// A menu model that builds the contents of the wrench menu.
-class WrenchMenuModel : public menus::SimpleMenuModel,
+class WrenchMenuModel : public menus::SimpleMenuModel::Delegate,
public menus::ButtonMenuItemModel::Delegate,
public TabStripModelObserver,
public NotificationObserver {
public:
- WrenchMenuModel(menus::SimpleMenuModel::Delegate* delegate,
- Browser* browser);
+ WrenchMenuModel(menus::AcceleratorProvider* provider, Browser* browser);
virtual ~WrenchMenuModel();
- // Overridden from menus::SimpleMenuModel:
- virtual bool IsLabelDynamicAt(int index) const;
- virtual string16 GetLabelAt(int index) const;
- virtual bool HasIcons() const { return true; }
- virtual bool GetIconAt(int index, SkBitmap* icon) const;
-
- // Overridden from menus::ButtonMenuItemModel::Delegate:
+ // Overridden for both ButtonMenuItemModel::Delegate and SimpleMenuModel:
virtual bool IsLabelForCommandIdDynamic(int command_id) const;
virtual string16 GetLabelForCommandId(int command_id) const;
virtual void ExecuteCommand(int command_id);
+ virtual bool IsCommandIdChecked(int command_id) const;
virtual bool IsCommandIdEnabled(int command_id) const;
+ virtual bool GetAcceleratorForCommandId(
+ int command_id,
+ menus::Accelerator* accelerator);
// Overridden from TabStripModelObserver:
virtual void TabSelectedAt(TabContents* old_contents,
@@ -104,6 +102,7 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
// Getters.
Browser* browser() const { return browser_; }
+ menus::SimpleMenuModel* menu_model() { return &model_; }
// Calculates |zoom_label_| in response to a zoom change.
void UpdateZoomControls();
@@ -111,7 +110,7 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
private:
// Testing constructor used for mocking.
friend class ::MockWrenchMenuModel;
- WrenchMenuModel() : menus::SimpleMenuModel(NULL) {}
+ WrenchMenuModel();
void Build();
@@ -125,7 +124,9 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
string16 GetSyncMenuLabel() const;
string16 GetAboutEntryMenuLabel() const;
- bool IsDynamicItem(int index) const;
+
+ // Our menu, for which we are the delegate.
+ menus::SimpleMenuModel model_;
// Models for the special menu items with buttons.
scoped_ptr<menus::ButtonMenuItemModel> edit_menu_item_model_;
@@ -137,7 +138,7 @@ class WrenchMenuModel : public menus::SimpleMenuModel,
// Tools menu.
scoped_ptr<ToolsMenuModel> tools_menu_model_;
- menus::SimpleMenuModel::Delegate* delegate_; // weak
+ menus::AcceleratorProvider* provider_;
Browser* browser_; // weak
TabStripModel* tabstrip_model_; // weak
diff --git a/chrome/browser/wrench_menu_model_unittest.cc b/chrome/browser/wrench_menu_model_unittest.cc
index 2b50bb0..d38147e 100644
--- a/chrome/browser/wrench_menu_model_unittest.cc
+++ b/chrome/browser/wrench_menu_model_unittest.cc
@@ -12,12 +12,51 @@
#include "testing/gtest/include/gtest/gtest.h"
class WrenchMenuModelTest : public BrowserWithTestWindowTest,
- public MenuModelTest {
+ public menus::AcceleratorProvider {
+ public:
+ // Don't handle accelerators.
+ virtual bool GetAcceleratorForCommandId(
+ int command_id,
+ menus::Accelerator* accelerator) { return false; }
+};
+
+// Copies parts of MenuModelTest::Delegate and combines them with the
+// WrenchMenuModel since WrenchMenuModel is now a SimpleMenuModel::Delegate and
+// not derived from SimpleMenuModel.
+class TestWrenchMenuModel : public WrenchMenuModel {
+ public:
+ TestWrenchMenuModel(menus::AcceleratorProvider* provider,
+ Browser* browser)
+ : WrenchMenuModel(provider, browser),
+ execute_count_(0),
+ checked_count_(0),
+ enable_count_(0) {
+ }
+
+ // Testing overrides to menus::SimpleMenuModel::Delegate:
+ virtual bool IsCommandIdChecked(int command_id) const {
+ bool val = WrenchMenuModel::IsCommandIdChecked(command_id);
+ if (val)
+ checked_count_++;
+ return val;
+ }
+
+ virtual bool IsCommandIdEnabled(int command_id) const {
+ ++enable_count_;
+ return true;
+ }
+
+ virtual void ExecuteCommand(int command_id) { ++execute_count_; }
+
+ int execute_count_;
+ mutable int checked_count_;
+ mutable int enable_count_;
};
TEST_F(WrenchMenuModelTest, Basics) {
- WrenchMenuModel model(&delegate_, browser());
- int itemCount = model.GetItemCount();
+ TestWrenchMenuModel wrench(this, browser());
+ menus::SimpleMenuModel* model = wrench.menu_model();
+ int itemCount = model->GetItemCount();
// Verify it has items. The number varies by platform, so we don't check
// the exact number.
@@ -26,34 +65,34 @@ TEST_F(WrenchMenuModelTest, Basics) {
// Execute a couple of the items and make sure it gets back to our delegate.
// We can't use CountEnabledExecutable() here because the encoding menu's
// delegate is internal, it doesn't use the one we pass in.
- model.ActivatedAt(0);
- EXPECT_TRUE(model.IsEnabledAt(0));
+ model->ActivatedAt(0);
+ EXPECT_TRUE(model->IsEnabledAt(0));
// Make sure to use the index that is not separator in all configurations.
- model.ActivatedAt(2);
- EXPECT_TRUE(model.IsEnabledAt(2));
- EXPECT_EQ(delegate_.execute_count_, 2);
- EXPECT_EQ(delegate_.enable_count_, 2);
+ model->ActivatedAt(2);
+ EXPECT_TRUE(model->IsEnabledAt(2));
+ EXPECT_EQ(wrench.execute_count_, 2);
+ EXPECT_EQ(wrench.enable_count_, 2);
- delegate_.execute_count_ = 0;
- delegate_.enable_count_ = 0;
+ wrench.execute_count_ = 0;
+ wrench.enable_count_ = 0;
// Choose something from the tools submenu and make sure it makes it back to
// the delegate as well. Use the first submenu as the tools one.
int toolsModelIndex = -1;
for (int i = 0; i < itemCount; ++i) {
- if (model.GetTypeAt(i) == menus::MenuModel::TYPE_SUBMENU) {
+ if (model->GetTypeAt(i) == menus::MenuModel::TYPE_SUBMENU) {
toolsModelIndex = i;
break;
}
}
EXPECT_GT(toolsModelIndex, -1);
- menus::MenuModel* toolsModel = model.GetSubmenuModelAt(toolsModelIndex);
+ menus::MenuModel* toolsModel = model->GetSubmenuModelAt(toolsModelIndex);
EXPECT_TRUE(toolsModel);
EXPECT_GT(toolsModel->GetItemCount(), 2);
toolsModel->ActivatedAt(2);
EXPECT_TRUE(toolsModel->IsEnabledAt(2));
- EXPECT_EQ(delegate_.execute_count_, 1);
- EXPECT_EQ(delegate_.enable_count_, 1);
+ EXPECT_EQ(wrench.execute_count_, 1);
+ EXPECT_EQ(wrench.enable_count_, 1);
}
class EncodingMenuModelTest : public BrowserWithTestWindowTest,
diff --git a/chrome/test/menu_model_test.h b/chrome/test/menu_model_test.h
index a87993b4..92c390c 100644
--- a/chrome/test/menu_model_test.h
+++ b/chrome/test/menu_model_test.h
@@ -6,6 +6,7 @@
#define CHROME_TEST_MENU_MODEL_TEST_H_
#pragma once
+#include "app/menus/accelerator.h"
#include "app/menus/simple_menu_model.h"
// A mix-in class to be used in addition to something that derrives from
@@ -18,7 +19,8 @@ class MenuModelTest {
protected:
// A menu delegate that counts the number of times certain things are called
// to make sure things are hooked up properly.
- class Delegate : public menus::SimpleMenuModel::Delegate {
+ class Delegate : public menus::SimpleMenuModel::Delegate,
+ public menus::AcceleratorProvider {
public:
Delegate() : execute_count_(0), enable_count_(0) { }
generated by cgit v1.1 at 2025-01-18 18:39:59 +0000