AcceleratorProvider::GetAcceleratorForCommandId (and related methods: SimpleMenuModel::Delegate::GetAcceleratorForCommandId and ButtonMenuItemModel::Delegate::GetAcceleratorForCommandId) has a number of cascading issues which I'd like to fix up (not urgent).
These methods have ~100 overrides between them so it is a non-trivial cleanup.
1. They are non-const. There is no reason for them to be non-const, as they purely retrieve information.
2. Nearly every. single. override. of SMM and BMIM's GetAcceleratorForCommandId just returns false. We could delete almost all of the overrides if SMM and BMIM provided a default implementation. (Returning false seems like a good default in this case; it just means you don't want any accelerators in your menu.)
3. SimpleMenuModel::Delegate and ButtonMenuItemModel::Delegate, by providing GetAcceleratorForCommandId, essentially implement the AcceleratorProvider interface, yet they are not subclasses of AcceleratorProvider.
4. RenderViewContextMenuBase declares a pure virtual override of GetAcceleratorForCommandId ("override = 0"). This can just be deleted.
I'm going to go ahead and fix up these issues.
Comment 1 by mgiuca@chromium.org
, Jul 12 2016