Add BrowserCommandController Interactive Test

chrome/browser/ui/browser_command_controller_interactive_browsertest.cc

Index: chrome/browser/ui/browser_command_controller_interactive_browsertest.cc
diff --git a/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc b/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5140dc8b71d48b9f123fa5f214a1ca2a293303a4
--- /dev/null
+++ b/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc
@@ -0,0 +1,379 @@
+// Copyright 2017 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 <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_util.h"
+#include "build/build_config.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
+#include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/events/keycodes/keyboard_code_conversion.h"
+#include "ui/events/keycodes/dom/keycode_converter.h"
+#include "url/gurl.h"
+
+namespace {
+// The html file to receive key events, prevent defaults and export all the
+// events with "getKeyEventReport()" function. It has two magic keys: pressing
+// "S" to enter fullscreen mode; pressing "X" to indicate the end of all the
+// keys.
+constexpr char kFullscreenKeyboardLockHTML[] = "/fullscreen_keyboardlock.html";
+
+// On MacOSX command key is used for most of the shortcuts, so replace it with
+// control to reduce the complexity of comparison of the results.
+#if defined(OS_MACOSX)

[msw, 2017/07/05 19:11:45]

nit: move the #if block inside the function, all platforms can share one
function declaration:
// On MacOSX command key is used for most of the shortcuts, so replace it with
// control to reduce the complexity of comparison of the results.
void NormalizeMetaKeyForMacOS(std::string* output) {
#if defined(OS_MACOSX)
  base::ReplaceSubstringsAfterOffset(output, 0, "MetaLeft", "ControlLeft");
#endif
}

[Hzj_jie, 2017/07/07 23:18:10]

Done.

+void NormalizeMetaKeyForMacOS(std::string* output) {
+  base::ReplaceSubstringsAfterOffset(output, 0, "MetaLeft", "ControlLeft");
+}
+#else
+// Avoid unused variable warning.
+void NormalizeMetaKeyForMacOS(std::string*) {}
+#endif
+
+}  // namespace
+
+class BrowserCommandControllerInteractiveTest : public InProcessBrowserTest {
+ public:
+  BrowserCommandControllerInteractiveTest() = default;
+  ~BrowserCommandControllerInteractiveTest() override = default;
+
+ protected:
+  // Starts the test page and waits for it to be loaded.
+  void StartTestPage();
+
+  // Wait for the browser to have the expected tab count or timeout.
+  void WaitForTabCount(int tab_count) const;
+
+  // Sends a control or command + |key| shortcut to the focused window. Shift
+  // modifier will be added if |shift| is true.
+  void SendShortcut(ui::KeyboardCode key, bool shift = false);
+
+  // Sends a control or command + shift + |key| shortcut to the focused window.
+  void SendShiftShortcut(ui::KeyboardCode key);
+
+  // Sends a fullscreen shortcut to the focused window and wait for the
+  // operation to take effect.
+  void SendFullscreenShortcutAndWait();
+
+  // Sends a KeyS to the focused window to trigger JavaScript fullscreen and
+  // wait for the operation to take effect.
+  void SendJsFullscreenShortcutAndWait();
+
+  // Sends an ESC to the focused window.
+  void SendEscape();
+
+  // Sends an ESC to the focused window to exit JavaScript fullscreen and wait
+  // for the operation to take effect.
+  void SendEscapeAndWaitForExitingFullscreen();
+
+  // Sends a magic KeyX to the focused window to stop the test case, receives
+  // the result and verifies if it equals to |expected_result_|.

[msw, 2017/07/05 19:11:45]

nit: "if it equals |expected_result_|." of "if it is equal to..."

[Hzj_jie, 2017/07/07 23:18:10]

Done.

+  void FinishTestAndVerifyResult();
+
+ private:
+  void SetUpOnMainThread() override;
+
+  std::string expected_result_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserCommandControllerInteractiveTest);
+};
+
+void BrowserCommandControllerInteractiveTest::StartTestPage() {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      embedded_test_server()->GetURL(kFullscreenKeyboardLockHTML),
+      WindowOpenDisposition::CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+}
+
+void BrowserCommandControllerInteractiveTest::WaitForTabCount(
+    int tab_count) const {
+  while (browser()->tab_strip_model()->count() != tab_count)
+    content::RunAllPendingInMessageLoop();
+}
+
+void BrowserCommandControllerInteractiveTest::SendShortcut(
+    ui::KeyboardCode key,
+    bool shift /* = false */) {
+#if defined(OS_MACOSX)
+  const bool control_modifier = false;
+  const bool command_modifier = true;
+#else
+  const bool control_modifier = true;
+  const bool command_modifier = false;
+#endif
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      key, control_modifier, shift, false, command_modifier));
+
+  // 1. ui_controls_mac.mm puts shift key before meta (command) key.

