Revert 194594 "Make sure manifest specified shortcut for Extensi..."

Got linker error not caught by try servers.

> Make sure manifest specified shortcut for Extension Command can not override the built-in shortcuts.
> 
> User is still free to override manually.
> 
> BUG=226994
> TEST=Automated test included.
> 
> Review URL: https://codereview.chromium.org/13044014

TBR=finnur@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@194597 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/commands/command_service.cc

@@ -14,7 +14,6 @@
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/accelerator_utils.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/api/commands/commands_handler.h"
 #include "chrome/common/pref_names.h"
@@ -233,49 +232,37 @@ 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_)) {
-      AddKeybindingPref(iter->second.accelerator(),
-                        extension->id(),
-                        iter->second.command_name(),
-                        false);  // Overwriting not allowed.
-    }
+    AddKeybindingPref(iter->second.accelerator(),
+                      extension->id(),
+                      iter->second.command_name(),
+                      false);  // Overwriting not allowed.
   }
 
   const extensions::Command* browser_action_command =
       CommandsInfo::GetBrowserActionCommand(extension);
   if (browser_action_command) {
-    if (!chrome::IsChromeAccelerator(
-        browser_action_command->accelerator(), profile_)) {
-      AddKeybindingPref(browser_action_command->accelerator(),
-                        extension->id(),
-                        browser_action_command->command_name(),
-                        false);  // Overwriting not allowed.
-    }
+    AddKeybindingPref(browser_action_command->accelerator(),
+                      extension->id(),
+                      browser_action_command->command_name(),
+                      false);  // Overwriting not allowed.
   }
 
   const extensions::Command* page_action_command =
       CommandsInfo::GetPageActionCommand(extension);
   if (page_action_command) {
-    if (!chrome::IsChromeAccelerator(
-        page_action_command->accelerator(), profile_)) {
-      AddKeybindingPref(page_action_command->accelerator(),
-                        extension->id(),
-                        page_action_command->command_name(),
-                        false);  // Overwriting not allowed.
-    }
+    AddKeybindingPref(page_action_command->accelerator(),
+                      extension->id(),
+                      page_action_command->command_name(),
+                      false);  // Overwriting not allowed.
   }
 
   const extensions::Command* script_badge_command =
       CommandsInfo::GetScriptBadgeCommand(extension);
   if (script_badge_command) {
-    if (!chrome::IsChromeAccelerator(
-        script_badge_command->accelerator(), profile_)) {
-      AddKeybindingPref(script_badge_command->accelerator(),
-                        extension->id(),
-                        script_badge_command->command_name(),
-                        false);  // Overwriting not allowed.
-    }
+    AddKeybindingPref(script_badge_command->accelerator(),
+                      extension->id(),
+                      script_badge_command->command_name(),
+                      false);  // Overwriting not allowed.
   }
 }
 

chrome/browser/extensions/extension_keybinding_apitest.cc

@@ -195,44 +195,4 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, SynthesizedCommand) {
   ASSERT_TRUE(RunExtensionTest("keybinding/synthesized")) << message_;
 }
 
-// This test validates that an extension cannot request a shortcut that is
-// already in use by Chrome.
-IN_PROC_BROWSER_TEST_F(CommandsApiTest, DontOverwriteSystemShortcuts) {
-  ASSERT_TRUE(test_server()->Start());
-  ASSERT_TRUE(RunExtensionTest("keybinding/dont_overwrite_system")) << message_;
-
-  ui_test_utils::NavigateToURL(browser(),
-      test_server()->GetURL("files/extensions/test_file.txt"));
-
-  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
-  ASSERT_TRUE(tab);
-
-  // Activate the shortcut (Alt+Shift+F) to make page blue.
-  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
-      browser(), ui::VKEY_F, false, true, true, false));
-
-  bool result = false;
-  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-      tab,
-      "setInterval(function() {"
-      "  if (document.body.bgColor == 'blue') {"
-      "    window.domAutomationController.send(true)}}, 100)",
-      &result));
-  ASSERT_TRUE(result);
-
-  // Activate the shortcut (Ctrl+F) to make page red (should not work).
-  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
-      browser(), ui::VKEY_F, true, false, false, false));
-
-  // The page should still be blue.
-  result = false;
-  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
-      tab,
-      "setInterval(function() {"
-      "  if (document.body.bgColor == 'blue') {"
-      "    window.domAutomationController.send(true)}}, 100)",
-      &result));
-  ASSERT_TRUE(result);
-}
-
 }  // extensions

