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

chrome/browser/ui/extensions/accelerator_priority.cc

Index: chrome/browser/ui/extensions/accelerator_priority.cc
diff --git a/chrome/browser/ui/extensions/accelerator_priority.cc b/chrome/browser/ui/extensions/accelerator_priority.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3ae21d387155435155adb7a4604659621924343b
--- /dev/null
+++ b/chrome/browser/ui/extensions/accelerator_priority.cc
@@ -0,0 +1,35 @@
+// Copyright 2014 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/extensions/accelerator_priority.h"
+
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/ui/accelerator_utils.h"
+#include "chrome/common/extensions/manifest_handlers/ui_overrides_handler.h"
+#include "extensions/browser/extension_registry.h"
+#include "ui/base/accelerators/accelerator.h"
+
+ui::AcceleratorManager::HandlerPriority GetAcceleratorPriority(
+    const ui::Accelerator& accelerator,
+    const extensions::Extension* extension) {
+  // Extensions overriding the bookmark shortcut need normal priority to
+  // preserve the built-in processing order of the key and not override
+  // WebContents key handling.
+  if (accelerator ==
+      chrome::GetPrimaryChromeAcceleratorForCommandId(IDC_BOOKMARK_PAGE) &&
+      extensions::UIOverrides::RemovesBookmarkShortcut(extension))
+    return ui::AcceleratorManager::kNormalPriority;
+
+  return ui::AcceleratorManager::kHighPriority;

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

It seems surprising to me that IDC_BOOKMARK_PAGE is the only shortcut we want to
special-case here.  What about other extension-overridable shortcuts that we
normally allow web pages to handle?  Shouldn't we also be returning
kNormalPriority for them?  I'd expect that we'd want "normal priority" in
general, rather than high.

[Finnur, 2014/07/08 10:35:02]

The intent here is to allow extensions to override the bookmarking system as a
whole, and the shortcut Ctrl+D as part of that, while at the same time
preserving how the bookmarking shortcut works now (not break Ctrl+D handling in
web pages just because the bookmarking system is implemented by an extension).
This is currently the only shortcut that we allow an extension to steal (take
away even if already in use)-- and only if the extension requests the privilege
to override the bookmarking system.

This is not the same as a user assigning Ctrl+D to an extension action. That
shortcut would have high priority. I can (when not on vacation) :) look up why
we elected to use High priority for all extension shortcuts. We can always
revisit that decision, but it would be beyond the scope of this CL.

[Peter Kasting, 2014/07/08 19:30:36]

Cool.  This is helpful info.  Consider putting some or all of it into comments
here and/or in the header.

[Mike Wittman, 2014/07/08 22:15:21]

Added a comment in the header.

+}
+
+ui::AcceleratorManager::HandlerPriority GetAcceleratorPriorityById(
+    const ui::Accelerator& accelerator,
+    const std::string& extension_id,
+    content::BrowserContext* browser_context) {
+  return GetAcceleratorPriority(
+      accelerator,
+      extensions::ExtensionRegistry::Get(browser_context)->enabled_extensions().
+          GetByID(extension_id));
+}