[msw, 2017/07/05 19:11:45]

Trent's point about test fragility is very much worth considering. Instead of
the html/js reporting the entire sequence of down and up events (like "keydown
ShiftLeft\nkeydown ControlLeft\nkeydown keyW\nkeyup keyW\n\nkeyup
ControlLeft\nkeyup ShiftLeft\n"), can the it just report the state of the
modifiers at the time of the non-modifier keys (like "key:w ctrl:1 shift:1 alt:0
meta:0\n" or similar)? That would make this command controller test completely
agnostic to differences in generated sequences and robust against bugs/changes
with unreported keyup events, etc.

[Hzj_jie, 2017/07/07 23:18:10]

A good suggestion to avoid addressing the order of command key.

But I still have concerns regarding to ignoring keyup events: they should not
happen at all. Since you two have the same opinion, I will modify the html page
to apply your suggestion.

[msw, 2017/07/10 18:48:17]

Feel free to add a separate keyup behavior test in a followup CL.

[Hzj_jie, 2017/07/11 02:46:07]

Acknowledged.

+  // https://cs.chromium.org/chromium/src/ui/base/test/ui_controls_mac.mm?type=cs&q=SynthesizeKeyEventsSequence&sq=package:chromium&l=67
+  // 2. To match safari, key up event won't fire if meta (command) key is down.
+  // See http://crbug.com/126282.
+  std::string code_string = ui::KeycodeConverter::DomCodeToCodeString(
+      ui::UsLayoutKeyboardCodeToDomCode(key));
+#if defined(OS_MACOSX)
+  if (shift) {
+    expected_result_ += "keydown ShiftLeft\n";
+  }
+  expected_result_ += "keydown ControlLeft\n";
+  expected_result_ += "keydown ";
+  expected_result_ += code_string;
+  expected_result_ += "\n";
+  expected_result_ += "keyup ControlLeft\n";
+  if (shift) {
+    expected_result_ += "keyup ShiftLeft\n";
+  }
+#else
+  expected_result_ += "keydown ControlLeft\n";
+  if (shift) {
+    expected_result_ += "keydown ShiftLeft\n";
+  }
+  expected_result_ += "keydown ";
+  expected_result_ += code_string;
+  expected_result_ += "\n";
+  expected_result_ += "keyup ";
+  expected_result_ += code_string;
+  expected_result_ += "\n";
+  if (shift) {
+    expected_result_ += "keyup ShiftLeft\n";
+  }
+  expected_result_ += "keyup ControlLeft\n";
+#endif
+}
+
+void BrowserCommandControllerInteractiveTest::SendShiftShortcut(
+    ui::KeyboardCode key) {
+  SendShortcut(key, true);
+}
+
+void BrowserCommandControllerInteractiveTest::SendFullscreenShortcutAndWait() {
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_FULLSCREEN_CHANGED,
+      content::NotificationService::AllSources());
+  // Enter fullscreen.
+#if defined(OS_MACOSX)
+  // On MACOSX, Command + Control + F is used.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+        browser(), ui::VKEY_F, true, false, false, true));
+  // Both Command and Control key presses are still catchable by web page.
+  expected_result_ += "keydown ControlLeft\n"
+                      "keydown MetaLeft\n"
+                      "keyup MetaLeft\n"
+                      "keyup ControlLeft\n";
+#elif defined(OS_CHROMEOS)
+  // A dedicated fullscreen key is used on Chrome OS, so send a fullscreen
+  // command directly instead, to avoid constructing the key press.
+  ASSERT_TRUE(chrome::ExecuteCommand(browser(), IDC_FULLSCREEN));
+#else
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+        browser(), ui::VKEY_F11, false, false, false, false));
+#endif
+
+  observer.Wait();
+}
+
+void
+BrowserCommandControllerInteractiveTest::SendJsFullscreenShortcutAndWait() {
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_FULLSCREEN_CHANGED,
+      content::NotificationService::AllSources());
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+        browser(), ui::VKEY_S, false, false, false, false));
+  expected_result_ += "keydown KeyS\n";
+  expected_result_ += "keyup KeyS\n";
+  observer.Wait();
+}
+
+void BrowserCommandControllerInteractiveTest::SendEscape() {
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+        browser(), ui::VKEY_ESCAPE, false, false, false, false));
+  expected_result_ += "keydown Escape\n";
+  expected_result_ += "keyup Escape\n";
+}
+
+void BrowserCommandControllerInteractiveTest
+    ::SendEscapeAndWaitForExitingFullscreen() {
+  content::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_FULLSCREEN_CHANGED,
+      content::NotificationService::AllSources());
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+        browser(), ui::VKEY_ESCAPE, false, false, false, false));
+  observer.Wait();
+}
+
+void BrowserCommandControllerInteractiveTest::FinishTestAndVerifyResult() {
+  // Magic KeyX to stop the test.
+  EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_X, false, false, false, false));
+  expected_result_ += "keydown KeyX\n";
+  expected_result_ += "keyup KeyX";
+  std::string result;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      browser()->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost(),
+      "getKeyEventReport();",
+      &result));
+  NormalizeMetaKeyForMacOS(&result);
+  NormalizeMetaKeyForMacOS(&expected_result_);
+  base::TrimWhitespaceASCII(result, base::TRIM_ALL, &result);
+  ASSERT_EQ(result, expected_result_);

