Media Keys should not count towards the max of four shortcuts per extension.

R=finnur@chromium.org
TEST=unit_tests --gtest_filter=CommandsManifestTest.CommandManifestShouldNotCountMediaKeys
BUG=329870

Review URL: https://codereview.chromium.org/180783012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254176 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 29 insertions, 1 deletions

diff --git a/chrome/common/extensions/api/commands/commands_handler.cc b/chrome/common/extensions/api/commands/commands_handler.cc
index d49a105..8d0788b 100644
--- a/chrome/common/extensions/api/commands/commands_handler.cc
+++ b/chrome/common/extensions/api/commands/commands_handler.cc
@@ -7,6 +7,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "chrome/common/extensions/command.h"
 #include "extensions/common/error_utils.h"
 #include "extensions/common/manifest_constants.h"
 
@@ -90,7 +91,13 @@ bool CommandsHandler::Parse(Extension* extension, base::string16* error) {
       return false;  // |error| already set.
 
     if (binding->accelerator().key_code() != ui::VKEY_UNKNOWN) {
-      if (++keybindings_found > kMaxCommandsWithKeybindingPerExtension) {
+      // Only media keys are allowed to work without modifiers, and because
+      // media keys aren't registered exclusively they should not count towards
+      // the max of four shortcuts per extension.
+      if (!Command::IsMediaKey(binding->accelerator()))
+        ++keybindings_found;
+
+      if (keybindings_found > kMaxCommandsWithKeybindingPerExtension) {
         *error = ErrorUtils::FormatErrorMessageUTF16(
             manifest_errors::kInvalidKeyBindingTooMany,
             base::IntToString(kMaxCommandsWithKeybindingPerExtension));
diff --git a/chrome/common/extensions/api/commands/commands_manifest_unittest.cc b/chrome/common/extensions/api/commands/commands_manifest_unittest.cc
index a0eab03..b753003 100644
--- a/chrome/common/extensions/api/commands/commands_manifest_unittest.cc
+++ b/chrome/common/extensions/api/commands/commands_manifest_unittest.cc
@@ -132,4 +132,9 @@ TEST_F(CommandsManifestTest, ChannelTests) {
   }
 }
 
+TEST_F(CommandsManifestTest, CommandManifestShouldNotCountMediaKeys) {
+  scoped_refptr<Extension> extension =
+      LoadAndExpectSuccess("command_should_not_count_media_keys.json");
+}
+
 }  // namespace extensions
diff --git a/chrome/common/extensions/command.cc b/chrome/common/extensions/command.cc
index faed130..5ebf3fc 100644
--- a/chrome/common/extensions/command.cc
+++ b/chrome/common/extensions/command.cc
@@ -380,6 +380,17 @@ std::string Command::AcceleratorToString(const ui::Accelerator& accelerator) {
   return shortcut;
 }
 
+// static
+bool Command::IsMediaKey(const ui::Accelerator& accelerator) {
+  if (accelerator.modifiers() != 0)
+    return false;
+
+  return (accelerator.key_code() == ui::VKEY_MEDIA_NEXT_TRACK ||
+          accelerator.key_code() == ui::VKEY_MEDIA_PREV_TRACK ||
+          accelerator.key_code() == ui::VKEY_MEDIA_PLAY_PAUSE ||
+          accelerator.key_code() == ui::VKEY_MEDIA_STOP);
+}
+
 bool Command::Parse(const base::DictionaryValue* command,
                     const std::string& command_name,
                     int index,
diff --git a/chrome/common/extensions/command.h b/chrome/common/extensions/command.h
index 0ea3fe4..41ec35a 100644
--- a/chrome/common/extensions/command.h
+++ b/chrome/common/extensions/command.h
@@ -42,6 +42,11 @@ class Command {
   // shortcut text (like accelerator::GetShortcutText() does).
   static std::string AcceleratorToString(const ui::Accelerator& accelerator);
 
+  // Return true if the specified accelerator is one of the following multimedia
+  // keys: Next Track key, Previous Track key, Stop Media key, Play/Pause Media
+  // key, without any modifiers.
+  static bool IsMediaKey(const ui::Accelerator& accelerator);
+
   // Parse the command.
   bool Parse(const base::DictionaryValue* command,
              const std::string& command_name,