Assign Ctrl-D keybinding to the new Bookmark Manager extension when removed from another extension

This is necessary to retain the same keybinding behavior as provided by
the existing bookmarking functionality.

BUG=446358

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

Cr-Commit-Position: refs/heads/master@{#311068}

13 files changed, 283 insertions, 20 deletions

diff --git a/chrome/browser/extensions/api/commands/command_service.cc b/chrome/browser/extensions/api/commands/command_service.cc
index b2af7bd..d38090c 100644
--- a/chrome/browser/extensions/api/commands/command_service.cc
+++ b/chrome/browser/extensions/api/commands/command_service.cc
@@ -72,6 +72,14 @@ bool IsForCurrentPlatform(const std::string& key) {
   return StartsWithASCII(key, Command::CommandPlatform() + ":", true);
 }
 
+std::string StripCurrentPlatform(const std::string& key) {
+  DCHECK(IsForCurrentPlatform(key));
+  std::string result = key;
+  ReplaceFirstSubstringAfterOffset(&result, 0, Command::CommandPlatform() + ":",
+                                   "");
+  return result;
+}
+
 void SetInitialBindingsHaveBeenAssigned(
     ExtensionPrefs* prefs, const std::string& extension_id) {
   prefs->UpdateExtensionPref(extension_id, kInitialBindingsHaveBeenAssigned,
@@ -804,13 +812,12 @@ void CommandService::RemoveKeybindingPrefs(const std::string& extension_id,
     std::string key = *it;
     bindings->Remove(key, NULL);
 
-    std::pair<const std::string, const std::string> details =
-        std::make_pair(extension_id, command_name);
+    ExtensionCommandRemovedDetails details(extension_id, command_name,
+                                           StripCurrentPlatform(key));
     content::NotificationService::current()->Notify(
         extensions::NOTIFICATION_EXTENSION_COMMAND_REMOVED,
         content::Source<Profile>(profile_),
-        content::Details<std::pair<const std::string, const std::string> >(
-            &details));
+        content::Details<ExtensionCommandRemovedDetails>(&details));
   }
 }
 
diff --git a/chrome/browser/extensions/extension_keybinding_registry.cc b/chrome/browser/extensions/extension_keybinding_registry.cc
index ea3aa5a..3b23942 100644
--- a/chrome/browser/extensions/extension_keybinding_registry.cc
+++ b/chrome/browser/extensions/extension_keybinding_registry.cc
@@ -185,13 +185,12 @@ void ExtensionKeybindingRegistry::Observe(
   switch (type) {
     case extensions::NOTIFICATION_EXTENSION_COMMAND_ADDED:
     case extensions::NOTIFICATION_EXTENSION_COMMAND_REMOVED: {
-      std::pair<const std::string, const std::string>* payload =
-          content::Details<std::pair<const std::string, const std::string> >(
-              details).ptr();
+      ExtensionCommandRemovedDetails* payload =
+          content::Details<ExtensionCommandRemovedDetails>(details).ptr();
 
       const Extension* extension = ExtensionRegistry::Get(browser_context_)
                                        ->enabled_extensions()
-                                       .GetByID(payload->first);
+                                       .GetByID(payload->extension_id);
       // During install and uninstall the extension won't be found. We'll catch
       // those events above, with the LOADED/UNLOADED, so we ignore this event.
       if (!extension)
@@ -199,9 +198,9 @@ void ExtensionKeybindingRegistry::Observe(
 
       if (ExtensionMatchesFilter(extension)) {
         if (type == extensions::NOTIFICATION_EXTENSION_COMMAND_ADDED)
-          AddExtensionKeybindings(extension, payload->second);
+          AddExtensionKeybindings(extension, payload->command_name);
         else
-          RemoveExtensionKeybinding(extension, payload->second);
+          RemoveExtensionKeybinding(extension, payload->command_name);
       }
       break;
     }
diff --git a/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc b/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
index 06b2006..f894cc7 100644
--- a/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
+++ b/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
@@ -61,6 +61,7 @@
 #include "chrome/browser/extensions/extension_management.h"
 #include "chrome/browser/search/hotword_service_factory.h"
 #include "chrome/browser/signin/easy_unlock_service_factory.h"
+#include "chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.h"
 #include "extensions/browser/browser_context_keyed_service_factories.h"
 #endif
 
@@ -186,6 +187,7 @@ EnsureBrowserContextKeyedServiceFactoriesBuilt() {
   DownloadServiceFactory::GetInstance();
 #if defined(ENABLE_EXTENSIONS)
   EasyUnlockServiceFactory::GetInstance();
+  EnhancedBookmarkKeyServiceFactory::GetInstance();
 #endif
   FaviconServiceFactory::GetInstance();
   FindBarStateFactory::GetInstance();
diff --git a/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.cc b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.cc
new file mode 100644
index 0000000..ff39e07
--- /dev/null
+++ b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.cc
@@ -0,0 +1,85 @@
+// Copyright 2015 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/ui/bookmarks/enhanced_bookmark_key_service.h"
+
+#include <algorithm>
+
+#include "base/sha1.h"
+#include "base/strings/string_number_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/extensions/api/commands/command_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/accelerator_utils.h"
+#include "chrome/common/extensions/command.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/notification_types.h"
+#include "extensions/common/manifest_constants.h"
+
+EnhancedBookmarkKeyService::EnhancedBookmarkKeyService(
+    content::BrowserContext* context) : browser_context_(context) {
+  Profile* profile = Profile::FromBrowserContext(browser_context_);
+  registrar_.Add(this,
+                 extensions::NOTIFICATION_EXTENSION_COMMAND_REMOVED,
+                 content::Source<Profile>(profile->GetOriginalProfile()));
+}
+
+EnhancedBookmarkKeyService::~EnhancedBookmarkKeyService() {
+}
+
+void EnhancedBookmarkKeyService::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(extensions::NOTIFICATION_EXTENSION_COMMAND_REMOVED, type);
+
+  const extensions::Extension* enhanced_bookmark_extension =
+      GetEnhancedBookmarkExtension();
+  if (!enhanced_bookmark_extension)
+    return;
+
+  extensions::ExtensionCommandRemovedDetails* payload =
+      content::Details<extensions::ExtensionCommandRemovedDetails>(details)
+          .ptr();
+
+  if (payload->extension_id == enhanced_bookmark_extension->id())
+    return;
+
+  ui::Accelerator key = extensions::Command::StringToAccelerator(
+      payload->accelerator, payload->command_name);
+  ui::Accelerator bookmark_accelerator =
+      chrome::GetPrimaryChromeAcceleratorForCommandId(IDC_BOOKMARK_PAGE);
+  if (key == bookmark_accelerator) {
+    extensions::CommandService* command_service =
+        extensions::CommandService::Get(browser_context_);
+    extensions::Command existing_command;
+    if (!command_service->GetPageActionCommand(
+            enhanced_bookmark_extension->id(),
+            extensions::CommandService::ACTIVE,
+            &existing_command, nullptr)) {
+      command_service->AddKeybindingPref(
+          bookmark_accelerator, enhanced_bookmark_extension->id(),
+          extensions::manifest_values::kPageActionCommandEvent, false,
+          false);
+    }
+  }
+}
+
+const extensions::Extension*
+EnhancedBookmarkKeyService::GetEnhancedBookmarkExtension() const {
+  const extensions::ExtensionSet& extensions =
+      extensions::ExtensionRegistry::Get(browser_context_)
+      ->enabled_extensions();
+  extensions::ExtensionSet::const_iterator loc =
+      std::find_if(extensions.begin(), extensions.end(),
+                   [](scoped_refptr<const extensions::Extension> extension) {
+                     static const char enhanced_extension_hash[] =
+                         // http://crbug.com/312900
+                         "D5736E4B5CF695CB93A2FB57E4FDC6E5AFAB6FE2";
+                     std::string hash = base::SHA1HashString(extension->id());
+                     return base::HexEncode(hash.c_str(), hash.length()) ==
+                         enhanced_extension_hash;
+                   });
+  return loc != extensions.end() ? loc->get() : nullptr;
+}
diff --git a/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.h b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.h
new file mode 100644
index 0000000..6e51431
--- /dev/null
+++ b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.h
@@ -0,0 +1,46 @@
+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_UI_BOOKMARKS_ENHANCED_BOOKMARK_KEY_SERVICE_H_
+#define CHROME_BROWSER_UI_BOOKMARKS_ENHANCED_BOOKMARK_KEY_SERVICE_H_
+
+#include "components/keyed_service/core/keyed_service.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_source.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+namespace extensions {
+class Extension;
+}  // namespace extensions
+
+// Maintains assignment of bookmark keybinding on the enhanced bookmarks
+// extension when not assigned to other extensions.
+class EnhancedBookmarkKeyService : public content::NotificationObserver,
+                                   public KeyedService {
+ public:
+  EnhancedBookmarkKeyService(content::BrowserContext* context);
+  ~EnhancedBookmarkKeyService() override;
+
+ private:
+  // Overridden from content::NotificationObserver:
+  void Observe(int type,
+               const content::NotificationSource& source,
+               const content::NotificationDetails& details) override;
+
+  const extensions::Extension* GetEnhancedBookmarkExtension() const;
+
+  // The content notification registrar for listening to extension key events.
+  content::NotificationRegistrar registrar_;
+
+  content::BrowserContext* browser_context_;
+
+  DISALLOW_COPY_AND_ASSIGN(EnhancedBookmarkKeyService);
+};
+
+#endif  // CHROME_BROWSER_UI_BOOKMARKS_ENHANCED_BOOKMARK_KEY_SERVICE_H_
diff --git a/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.cc b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.cc
new file mode 100644
index 0000000..ff2b876
--- /dev/null
+++ b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.cc
@@ -0,0 +1,45 @@
+// Copyright 2015 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/ui/bookmarks/enhanced_bookmark_key_service_factory.h"
+
+#include "base/memory/singleton.h"
+#include "chrome/browser/profiles/incognito_helpers.h"
+#include "chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.h"
+#include "components/keyed_service/content/browser_context_dependency_manager.h"
+
+// static
+EnhancedBookmarkKeyServiceFactory*
+    EnhancedBookmarkKeyServiceFactory::GetInstance() {
+  return Singleton<EnhancedBookmarkKeyServiceFactory>::get();
+}
+
+EnhancedBookmarkKeyServiceFactory::EnhancedBookmarkKeyServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+        "EnhancedBookmarkKeyService",
+        BrowserContextDependencyManager::GetInstance()) {
+}
+
+EnhancedBookmarkKeyServiceFactory::~EnhancedBookmarkKeyServiceFactory() {
+}
+
+KeyedService* EnhancedBookmarkKeyServiceFactory::BuildServiceInstanceFor(
+    content::BrowserContext* context) const {
+  return new EnhancedBookmarkKeyService(context);
+}
+
+content::BrowserContext*
+    EnhancedBookmarkKeyServiceFactory::GetBrowserContextToUse(
+        content::BrowserContext* context) const {
+  return chrome::GetBrowserContextRedirectedInIncognito(context);
+}
+
+bool EnhancedBookmarkKeyServiceFactory::ServiceIsCreatedWithBrowserContext()
+    const {
+  return true;
+}
+
+bool EnhancedBookmarkKeyServiceFactory::ServiceIsNULLWhileTesting() const {
+  return true;
+}
diff --git a/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.h b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.h
new file mode 100644
index 0000000..5d6ce10
--- /dev/null
+++ b/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service_factory.h
@@ -0,0 +1,43 @@
+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_UI_BOOKMARKS_ENHANCED_BOOKMARK_KEY_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_UI_BOOKMARKS_ENHANCED_BOOKMARK_KEY_SERVICE_FACTORY_H_
+
+#include "base/macros.h"
+#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
+
+namespace content {
+class BrowserContext;
+}  // namespace content
+
+template <typename T> struct DefaultSingletonTraits;
+
+class EnhancedBookmarkKeyService;
+
+// Singleton that owns all EnhancedBookmarkKeyServices and associates them with
+// BrowserContexts.
+class EnhancedBookmarkKeyServiceFactory
+    : public BrowserContextKeyedServiceFactory {
+ public:
+  static EnhancedBookmarkKeyServiceFactory* GetInstance();
+
+ private:
+  friend struct DefaultSingletonTraits<EnhancedBookmarkKeyServiceFactory>;
+
+  EnhancedBookmarkKeyServiceFactory();
+  ~EnhancedBookmarkKeyServiceFactory() override;
+
+  // BrowserContextKeyedServiceFactory:
+  KeyedService* BuildServiceInstanceFor(
+      content::BrowserContext* context) const override;
+  content::BrowserContext* GetBrowserContextToUse(
+      content::BrowserContext* context) const override;
+  bool ServiceIsCreatedWithBrowserContext() const override;
+  bool ServiceIsNULLWhileTesting() const override;
+
+  DISALLOW_COPY_AND_ASSIGN(EnhancedBookmarkKeyServiceFactory);
+};
+
+#endif  // CHROME_BROWSER_UI_BOOKMARKS_ENHANCED_BOOKMARK_KEY_SERVICE_FACTORY_H_
diff --git a/chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc b/chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc
index 88625b5..2bd6094 100644
--- a/chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc
+++ b/chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc
@@ -147,14 +147,14 @@ void ExtensionActionPlatformDelegateViews::Observe(
     const content::NotificationDetails& details) {
   DCHECK(type == extensions::NOTIFICATION_EXTENSION_COMMAND_ADDED ||
          type == extensions::NOTIFICATION_EXTENSION_COMMAND_REMOVED);
-  std::pair<const std::string, const std::string>* payload =
-      content::Details<std::pair<const std::string, const std::string> >(
-          details).ptr();
-  if (controller_->extension()->id() == payload->first &&
-      (payload->second ==
-       extensions::manifest_values::kBrowserActionCommandEvent ||
-       payload->second ==
-       extensions::manifest_values::kPageActionCommandEvent)) {
+  extensions::ExtensionCommandRemovedDetails* payload =
+      content::Details<extensions::ExtensionCommandRemovedDetails>(details)
+          .ptr();
+  if (controller_->extension()->id() == payload->extension_id &&
+      (payload->command_name ==
+           extensions::manifest_values::kBrowserActionCommandEvent ||
+       payload->command_name ==
+           extensions::manifest_values::kPageActionCommandEvent)) {
     if (type == extensions::NOTIFICATION_EXTENSION_COMMAND_ADDED)
       RegisterCommand();
     else
diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi
index 0ca524a..fccc399 100644
--- a/chrome/chrome_browser_ui.gypi
+++ b/chrome/chrome_browser_ui.gypi
@@ -1395,6 +1395,10 @@
       'browser/ui/bookmarks/bookmark_drag_drop.h',
       'browser/ui/bookmarks/bookmark_tab_helper_delegate.cc',
       'browser/ui/bookmarks/bookmark_tab_helper_delegate.h',
+      'browser/ui/bookmarks/enhanced_bookmark_key_service_factory.cc',
+      'browser/ui/bookmarks/enhanced_bookmark_key_service_factory.h',
+      'browser/ui/bookmarks/enhanced_bookmark_key_service.cc',
+      'browser/ui/bookmarks/enhanced_bookmark_key_service.h',
       'browser/ui/browser.cc',
       'browser/ui/browser.h',
       'browser/ui/browser_command_controller.cc',
diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
index 1b717da..1b50709 100644
--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -434,6 +434,8 @@ source_set("browser") {
       "mojo/service_registration_manager.h",
       "mojo/stash_backend.cc",
       "mojo/stash_backend.h",
+      "notification_types.cc",
+      "notification_types.h",
       "null_app_sorting.cc",
       "null_app_sorting.h",
       "pref_names.cc",
diff --git a/extensions/browser/notification_types.cc b/extensions/browser/notification_types.cc
new file mode 100644
index 0000000..8238fb2
--- /dev/null
+++ b/extensions/browser/notification_types.cc
@@ -0,0 +1,16 @@
+// Copyright 2015 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 "extensions/browser/notification_types.h"
+
+namespace extensions {
+ExtensionCommandRemovedDetails::ExtensionCommandRemovedDetails(
+    const std::string& extension_id,
+    const std::string& command_name,
+    const std::string& accelerator)
+    : extension_id(extension_id),
+      command_name(command_name),
+      accelerator(accelerator) {
+}
+}  // namespace extensions
diff --git a/extensions/browser/notification_types.h b/extensions/browser/notification_types.h
index 6eee02e..f1d0b17 100644
--- a/extensions/browser/notification_types.h
+++ b/extensions/browser/notification_types.h
@@ -5,6 +5,8 @@
 #ifndef EXTENSIONS_BROWSER_NOTIFICATION_TYPES_H_
 #define EXTENSIONS_BROWSER_NOTIFICATION_TYPES_H_
 
+#include <string>
+
 #include "content/public/browser/notification_types.h"
 
 #if !defined(ENABLE_EXTENSIONS)
@@ -132,8 +134,9 @@ enum NotificationType {
   NOTIFICATION_EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
 
   // Sent when an extension command has been removed. The source is the
-  // BrowserContext* and the details is a std::pair of two std::string objects
-  // (an extension ID and the name of the command being removed).
+  // BrowserContext* and the details is an ExtensionCommandRemovedDetails
+  // consisting of std::strings representing an extension ID, the name of the
+  // command being removed, and the accelerator associated with the command.
   NOTIFICATION_EXTENSION_COMMAND_REMOVED,
 
   // Sent when an extension command has been added. The source is the
@@ -212,6 +215,16 @@ enum NotificationType {
   NOTIFICATION_EXTENSIONS_END
 };
 
+struct ExtensionCommandRemovedDetails {
+  ExtensionCommandRemovedDetails(const std::string& extension_id,
+                                 const std::string& command_name,
+                                 const std::string& accelerator);
+
+  std::string extension_id;
+  std::string command_name;
+  std::string accelerator;
+};
+
 }  // namespace extensions
 
 #endif  // EXTENSIONS_BROWSER_NOTIFICATION_TYPES_H_
diff --git a/extensions/extensions.gyp b/extensions/extensions.gyp
index e7e2734..193a291 100644
--- a/extensions/extensions.gyp
+++ b/extensions/extensions.gyp
@@ -724,6 +724,7 @@
         'browser/mojo/service_registration_manager.h',
         'browser/mojo/stash_backend.cc',
         'browser/mojo/stash_backend.h',
+        'browser/notification_types.cc',
         'browser/notification_types.h',
         'browser/null_app_sorting.cc',
         'browser/null_app_sorting.h',