Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4114)

Unified Diff: chrome/browser/extensions/api/commands/command_service.cc

Issue 64273008: [Windows] Finish global and non-global media keys support on Windows. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix the test case!! Created 7 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/api/commands/command_service.cc
diff --git a/chrome/browser/extensions/api/commands/command_service.cc b/chrome/browser/extensions/api/commands/command_service.cc
index 9bff7a572ad6b3dfc860973b97a532ed4069ecb4..d9302d9842f500e1ea91ce30ca7e88b30087cd98 100644
--- a/chrome/browser/extensions/api/commands/command_service.cc
+++ b/chrome/browser/extensions/api/commands/command_service.cc
@@ -4,8 +4,11 @@
#include "chrome/browser/extensions/api/commands/command_service.h"
+#include <vector>
+
#include "base/lazy_instance.h"
#include "base/prefs/scoped_user_pref_update.h"
+#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/chrome_notification_types.h"
@@ -23,6 +26,7 @@
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_service.h"
#include "extensions/common/feature_switch.h"
+#include "extensions/common/manifest_constants.h"
using extensions::Extension;
using extensions::ExtensionPrefs;
@@ -38,9 +42,18 @@ const char kGlobal[] = "global";
const char kInitialBindingsHaveBeenAssigned[] = "initial_keybindings_set";
std::string GetPlatformKeybindingKeyForAccelerator(
- const ui::Accelerator& accelerator) {
- return extensions::Command::CommandPlatform() + ":" +
- extensions::Command::AcceleratorToString(accelerator);
+ const ui::Accelerator& accelerator, const std::string extension_id) {
+ std::string key = extensions::Command::CommandPlatform() + ":" +
+ extensions::Command::AcceleratorToString(accelerator);
+
+ // Media keys have a 1-to-many relationship with targets, unlike regular
+ // shortcut (1-to-1 relationship). That means two or more extensions can
+ // register for the same media key so the extension ID needs to be added to
+ // the key to make sure the key is unique.
+ if (extensions::CommandService::IsMediaKey(accelerator))
+ key += ":" + extension_id;
+
+ return key;
}
void SetInitialBindingsHaveBeenAssigned(
@@ -110,6 +123,17 @@ CommandService* CommandService::Get(Profile* profile) {
return ProfileKeyedAPIFactory<CommandService>::GetForProfile(profile);
}
+// static
+bool CommandService::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 CommandService::GetBrowserActionCommand(
const std::string& extension_id,
QueryType type,
@@ -188,11 +212,19 @@ bool CommandService::AddKeybindingPref(
if (accelerator.key_code() == ui::VKEY_UNKNOWN)
return false;
+ // Media Keys are allowed to be used by named command only.
+ if (IsMediaKey(accelerator)) {
+ DCHECK(command_name != manifest_values::kPageActionCommandEvent &&
+ command_name != manifest_values::kBrowserActionCommandEvent &&
+ command_name != manifest_values::kScriptBadgeCommandEvent);
Finnur 2013/11/19 15:29:47 nit: Probably preferable to do DCHECK(!IsMediaKey(
zhchbin 2013/11/19 15:49:11 Done.
+ }
+
DictionaryPrefUpdate updater(profile_->GetPrefs(),
prefs::kExtensionCommands);
base::DictionaryValue* bindings = updater.Get();
- std::string key = GetPlatformKeybindingKeyForAccelerator(accelerator);
+ std::string key = GetPlatformKeybindingKeyForAccelerator(accelerator,
+ extension_id);
if (!allow_overrides && bindings->HasKey(key))
return false; // Already taken.
@@ -286,9 +318,14 @@ Command CommandService::FindCommandByName(
if (FeatureSwitch::global_commands()->IsEnabled())
item->GetBoolean(kGlobal, &global);
+ // Format stored in Preferences is: "Platform:Shortcut[:ExtensionId]".
std::string shortcut = it.key();
- if (StartsWithASCII(shortcut, Command::CommandPlatform() + ":", true))
- shortcut = shortcut.substr(Command::CommandPlatform().length() + 1);
+ if (StartsWithASCII(shortcut, Command::CommandPlatform() + ":", true)) {
+ std::vector<std::string> tokens;
+ base::SplitString(shortcut, ':', &tokens);
+ CHECK(tokens.size() >= 2);
+ shortcut = tokens[1];
+ }
return Command(command_name, string16(), shortcut, global);
}
@@ -311,9 +348,11 @@ void CommandService::AssignInitialKeybindings(const Extension* extension) {
extensions::CommandMap::const_iterator iter = commands->begin();
for (; iter != commands->end(); ++iter) {
- if (!chrome::IsChromeAccelerator(
- iter->second.accelerator(), profile_) &&
- IsWhitelistedGlobalShortcut(iter->second)) {
+ // Make sure registered Chrome shortcuts cannot be automatically assigned
+ // (overwritten) by extension developers. Media keys are an exception here.
+ if ((!chrome::IsChromeAccelerator(iter->second.accelerator(), profile_) &&
+ IsWhitelistedGlobalShortcut(iter->second)) ||
+ extensions::CommandService::IsMediaKey(iter->second.accelerator())) {
AddKeybindingPref(iter->second.accelerator(),
extension->id(),
iter->second.command_name(),

Powered by Google App Engine
This is Rietveld 408576698