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

Unified Diff: chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm

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: Address comments. 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/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm b/chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm
index 54b0d3d375e4784ee7ac9ba35c14ad5d2f15b6e1..1fa93bd374c1933b2d4ae2955533b06afd002108 100644
--- a/chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm
+++ b/chrome/browser/ui/cocoa/extensions/extension_keybinding_registry_cocoa.mm
@@ -45,12 +45,16 @@ bool ExtensionKeybindingRegistryCocoa::ProcessKeyEvent(
ui::Accelerator accelerator(
static_cast<ui::KeyboardCode>(event.windowsKeyCode),
content::GetModifiersFromNativeWebKeyboardEvent(event));
- EventTargets::iterator it = event_targets_.find(accelerator);
- if (it == event_targets_.end())
+ EventTargets::iterator targets = event_targets_.find(accelerator);
+ if (targets == event_targets_.end())
return false;
- std::string extension_id = it->second.first;
- std::string command_name = it->second.second;
+ TargetList::const_iterator first_target = targets->second.begin();
+ if (first_target == targets->second.end())
+ return false;
+
+ std::string extension_id = first_target->first;
+ std::string command_name = first_target->second;
int type = 0;
if (command_name == values::kPageActionCommandEvent) {
type = chrome::NOTIFICATION_EXTENSION_COMMAND_PAGE_ACTION_MAC;
@@ -60,11 +64,12 @@ bool ExtensionKeybindingRegistryCocoa::ProcessKeyEvent(
type = chrome::NOTIFICATION_EXTENSION_COMMAND_SCRIPT_BADGE_MAC;
} else {
// Not handled by using notifications. Route it through the Browser Event
- // Router.
- CommandExecuted(extension_id, command_name);
- return true;
+ // Router using the base class (it will iterate through all targets).
+ return ExtensionKeybindingRegistry::NotifyEventTargetsByAccelerator(
+ accelerator);
}
+ // type != named command, so we need to dispatch this event directly.
Finnur 2013/11/18 11:16:14 nit: Capitalize 'type'.
zhchbin 2013/11/18 13:10:13 Done.
std::pair<const std::string, gfx::NativeWindow> details =
std::make_pair(extension_id, window_);
content::NotificationService::current()->Notify(
@@ -72,6 +77,8 @@ bool ExtensionKeybindingRegistryCocoa::ProcessKeyEvent(
content::Source<Profile>(profile_),
content::Details<
std::pair<const std::string, gfx::NativeWindow> >(&details));
+ // We expect only one target for these types of events.
+ DCHECK(++first_target == targets->second.end());
return true;
}
@@ -93,8 +100,12 @@ void ExtensionKeybindingRegistryCocoa::AddExtensionKeybinding(
continue;
ui::Accelerator accelerator(iter->second.accelerator());
- event_targets_[accelerator] =
- std::make_pair(extension->id(), iter->second.command_name());
+ event_targets_[accelerator].push_back(
+ std::make_pair(extension->id(), iter->second.command_name()));
+ // Shortcuts except media keys have only one target in the list. See
+ // comment about |event_targets_|.
+ if (!extensions::CommandService::IsMediaKey(iter->second.accelerator()))
+ DCHECK(event_targets_[iter->second.accelerator()].size() == 1);
}
// Mac implemenetation behaves like GTK with regards to what is kept in the
@@ -107,8 +118,10 @@ void ExtensionKeybindingRegistryCocoa::AddExtensionKeybinding(
&browser_action,
NULL)) {
ui::Accelerator accelerator(browser_action.accelerator());
- event_targets_[accelerator] =
- std::make_pair(extension->id(), browser_action.command_name());
+ event_targets_[accelerator].push_back(
+ std::make_pair(extension->id(), browser_action.command_name()));
+ // We should have only one target. See comment about |event_targets_|.
+ DCHECK(event_targets_[accelerator].size() == 1);
}
// Add the Page Action (if any).
@@ -119,8 +132,9 @@ void ExtensionKeybindingRegistryCocoa::AddExtensionKeybinding(
&page_action,
NULL)) {
ui::Accelerator accelerator(page_action.accelerator());
- event_targets_[accelerator] =
- std::make_pair(extension->id(), page_action.command_name());
+ event_targets_[accelerator].push_back(
+ std::make_pair(extension->id(), page_action.command_name()));
+ DCHECK(event_targets_[accelerator].size() == 1); // Ditto.
}
// Add the Script Badge (if any).
@@ -131,8 +145,9 @@ void ExtensionKeybindingRegistryCocoa::AddExtensionKeybinding(
&script_badge,
NULL)) {
ui::Accelerator accelerator(script_badge.accelerator());
- event_targets_[accelerator] =
- std::make_pair(extension->id(), script_badge.command_name());
+ event_targets_[accelerator].push_back(
+ std::make_pair(extension->id(), script_badge.command_name()));
+ DCHECK(event_targets_[accelerator].size() == 1); // Ditto.
}
}

Powered by Google App Engine
This is Rietveld 408576698