mac: Allow WebContents key handling to supplant extension overrides of the bookmark shortcut.

chrome/browser/extensions/extension_keybinding_apitest.cc

Index: chrome/browser/extensions/extension_keybinding_apitest.cc
diff --git a/chrome/browser/extensions/extension_keybinding_apitest.cc b/chrome/browser/extensions/extension_keybinding_apitest.cc
index 14a755f4542097615643386f9257282081c9ab0b..a98be699f30eb5d818ecc825ae171352b00a371a 100644
--- a/chrome/browser/extensions/extension_keybinding_apitest.cc
+++ b/chrome/browser/extensions/extension_keybinding_apitest.cc
@@ -282,17 +282,10 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, OverwriteBookmarkShortcut) {
   ASSERT_TRUE(result);
 }
 
-// Behavior to be implemented on Mac. See http://crbug.com/389340.
-#if defined(OS_MACOSX)
-#define MAYBE_OverwriteBookmarkShortcutDoesNotOverrideWebKeybinding DISABLED_OverwriteBookmarkShortcutDoesNotOverrideWebKeybinding
-#else
-#define MAYBE_OverwriteBookmarkShortcutDoesNotOverrideWebKeybinding OverwriteBookmarkShortcutDoesNotOverrideWebKeybinding
-#endif
 // This test validates that an extension override of the Chrome bookmark
 // shortcut does not supersede the same keybinding by web pages.
-IN_PROC_BROWSER_TEST_F(
-    CommandsApiTest,
-    MAYBE_OverwriteBookmarkShortcutDoesNotOverrideWebKeybinding) {
+IN_PROC_BROWSER_TEST_F(CommandsApiTest,
+                       OverwriteBookmarkShortcutDoesNotOverrideWebKeybinding) {
   ASSERT_TRUE(test_server()->Start());
 
   ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
@@ -332,18 +325,11 @@ IN_PROC_BROWSER_TEST_F(
   ASSERT_TRUE(result);
 }
 
-// Behavior to be implemented on Mac. See http://crbug.com/389340.
-#if defined(OS_MACOSX)
-#define MAYBE_OverwriteBookmarkShortcutByUserOverridesWebKeybinding DISABLED_OverwriteBookmarkShortcutByUserOverridesWebKeybinding
-#else
-#define MAYBE_OverwriteBookmarkShortcutByUserOverridesWebKeybinding OverwriteBookmarkShortcutByUserOverridesWebKeybinding
-#endif
 // This test validates that user-set override of the Chrome bookmark shortcut in
 // an extension that does not request it does supersede the same keybinding by
 // web pages.
-IN_PROC_BROWSER_TEST_F(
-    CommandsApiTest,
-    MAYBE_OverwriteBookmarkShortcutByUserOverridesWebKeybinding) {
+IN_PROC_BROWSER_TEST_F(CommandsApiTest,
+                       OverwriteBookmarkShortcutByUserOverridesWebKeybinding) {
   ASSERT_TRUE(test_server()->Start());
 
   ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
@@ -360,8 +346,15 @@ IN_PROC_BROWSER_TEST_F(
 
   const Extension* extension = GetSingleLoadedExtension();
   // Simulate the user setting the keybinding to Ctrl+D.
+#if defined(OS_MACOSX)
+  command_service->UpdateKeybindingPrefs(
+      extension->id(),
+      manifest_values::kBrowserActionCommandEvent,
+      "Command+D");
+#else
   command_service->UpdateKeybindingPrefs(
       extension->id(), manifest_values::kBrowserActionCommandEvent, "Ctrl+D");
+#endif  // defined(OS_MACOSX)

[Scott Hess - ex-Googler, 2014/07/15 19:20:59]

AFAICT, these only differ on "Command+D" vs "Ctrl+D".  Maybe better to have the
#ifdef around a local constant with a single function call.  [Mostly my
complaint is around having the two bits of code look very different.]

[Yoyo Zhou, 2014/07/15 19:46:37]

Agreed.

[erikchen, 2014/07/15 20:48:40]

Done.

 
   ui_test_utils::NavigateToURL(browser(),
       test_server()->GetURL(