Continue key propagation for extensions without any listeners to chrome.commands.onCommand.

chrome/browser/extensions/extension_keybinding_registry.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/extensions/extension_keybinding_registry.h"
 
 #include "base/values.h"
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/command.h"
 #include "content/public/browser/browser_context.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/manifest_constants.h"
 
+namespace {
+const char kCommandsOnCommandEventName[] = "commands.onCommand";

[Finnur, 2014/08/28 11:15:39]

nit: Why not simply kOnCommandEventName or kCommandHandlerEvent?

[David Tseng, 2014/08/28 23:37:11]

Done. (Was trying to follow the "new" format generated from idl's
namespace::kFooEventName).

[Finnur, 2014/08/29 12:17:00]

Do we do that elsewhere?

[David Tseng, 2014/08/29 17:18:45]

Not sure what you mean; an example is location::OnLocationError::kEventName
which gets generated from an idl.

In this instance, I'm fine with keeping this constant in an empty namespace. The
real fix would be to transition the json api declaration into idl.

+}  // namespace
+
 namespace extensions {
 
 ExtensionKeybindingRegistry::ExtensionKeybindingRegistry(
     content::BrowserContext* context,
     ExtensionFilter extension_filter,
     Delegate* delegate)
     : browser_context_(context),
       extension_filter_(extension_filter),
       delegate_(delegate),
       extension_registry_observer_(this) {
@@ skipping 78 matching lines @@
   // not set the delegate as it deals only with named commands (not page/browser
   // actions that are associated with the current page directly).
   ActiveTabPermissionGranter* granter =
       delegate_ ? delegate_->GetActiveTabPermissionGranter() : NULL;
   if (granter)
     granter->GrantIfRequested(extension);
 
   scoped_ptr<base::ListValue> args(new base::ListValue());
   args->Append(new base::StringValue(command));
 
-  scoped_ptr<Event> event(new Event("commands.onCommand", args.Pass()));
+  scoped_ptr<Event> event(new Event(kCommandsOnCommandEventName, args.Pass()));
   event->restrict_to_browser_context = browser_context_;
   event->user_gesture = EventRouter::USER_GESTURE_ENABLED;
   EventRouter::Get(browser_context_)
       ->DispatchEventToExtension(extension_id, event.Pass());
 }
 
 bool ExtensionKeybindingRegistry::IsAcceleratorRegistered(
     const ui::Accelerator& accelerator) const {
   return event_targets_.find(accelerator) != event_targets_.end();
 }
@@ skipping 98 matching lines @@
   return false;
 }
 
 bool ExtensionKeybindingRegistry::ExecuteCommands(
     const ui::Accelerator& accelerator,
     const std::string& extension_id) {
   EventTargets::iterator targets = event_targets_.find(accelerator);
   if (targets == event_targets_.end() || targets->second.empty())
     return false;
 
+  if (!extension_id.empty() &&
+      !extensions::EventRouter::Get(browser_context_)
+           ->ExtensionHasEventListener(extension_id,
+                                       kCommandsOnCommandEventName))
+    return false;
+
   bool executed = false;
   for (TargetList::const_iterator it = targets->second.begin();
        it != targets->second.end(); it++) {
     if (extension_id.empty() || it->first == extension_id) {
       CommandExecuted(it->first, it->second);
       executed = true;
     }
   }
 
   return executed;
 }
 
 }  // namespace extensions