MacViews: Fix duplicate definition of ExtensionKeyBindingRegistry::SetShortcutHandlingSuspended

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/extensions/extension_keybinding_registry_views.h"
 
 #include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/extension_keybinding_registry.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/extensions/accelerator_priority.h"
 #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,
     ExtensionFilter extension_filter,
     Delegate* delegate)
     : ExtensionKeybindingRegistry(profile, extension_filter, delegate),
       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);
 }
 
 void ExtensionKeybindingRegistryViews::AddExtensionKeybinding(
     const extensions::Extension* extension,
     const std::string& command_name) {
   // This object only handles named commands, not browser/page actions.
   if (ShouldIgnoreCommand(command_name))
     return;
 
@@ skipping 21 matching lines @@
     AddEventTarget(accelerator, extension->id(), iter->second.command_name());
   }
 }
 
 void ExtensionKeybindingRegistryViews::RemoveExtensionKeybindingImpl(
     const ui::Accelerator& accelerator,
     const std::string& command_name) {
   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;
   GetFirstTarget(accelerator, &extension_id, &command_name);
   const ui::AcceleratorManager::HandlerPriority priority =
       GetAcceleratorPriorityById(accelerator, extension_id, browser_context());
   // Normal priority shortcuts must be handled via standard browser commands to
   // be processed at the proper time.
   return (priority == ui::AcceleratorManager::kHighPriority) &&
       ExtensionKeybindingRegistry::NotifyEventTargets(accelerator);
 }
 
 bool ExtensionKeybindingRegistryViews::CanHandleAccelerators() const {
   return true;
 }