[msw, 2017/07/05 19:11:45]

nit: ASSERT_EQ and EXPECTED_EQ macros should have the expected value as the
first argument, and the actual value as the second argument.

[Hzj_jie, 2017/07/07 23:18:10]

Done.

+}
+
+void BrowserCommandControllerInteractiveTest::SetUpOnMainThread() {
+  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserCommandControllerInteractiveTest,
+                       ShortcutsShouldTakeEffectInWindowMode) {
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  SendShortcut(ui::VKEY_T);
+  WaitForTabCount(2);
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 2);
+  SendShortcut(ui::VKEY_T);
+  WaitForTabCount(3);
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 3);
+  SendShortcut(ui::VKEY_W);
+  WaitForTabCount(2);
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 2);
+  SendShortcut(ui::VKEY_W);
+  WaitForTabCount(1);
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  SendFullscreenShortcutAndWait();
+  ASSERT_TRUE(browser()->
+      exclusive_access_manager()->
+      fullscreen_controller()->
+      IsFullscreenForBrowser());
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserCommandControllerInteractiveTest,
+                       UnpreservedShortcutsShouldBePreventable) {
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  StartTestPage();
+
+  // The browser print function should be blocked by the web page.
+  SendShortcut(ui::VKEY_P);
+  // The system print function should be blocked by the web page.
+  SendShiftShortcut(ui::VKEY_P);
+  FinishTestAndVerifyResult();
+}
+
+#if defined(OS_MACOSX)
+// TODO(zijiehe): Figure out why this test crashes on Mac OSX. The suspicious
+// command is "SendFullscreenShortcutAndWait()". See, http://crbug.com/738949.
+#define MAYBE_KeyEventsShouldBeConsumedByWebPageInBrowserFullscreen \
+    DISABLED_KeyEventsShouldBeConsumedByWebPageInBrowserFullscreen
+#else
+#define MAYBE_KeyEventsShouldBeConsumedByWebPageInBrowserFullscreen \
+    KeyEventsShouldBeConsumedByWebPageInBrowserFullscreen
+#endif
+IN_PROC_BROWSER_TEST_F(
+    BrowserCommandControllerInteractiveTest,
+    MAYBE_KeyEventsShouldBeConsumedByWebPageInBrowserFullscreen) {
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  StartTestPage();
+
+  SendFullscreenShortcutAndWait();
+  // The tab should not be closed.

[msw, 2017/07/05 19:11:45]

Can you make a helper function that sends these commands, since the code is
repeated three times? The code should also test for the expected behavior. For
example:

void CheckShortcutsInFullscreen() {
  // Add a second tab for counting and focus purposes.
  AddTabAtIndex(1, GURL("about:blank"), ui::PAGE_TRANSITION_LINK);
  ASSERT_EQ(2, browser()->tab_strip_model()->count());
  ASSERT_EQ(0, browser()->tab_strip_model()->active_index());
  ASSERT_EQ(1, BrowserList::GetInstance()->size());

  // The tab should not be closed.
  SendShortcut(ui::VKEY_W);
  ASSERT_EQ(2, browser()->tab_strip_model()->count());
  // The window should not be closed.
  SendShiftShortcut(ui::VKEY_W);
  ASSERT_EQ(1, BrowserList::GetInstance()->size());
  // TODO(zijiehe): ChromeOS incorrectly handles these;
  // see http://crbug.com/737307.
#if !defined(OS_CHROMEOS)
  // A new tab should not be created.
  SendShortcut(ui::VKEY_T);
  ASSERT_EQ(2, browser()->tab_strip_model()->count());
  // A new window should not be created.
  SendShortcut(ui::VKEY_N);
  ASSERT_EQ(1, BrowserList::GetInstance()->size());
  // A new incognito window should not be created.
  SendShiftShortcut(ui::VKEY_N);
  ASSERT_EQ(1, BrowserList::GetInstance()->size());
  // Last closed tab should not be restored.
  SendShiftShortcut(ui::VKEY_T);
  ASSERT_EQ(2, browser()->tab_strip_model()->count());
#endif
  // Browser should not switch to the next tab.
  SendShortcut(ui::VKEY_TAB);
  ASSERT_EQ(0, browser()->tab_strip_model()->active_index());
  // Browser should not switch to the previous tab.
  SendShiftShortcut(ui::VKEY_TAB);
  ASSERT_EQ(0, browser()->tab_strip_model()->active_index());
}

[Hzj_jie, 2017/07/07 23:18:10]

AddTabAtIndex() always creates a focused tab and cause other tabs to exit
fullscreen mode. So I moved the logic into StartTestPage() to ensure the test
page can be correctly loaded in the focused tab.

+  SendShortcut(ui::VKEY_W);
+  // The window should not be closed.
+  SendShiftShortcut(ui::VKEY_W);
+  // TODO(zijiehe): ChromeOS incorrectly handles these;
+  // see http://crbug.com/737307.
+#if !defined(OS_CHROMEOS)
+  // A new tab should not be created.
+  SendShortcut(ui::VKEY_T);
+  // A new window should not be created.
+  SendShortcut(ui::VKEY_N);
+  // A new incognito window should not be created.
+  SendShiftShortcut(ui::VKEY_N);
+  // Last closed tab should not be restored.
+  SendShiftShortcut(ui::VKEY_T);
+#endif
+  // Browser should not switch to the next tab.
+  SendShortcut(ui::VKEY_TAB);
+  // Browser should not switch to the previous tab.
+  SendShiftShortcut(ui::VKEY_TAB);
+  // Current page should not exit browser fullscreen mode.
+  SendEscape();
+
+  FinishTestAndVerifyResult();
+}
+
+IN_PROC_BROWSER_TEST_F(
+    BrowserCommandControllerInteractiveTest,
+    KeyEventsShouldBeConsumedByWebPageInJsFullscreenExceptForEsc) {
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  StartTestPage();
+
+  SendJsFullscreenShortcutAndWait();
+  // The tab should not be closed.
+  SendShortcut(ui::VKEY_W);
+  // The window should not be closed.
+  SendShiftShortcut(ui::VKEY_W);
+  // TODO(zijiehe): ChromeOS incorrectly handles these;
+  // see http://crbug.com/737307.
+#if !defined(OS_CHROMEOS)
+  // A new tab should not be created.
+  SendShortcut(ui::VKEY_T);
+  // A new window should not be created.
+  SendShortcut(ui::VKEY_N);
+  // A new incognito window should not be created.
+  SendShiftShortcut(ui::VKEY_N);
+  // Last closed tab should not be restored.
+  SendShiftShortcut(ui::VKEY_T);
+#endif
+  // Browser should not switch to the next tab.
+  SendShortcut(ui::VKEY_TAB);
+  // Browser should not switch to the previous tab.
+  SendShiftShortcut(ui::VKEY_TAB);
+  // Current page should exit HTML fullscreen mode.
+  SendEscapeAndWaitForExitingFullscreen();
+
+  FinishTestAndVerifyResult();
+}
+
+IN_PROC_BROWSER_TEST_F(
+    BrowserCommandControllerInteractiveTest,
+    KeyEventsShouldBeConsumedByWebPageInJsFullscreenExceptForF11) {
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 1);
+  StartTestPage();
+
+  SendJsFullscreenShortcutAndWait();
+  // The tab should not be closed.
+  SendShortcut(ui::VKEY_W);
+  // The window should not be closed.
+  SendShiftShortcut(ui::VKEY_W);
+  // TODO(zijiehe): ChromeOS incorrectly handles these;
+  // see http://crbug.com/737307.
+#if !defined(OS_CHROMEOS)
+  // A new tab should not be created.
+  SendShortcut(ui::VKEY_T);
+  // A new window should not be created.
+  SendShortcut(ui::VKEY_N);
+  // A new incognito window should not be created.
+  SendShiftShortcut(ui::VKEY_N);
+  // Last closed tab should not be restored.
+  SendShiftShortcut(ui::VKEY_T);
+#endif
+  // Browser should not switch to the next tab.
+  SendShortcut(ui::VKEY_TAB);
+  // Browser should not switch to the previous tab.
+  SendShiftShortcut(ui::VKEY_TAB);
+  // Current page should exit browser fullscreen mode.
+  SendFullscreenShortcutAndWait();
+
+  FinishTestAndVerifyResult();
+}