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

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: 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/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 2eb924c8f01fe237adf06a9d03ea123c76b9a4c2..af9ade1370ec69b3dfe9585d0741942955444083 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,11 @@ 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::NotifyEventTargets(accelerator);
}
+ // Type != named command, so we need to dispatch this event directly.
std::pair<const std::string, gfx::NativeWindow> details =
std::make_pair(extension_id, window_);
content::NotificationService::current()->Notify(
@@ -72,6 +76,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 +99,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 +117,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 +131,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 +144,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