chrome/browser/ui/accelerator_utils.h

-// Copyright (c) 2013 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_ACCELERATOR_UTILS_H_
-#define CHROME_BROWSER_UI_ACCELERATOR_UTILS_H_
-
-class Profile;
-
-namespace ui {
-class Accelerator;
-}
-
-namespace chrome {
-
-// Returns true if the given |accelerator| is currently registered by
-// Chrome.
-bool IsChromeAccelerator(const ui::Accelerator& accelerator,
-                         Profile* profile);
-
-}  // namespace chrome
-
-#endif  // CHROME_BROWSER_UI_ACCELERATOR_UTILS_H_

chrome/browser/ui/cocoa/accelerator_utils_cocoa.mm

-// Copyright (c) 2013 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/profiles/profile.h"
-#include "chrome/browser/ui/cocoa/accelerators_cocoa.h"
-#import "chrome/browser/ui/cocoa/browser_window_utils.h"
-#include "content/public/browser/native_web_keyboard_event.h"
-#include "ui/base/accelerators/accelerator.h"
-#import "ui/base/accelerators/platform_accelerator_cocoa.h"
-#import "ui/base/keycodes/keyboard_code_conversion_mac.h"
-
-namespace chrome {
-
-bool IsChromeAccelerator(const ui::Accelerator& accelerator, Profile* profile) {
-  // The |accelerator| passed in contains a Windows key code but no platform
-  // accelerator info. The Accelerator list is the opposite: It has accelerators
-  // that have key_code() == VKEY_UNKNOWN but they contain a platform
-  // accelerator. We find common ground by converting the passed in Windows key
-  // code to a character and use that when comparing against the Accelerator
-  // list.
-  unichar character;
-  unichar characterIgnoringModifiers;
-  ui::MacKeyCodeForWindowsKeyCode(accelerator.key_code(),
-                                  0,
-                                  &character,
-                                  &characterIgnoringModifiers);
-  NSString* characters =
-      [[[NSString alloc] initWithCharacters:&character length:1] autorelease];
-
-  NSUInteger modifiers =
-      (accelerator.IsCtrlDown() ? NSControlKeyMask : 0) |
-      (accelerator.IsCmdDown() ? NSCommandKeyMask : 0) |
-      (accelerator.IsAltDown() ? NSAlternateKeyMask : 0) |
-      (accelerator.IsShiftDown() ? NSShiftKeyMask : 0);
-
-  NSEvent* event = [NSEvent keyEventWithType:NSKeyDown
-                                    location:NSZeroPoint
-                               modifierFlags:modifiers
-                                   timestamp:0
-                                windowNumber:0
-                                     context:nil
-                                  characters:characters
-                 charactersIgnoringModifiers:characters
-                                   isARepeat:NO
-                                     keyCode:accelerator.key_code()];
-
-  content::NativeWebKeyboardEvent keyboard_event(event);
-  int id = [BrowserWindowUtils getCommandId:keyboard_event];
-  return id != -1;
-}
-
-}  // namespace chrome
\ No newline at end of file

chrome/browser/ui/cocoa/accelerators_cocoa.h

