Allow WebContents key handling to supplant extension overrides of the bookmark shortcut

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/extensions/extension_service.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(
@@ skipping 26 matching lines @@
   if (!command_service->GetNamedCommands(
           extension->id(),
           extensions::CommandService::ACTIVE_ONLY,
           extensions::CommandService::REGULAR,
           &commands))
     return;
   extensions::CommandMap::const_iterator iter = commands.begin();
   for (; iter != commands.end(); ++iter) {
     if (!command_name.empty() && (iter->second.command_name() != command_name))
       continue;
     if (!IsAcceleratorRegistered(iter->second.accelerator())) {

[Peter Kasting, 2014/07/07 23:47:10]

Nit: Shorter:

    const ui::Accelerator& accelerator = iter->second.accelerator();
    if (!IsAcceleratorRegistered(accelerator)) {
      focus_manager_->RegisterAccelerator(
          accelerator, GetAcceleratorPriority(accelerator, extension), this);
    }

    AddEventTarget(accelerator, extension->id(), iter->second.command_name());

[Mike Wittman, 2014/07/08 19:29:01]

Done.

       focus_manager_->RegisterAccelerator(iter->second.accelerator(),
-                                          ui::AcceleratorManager::kHighPriority,
+                                          GetAcceleratorPriority(
+                                              iter->second.accelerator(),
+                                              extension),
                                           this);
     }
 
     AddEventTarget(iter->second.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::AcceleratorPressed(
     const ui::Accelerator& accelerator) {
+  std::string extension_id, command_name;
+  GetFirstTarget(accelerator, &extension_id, &command_name);
+  // Normal priority shortcuts must be handled via the standard browser command
+  // processing to be effective.

[Peter Kasting, 2014/07/07 23:47:10]

Nit: What does "to be effective" mean here?  This phrase seems somewhat
ambiguous to a reader; I suggest finding a clearer formulation.  (3 different
files)

[Mike Wittman, 2014/07/08 19:29:01]

Done.

+  if (GetAcceleratorPriorityById(accelerator,

[Peter Kasting, 2014/07/07 23:47:10]

Nit: Shorter:

  const ui::AcceleratorManager::HandlerPriority priority =
      GetAcceleratorPriorityById(accelerator, extension_id, browser_context());
  return (priority == ui::AcceleratorManager::kHighPriority) &&
      ExtensionKeybindingRegistry::NotifyEventTargets(accelerator);

[Mike Wittman, 2014/07/08 19:29:01]

Done.

+                                 extension_id,
+                                 browser_context()) ==
+      ui::AcceleratorManager::kNormalPriority)
+    return false;
+
   return ExtensionKeybindingRegistry::NotifyEventTargets(accelerator);
 }
 
 bool ExtensionKeybindingRegistryViews::CanHandleAccelerators() const {
   return true;
 }