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

chrome/browser/extensions/extension_keybinding_apitest.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 "base/command_line.h"
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
 #include "chrome/browser/extensions/api/commands/command_service.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
@@ skipping 264 matching lines @@
   bool result = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       tab,
       "setInterval(function() {"
       "  if (document.body.bgColor == 'green') {"
       "    window.domAutomationController.send(true)}}, 100)",
       &result));
   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) {
+  ASSERT_TRUE(test_server()->Start());
+
+  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  // This functionality requires a feature flag.
+  CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      "--enable-override-bookmarks-ui",
+      "1");
+
+  ASSERT_TRUE(RunExtensionTest("keybinding/overwrite_bookmark_shortcut"))
+      << message_;
+
+  ui_test_utils::NavigateToURL(browser(),
+      test_server()->GetURL(
+          "files/extensions/test_file_with_ctrl-d_keybinding.html"));
+
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(tab);
+
+  // Activate the shortcut (Ctrl+D) which should be handled by the page and make
+  // the background color magenta.
+#if defined(OS_MACOSX)
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_D, false, false, false, true));
+#else
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_D, true, false, false, false));
+#endif
+
+  bool result = false;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+      tab,
+      "setInterval(function() {"
+      "  if (document.body.bgColor == 'magenta') {"
+      "    window.domAutomationController.send(true)}}, 100)",
+      &result));
+  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) {
+  ASSERT_TRUE(test_server()->Start());
+
+  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+
+  ASSERT_TRUE(RunExtensionTest("keybinding/basics"))

[Finnur, 2014/07/03 11:09:59]

You should probably set the --enable-override-bookmarks-ui flag here, to make
sure that the absence of that flag isn't causing this test to pass.

[Mike Wittman, 2014/07/07 18:43:58]

Done.

+      << message_;
+
+  CommandService* command_service = CommandService::Get(browser()->profile());
+
+  const Extension* extension = GetSingleLoadedExtension();
+  // Simulate the user setting the keybinding to Ctrl+D.
+  command_service->UpdateKeybindingPrefs(
+      extension->id(), manifest_values::kBrowserActionCommandEvent, "Ctrl+D");
+
+  ui_test_utils::NavigateToURL(browser(),
+      test_server()->GetURL(
+          "files/extensions/test_file_with_ctrl-d_keybinding.html"));
+
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(tab);
+
+  // Activate the shortcut (Ctrl+D) which should be handled by the extension and
+  // make the background color red.
+#if defined(OS_MACOSX)
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_D, false, false, false, true));
+#else
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_D, true, false, false, false));
+#endif
+
+  bool result = false;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+      tab,
+      "setInterval(function() {"
+      "  if (document.body.bgColor == 'red') {"
+      "    window.domAutomationController.send(true)}}, 100)",
+      &result));
+  ASSERT_TRUE(result);
+}
+
 #if defined(OS_WIN)
 // Currently this feature is implemented on Windows only.
 #define MAYBE_AllowDuplicatedMediaKeys AllowDuplicatedMediaKeys
 #else
 #define MAYBE_AllowDuplicatedMediaKeys DISABLED_AllowDuplicatedMediaKeys
 #endif
 
 // Test that media keys go to all extensions that register for them.
 IN_PROC_BROWSER_TEST_F(CommandsApiTest, MAYBE_AllowDuplicatedMediaKeys) {
   ResultCatcher catcher;
@@ skipping 301 matching lines @@
   // Verify the keybinding is still set.
   accelerator = command_service->FindCommandByName(
       kId, manifest_values::kBrowserActionCommandEvent).accelerator();
   EXPECT_EQ(ui::VKEY_G, accelerator.key_code());
   EXPECT_FALSE(accelerator.IsCtrlDown());
   EXPECT_TRUE(accelerator.IsShiftDown());
   EXPECT_TRUE(accelerator.IsAltDown());
 }
 
 }  // namespace extensions