[Windows] Finish global and non-global media keys support on Windows.

chrome/browser/extensions/extension_keybinding_registry.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_keybinding_registry.h"
 
 #include "base/values.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
+#include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/extension_set.h"
 #include "extensions/common/manifest_constants.h"
 
 namespace extensions {
 
 ExtensionKeybindingRegistry::ExtensionKeybindingRegistry(
@@ skipping 10 matching lines @@
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_COMMAND_REMOVED,
                  content::Source<Profile>(profile->GetOriginalProfile()));
 }
 
 ExtensionKeybindingRegistry::~ExtensionKeybindingRegistry() {
 }
 
 void ExtensionKeybindingRegistry::RemoveExtensionKeybinding(
     const Extension* extension,
     const std::string& command_name) {
-  EventTargets::iterator iter = event_targets_.begin();
-  while (iter != event_targets_.end()) {
-    if (iter->second.first != extension->id() ||
-        (!command_name.empty() && (iter->second.second != command_name))) {
-      ++iter;
-      continue;  // Not the extension or command we asked for.
+  EventTargets::iterator it = event_targets_.begin();
+  while (it != event_targets_.end()) {
+    TargetList& target_list = it->second;
+    TargetList::iterator target = target_list.begin();
+    while (target != target_list.end()) {
+      if (target->first == extension->id() &&
+          (command_name.empty() || command_name == target->second)) {
+        target = target_list.erase(target);

[Finnur, 2013/11/18 11:16:14]

Um, how does this work? Don't you need to advance |target| before erasing?

[zhchbin, 2013/11/18 13:10:13]

The return value is an iterator pointing to the new location of the element that
followed the last element erased by the function call. This is the container end
if the operation erased the last element in the sequence.

Reference: http://www.cplusplus.com/reference/list/list/erase/

[Finnur, 2013/11/18 14:14:53]

Oh, I missed the assignment there.

+      } else {
+        target++;
+      }

[Finnur, 2013/11/18 11:16:14]

Nit: Both of these are single-line so no braces needed (but address comment
above first).

[zhchbin, 2013/11/18 13:10:13]

Done.

     }
 
-    // Let each platform-specific implementation get a chance to clean up.
-    RemoveExtensionKeybindingImpl(iter->first, command_name);
+    EventTargets::iterator old = it++;
+    if (target_list.empty()) {
+      // Let each platform-specific implementation get a chance to clean up.
+      RemoveExtensionKeybindingImpl(old->first, command_name);
+      event_targets_.erase(old);
 
-    EventTargets::iterator old = iter++;
-    event_targets_.erase(old);
-
-    // If a specific command_name was requested, it has now been deleted so
-    // no further work is required.
-    if (!command_name.empty())
-      break;
+      // If a specific command_name was requested, it has now been deleted so
+      // no further work is required.
+      if (!command_name.empty())
+        break;

[Finnur, 2013/11/18 11:16:14]

I think this should be outside the if clause that ends on line 64. If we found
the right command name we can quit, it doesn't matter whether we erase all the
targets, we won't find the same |command_name| in the rest of the map.

[zhchbin, 2013/11/18 13:10:13]

Done.

+    }
   }
 }
 
 void ExtensionKeybindingRegistry::Init() {
   ExtensionService* service =
       extensions::ExtensionSystem::Get(profile_)->extension_service();
   if (!service)
     return;  // ExtensionService can be null during testing.
 
   const ExtensionSet* extensions = service->extensions();
   ExtensionSet::const_iterator iter = extensions->begin();
   for (; iter != extensions->end(); ++iter)
     if (ExtensionMatchesFilter(iter->get()))
       AddExtensionKeybinding(iter->get(), std::string());
 }
 
 bool ExtensionKeybindingRegistry::ShouldIgnoreCommand(
     const std::string& command) const {
   return command == manifest_values::kPageActionCommandEvent ||
          command == manifest_values::kBrowserActionCommandEvent ||
          command == manifest_values::kScriptBadgeCommandEvent;
 }
 
+bool ExtensionKeybindingRegistry::NotifyEventTargetsByAccelerator(
+    const ui::Accelerator& accelerator) {
+  EventTargets::iterator targets = event_targets_.find(accelerator);
+  if (targets == event_targets_.end() || targets->second.empty())
+    return false;
+
+  for (TargetList::const_iterator it = targets->second.begin();
+       it != targets->second.end(); it++)
+    CommandExecuted(it->first, it->second);
+  return true;

[Finnur, 2013/11/18 11:16:14]

nit: Line break above.

[zhchbin, 2013/11/18 13:10:13]

Done.

+}
+
 void ExtensionKeybindingRegistry::CommandExecuted(
     const std::string& extension_id, const std::string& command) {
   ExtensionService* service =
       ExtensionSystem::Get(profile_)->extension_service();
 
   const Extension* extension = service->extensions()->GetByID(extension_id);
   if (!extension)
     return;
 
   // Grant before sending the event so that the permission is granted before
@@ skipping 70 matching lines @@
       return true;
     case PLATFORM_APPS_ONLY:
       return extension->is_platform_app();
     default:
       NOTREACHED();
   }
   return false;
 }
 
 }  // namespace extensions