@@ -26,10 +26,6 @@ template <typename T> struct DefaultSingletonTraits;
 class AcceleratorsCocoa {
  public:
   typedef std::map<int, ui::Accelerator> AcceleratorMap;
-  typedef AcceleratorMap::const_iterator const_iterator;
-
-  const_iterator const begin() { return accelerators_.begin(); }
-  const_iterator const end() { return accelerators_.end(); }
 
   // Returns NULL if there is no accelerator for the command.
   const ui::Accelerator* GetAcceleratorForCommand(int command_id);

chrome/browser/ui/gtk/accelerator_utils_gtk.cc

-// Copyright (c) 2013 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/profiles/profile.h"
-#include "chrome/browser/ui/gtk/accelerators_gtk.h"
-#include "ui/base/accelerators/accelerator.h"
-
-namespace chrome {
-
-bool IsChromeAccelerator(const ui::Accelerator& accelerator, Profile* profile) {
-  AcceleratorsGtk* accelerators = AcceleratorsGtk::GetInstance();
-  for (AcceleratorsGtk::const_iterator iter = accelerators->begin();
-       iter != accelerators->end(); ++iter) {
-    if (iter->second.key_code() == accelerator.key_code() &&
-        iter->second.modifiers() == accelerator.modifiers())
-      return true;
-  }
-
-  return false;
-}
-
-}  // namespace chrome

chrome/browser/ui/views/accelerator_utils_aura.cc

-// Copyright (c) 2013 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/profiles/profile.h"
-#include "chrome/browser/ui/views/accelerator_table.h"
-#include "ui/base/accelerators/accelerator.h"
-
-#if defined(USE_ASH)
-#include "ash/accelerators/accelerator_table.h"
-#endif  // USE_ASH
-
-namespace chrome {
-
-bool IsChromeAccelerator(const ui::Accelerator& accelerator, Profile* profile) {
-#if defined(USE_ASH)
-  for (size_t i = 0; i < ash::kAcceleratorDataLength; ++i) {
-    const ash::AcceleratorData& accel_data = ash::kAcceleratorData[i];
-    if (accel_data.keycode == accelerator.key_code() &&
-        accel_data.modifiers == accelerator.modifiers()) {
-      return true;
-    }
-  }
-#endif
-
-  std::vector<chrome::AcceleratorMapping> accelerators =
-      chrome::GetAcceleratorList();
-  for (std::vector<chrome::AcceleratorMapping>::const_iterator it =
-       accelerators.begin(); it != accelerators.end(); ++it) {
-    if (it->keycode == accelerator.key_code() &&
-        it->modifiers == accelerator.modifiers())
-      return true;
-  }
-
-  return false;
-}
-
-}  // namespace chrome

chrome/browser/ui/views/accelerator_utils_views.cc

-// Copyright (c) 2013 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 "ui/base/accelerators/accelerator.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
-
-namespace chrome {
-
-bool IsChromeAccelerator(const ui::Accelerator& accelerator, Profile* profile) {
-  Browser* browser = chrome::FindLastActiveWithProfile(
-      profile, chrome::HOST_DESKTOP_TYPE_NATIVE);
-  if (!browser)
-    return false;
-  BrowserView* browser_view = BrowserView::GetBrowserViewForNativeWindow(
-      browser->window()->GetNativeWindow());
-  return browser_view->IsAcceleratorRegistered(accelerator);
-}
-
-}  // namespace chrome

chrome/browser/ui/views/frame/browser_view.cc

@@ -638,10 +638,6 @@ bool BrowserView::GetAccelerator(int cmd_id, ui::Accelerator* accelerator) {
       cmd_id, browser_->host_desktop_type(), accelerator);
 }
 
-bool BrowserView::IsAcceleratorRegistered(const ui::Accelerator& accelerator) {
-  return accelerator_table_.find(accelerator) != accelerator_table_.end();
-}
-
 WebContents* BrowserView::GetActiveWebContents() const {
   return browser_->tab_strip_model()->GetActiveWebContents();
 }

chrome/browser/ui/views/frame/browser_view.h

@@ -188,9 +188,6 @@ class BrowserView : public BrowserWindow,
   // otherwise.
   bool GetAccelerator(int cmd_id, ui::Accelerator* accelerator);
 
