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

chrome/browser/extensions/extension_keybinding_registry.cc

Index: chrome/browser/extensions/extension_keybinding_registry.cc
diff --git a/chrome/browser/extensions/extension_keybinding_registry.cc b/chrome/browser/extensions/extension_keybinding_registry.cc
index 86b9116d0b1fb951c3886dc36f0df7c98b74269f..c1e6a7ee204a35fb7b0f55ae0a999094d223e154 100644
--- a/chrome/browser/extensions/extension_keybinding_registry.cc
+++ b/chrome/browser/extensions/extension_keybinding_registry.cc
@@ -234,6 +234,12 @@ bool ExtensionKeybindingRegistry::ExecuteCommands(
   if (targets == event_targets_.end() || targets->second.empty())
     return false;
 
+  if (!extensions::EventRouter::Get(browser_context_)

[Finnur, 2014/08/27 11:28:31]

Be careful here: extension_id can be empty (see NotifyEventTargets above), and
I'm guessing that might cause this to return early, breaking some codepaths.

[David Tseng, 2014/08/27 16:34:22]

You're totally right; this broke the majority of the [global]commands tests.

+      ->ExtensionHasEventListener(
+            extension_id,
+            "commands.onCommand"))

[Finnur, 2014/08/27 11:28:31]

This string literal also appears on line 118, so a const might be warranted at
the top.

[David Tseng, 2014/08/27 16:34:22]

Done.

+    return false;
+

[Finnur, 2014/08/27 11:28:31]

I'd like to see tests added for this functionality, both for global and
non-global extension. Also would have been good to run this through the try
servers. I'll add a few bots, just to see what happens.

[David Tseng, 2014/08/27 16:34:22]

Working on some tests now; the existing tests appear to pass in patchset 2 (at
least locally on Chrome OS).

   bool executed = false;
   for (TargetList::const_iterator it = targets->second.begin();
        it != targets->second.end(); it++) {