MacViews: Fix duplicate definition of ExtensionKeyBindingRegistry::SetShortcutHandlingSuspended

chrome/browser/ui/views/extensions/extension_keybinding_registry_views.cc

Index: chrome/browser/ui/views/extensions/extension_keybinding_registry_views.cc
diff --git a/chrome/browser/ui/views/extensions/extension_keybinding_registry_views.cc b/chrome/browser/ui/views/extensions/extension_keybinding_registry_views.cc
index d0fb3daeee0fdc768d7c5254e98ee47fe8180de5..2a084e963d21fc3343bd15a6861451e87db70459 100644
--- a/chrome/browser/ui/views/extensions/extension_keybinding_registry_views.cc
+++ b/chrome/browser/ui/views/extensions/extension_keybinding_registry_views.cc
@@ -11,12 +11,6 @@
 #include "extensions/common/extension.h"
 #include "ui/views/focus/focus_manager.h"
 
-// static
-void extensions::ExtensionKeybindingRegistry::SetShortcutHandlingSuspended(
-    bool suspended) {
-  views::FocusManager::set_shortcut_handling_suspended(suspended);
-}
-
 ExtensionKeybindingRegistryViews::ExtensionKeybindingRegistryViews(
     Profile* profile,
     views::FocusManager* focus_manager,
@@ -26,9 +20,11 @@ ExtensionKeybindingRegistryViews::ExtensionKeybindingRegistryViews(
       profile_(profile),
       focus_manager_(focus_manager) {
   Init();
+  focus_manager->AddAcceleratorPreProcessor(this);

[Devlin, 2015/01/09 17:09:26]

This is somewhat less definitive than the old implementation, since if another
object is added as a pre-processor, it could process the accelerator before this
one can suppress it.  If we currently only have this as a pre-processor, could
we establish the pattern that pre-processors are only designed to suppress,
rather than handle, accelerators?  This would avoid racy behavior.

On that note, I'm not entirely sure that I like the pre-post processor
vernacular, since it (to me) implies that one does work before the processing of
the accelerator, and one does work after the processing - which isn't true in
either case (they both just have the option to "process" them).  If Scott feels
differently, go with his preference, but I think I prefer something more like
"AcceleratorSuppressor" which has the chance to suppress them (and only ever
consumes events) and "AcceleratorProcessor" (which actually takes an action on
the event) - it makes the purposes seem more clear to me.

 }
 
 ExtensionKeybindingRegistryViews::~ExtensionKeybindingRegistryViews() {
+  focus_manager_->RemoveAcceleratorProcessor(this);
   focus_manager_->UnregisterAccelerators(this);
 }
 
@@ -70,6 +66,18 @@ void ExtensionKeybindingRegistryViews::RemoveExtensionKeybindingImpl(
   focus_manager_->UnregisterAccelerator(accelerator, this);
 }
 
+bool ExtensionKeybindingRegistryViews::ProcessAccelerator(
+    const ui::Accelerator& accelerator) {
+  // Consume the accelerator if shortcut handling is suspended.
+  return IsShortcutHandlingSuspended();
+}
+
+ui::AcceleratorTarget*
+ExtensionKeybindingRegistryViews::GetTargetForAccelerator(
+    const ui::Accelerator& accelerator) const {
+  return nullptr;
+}
+
 bool ExtensionKeybindingRegistryViews::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
   std::string extension_id, command_name;