-  // Returns true if the specificed |accelerator| is registered with this view.
-  bool IsAcceleratorRegistered(const ui::Accelerator& accelerator);
-
   // Returns the active WebContents. Used by our NonClientView's
   // TabIconView::TabContentsProvider implementations.
   // TODO(beng): exposing this here is a bit bogus, since it's only used to

chrome/chrome_browser_ui.gypi

@@ -66,7 +66,6 @@
       'sources': [
         # All .cc, .h, .m, and .mm files under browser/ui except for:
         # * tests and mocks.
-        'browser/ui/accelerator_utils.h',
         'browser/ui/active_tab_tracker.cc',
         'browser/ui/active_tab_tracker.h',
         'browser/ui/alternate_error_tab_observer.cc',
@@ -325,7 +324,6 @@
         'browser/ui/chrome_select_file_policy.h',
         'browser/ui/chrome_style.cc',
         'browser/ui/chrome_style.h',
-        'browser/ui/cocoa/accelerator_utils_cocoa.mm',
         'browser/ui/cocoa/about_ipc_controller.h',
         'browser/ui/cocoa/about_ipc_controller.mm',
         'browser/ui/cocoa/about_ipc_dialog.h',
@@ -909,7 +907,6 @@
         'browser/ui/global_error/global_error_service.h',
         'browser/ui/global_error/global_error_service_factory.cc',
         'browser/ui/global_error/global_error_service_factory.h',
-        'browser/ui/gtk/accelerator_utils_gtk.cc',
         'browser/ui/gtk/accelerators_gtk.cc',
         'browser/ui/gtk/accelerators_gtk.h',
         'browser/ui/gtk/action_box_button_gtk.cc',
@@ -1351,8 +1348,8 @@
         'browser/ui/sync/one_click_signin_infobar_delegate.h',
         'browser/ui/sync/one_click_signin_sync_starter.cc',
         'browser/ui/sync/one_click_signin_sync_starter.h',
-        'browser/ui/sync/profile_signin_confirmation_helper.cc',
-        'browser/ui/sync/profile_signin_confirmation_helper.h',
+	'browser/ui/sync/profile_signin_confirmation_helper.cc',
+	'browser/ui/sync/profile_signin_confirmation_helper.h',
         'browser/ui/sync/signin_histogram.h',
         'browser/ui/sync/tab_contents_synced_tab_delegate.cc',
         'browser/ui/sync/tab_contents_synced_tab_delegate.h',
@@ -1427,8 +1424,6 @@
         'browser/ui/unload_controller.h',
         'browser/ui/user_data_dir_dialog.h',
         'browser/ui/view_ids.h',
-        'browser/ui/views/accelerator_utils_aura.cc',
-        'browser/ui/views/accelerator_utils_views.cc',
         'browser/ui/views/about_ipc_dialog.cc',
         'browser/ui/views/about_ipc_dialog.h',
         'browser/ui/views/accessibility/accessibility_event_router_views.cc',

chrome/test/data/extensions/api_test/keybinding/dont_overwrite_system/background.js

-// Copyright (c) 2013 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.
-
-// Called when the user activates the command.
-chrome.commands.onCommand.addListener(function(command) {
-  chrome.tabs.executeScript(null, {
-    code: "document.body.bgColor='" + command + "'" });
-});
-
-chrome.test.notifyPass();

chrome/test/data/extensions/api_test/keybinding/dont_overwrite_system/manifest.json

-{
-  "name": "An extension that tries to overwrite system keybindings",
-  "version": "1.0",
-  "manifest_version": 2,
-  "background": {
-    "scripts": ["background.js"]
-  },
-  "permissions": ["activeTab"],
-  "commands": {
-    "red": {
-      "suggested_key": "Ctrl+F",
-      "description": "Make the page red"
-    },
-    "blue": {
-      "suggested_key": "Alt+Shift+F",
-      "description": "Make the page blue"
-    }
-  }
-}