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..0db16364bf7ae0b9cd9f2ac2f8bc07928a16f365
--- /dev/null
+++ b/chrome/browser/ui/browser_command_controller_interactive_browsertest.cc
@@ -0,0 +1,397 @@
+// 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 <string>
+
+#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/test/base/interactive_test_utils.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/browser_test_utils.h"
+#include "ui/events/keycodes/keyboard_codes.h"
+#include "url/gurl.h"
+
+namespace {
+constexpr char kFullscreenKeyboardLockHTML[] =

[msw, 2017/06/06 20:32:07]

nit: comment on what this is

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+    "/fullscreen_keyboardlock/fullscreen_keyboardlock.html";
+
+class FullscreenNotificationObserver

[msw, 2017/06/06 20:32:07]

Avoid this subclass, just construct a WindowedNotificationObserver instance with
the arguments you need and call Wait() on that, right?

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+    : public content::WindowedNotificationObserver {

[msw, 2017/06/06 20:32:06]

nit: include content/public/test/test_utils.h to use this

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+ public:
+  FullscreenNotificationObserver() : WindowedNotificationObserver(
+      chrome::NOTIFICATION_FULLSCREEN_CHANGED,
+      content::NotificationService::AllSources()) {}
+  ~FullscreenNotificationObserver() override {
+    Wait();
+  }
+};
+
+}  // namespace
+
+using BrowserCommandControllerInteractiveTest = InProcessBrowserTest;
+
+#if defined(OS_MACOSX)
+// On Mac OSX, accelerators cannot receive the events sent from
+// SendKeyPressSync(), so the following tests cannot work.
+// TODO(zijiehe): Make SendKeyPressSync() respect shortcuts on Mac OSX.

[msw, 2017/06/06 20:32:06]

File a bug; cite its number here.

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+#define MAYBE(X) DISABLED_##X
+#elif defined(OS_CHROMEOS)
+// Crash on ChromeOS
+// BrowserTestBase received signal: Terminated. Backtrace:

[msw, 2017/06/06 20:32:06]

Don't include a stack trace here; address this before landing or investigate
sufficiently to file an actionable bug. ie. why does this crash? (if nothing
else, use a binary search of removing test code and commenting out support code
to determine what triggers this, we can obviously run some browser_tests on
chrome os)

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+// #0 0x000003363eec base::debug::StackTrace::StackTrace()
+// #1 0x0000029b7f62 content::(anonymous namespace)::DumpStackTraceSignalHandler()
+// #2 0x7f0297078cb0 <unknown>
+// #3 0x7f029713284d __poll
+// #4 0x7f029ade5fe4 <unknown>
+// #5 0x7f029ade60ec g_main_context_iteration
+// #6 0x000003386825 base::MessagePumpGlib::Run()
+// #7 0x0000033840eb base::MessageLoop::Run()
+// #8 0x0000033ae25a base::RunLoop::Run()
+// #9 0x0000029c2726 content::MessageLoopRunner::Run()
+// #10 0x0000029c2b9d content::WindowedNotificationObserver::Wait()
+// TODO(zijiehe): Fix crash on ChromeOS.
+#define MAYBE(X) DISABLED_##X
+#elif defined(OS_WIN)
+// Tests depending on keyboard inputs are flaky on Windows, see test case

[msw, 2017/06/06 20:32:07]

Citing that file isn't sufficient (I don't see what 'test case' you mean there),
cite/file a bug, or address this by finding a non-flaky way of doing this on
Windows, I can't believe that all keyboard inputs in browser tests are flaky on
windows, otherwise there would be a Pri-0 bug to fix that...

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+// https://cs.chromium.org/chromium/src/chrome/browser/browser_keyevents_browsertest.cc
+// for detail.
+#define MAYBE(X) DISABLED_##X

[msw, 2017/06/06 20:32:06]

Sorry I don't have any great ideas for making these tests more reliable and
consistent. Maybe try event generators, look through other tests in the
interactive_ui_tests target, or ping chrome/test/OWNERS? 

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+#else
+#define MAYBE(X) X
+#endif
+
+IN_PROC_BROWSER_TEST_F(BrowserCommandControllerInteractiveTest,
+                       MAYBE(InF11Fullscreen)) {

[msw, 2017/06/06 20:32:06]

nit: use a better name here, what is this test checking? ie.
KeyEventsSentToPageWithShortcutTriggeredFullscreen or use a simpler name like
EventsInShortcutTriggeredFullscreen and add a comment above the test like "Test
which key events are sent to the page when fullscreen is triggered by the user
pressing the fullscreen shortcut.", ditto for other tests below (use a
consistent naming scheme and add explanatory comments)

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+  ASSERT_TRUE(embedded_test_server()->Start());
+  {
+    FullscreenNotificationObserver fullscreen_observer;
+    // Enter fullscreen.
+#if defined(OS_MACOSX)

[msw, 2017/06/06 20:32:06]

nit: add a local helper: void PressFullscreenShortcut() { #if... }

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+          browser(), ui::VKEY_F, true, false, false, true));
+#else
+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+          browser(), ui::VKEY_F11, false, false, false, false));
+#endif
+  }
+  ui_test_utils::NavigateToURL(browser(),
+      embedded_test_server()->GetURL(kFullscreenKeyboardLockHTML));
+#if defined(OS_MACOSX)

[msw, 2017/06/06 20:32:07]

nit: add a local helper: bool PressKeySync(ui::KeyboardCode key) { #if... }
Optionally pass a string pointer for it to append the expected keydown/keyup
report sequences, to avoid hardcoding the super long string literals below (that
should facilitate test readability/maintainability/extensibility)

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+  const bool control_modifier = false;
+  const bool command_modifier = true;
+#else
+  const bool control_modifier = true;
+  const bool command_modifier = false;
+#endif
+  // Close tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_W, control_modifier, false, false, command_modifier));
+  // Close window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_W, control_modifier, true, false, command_modifier));
+  // New tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_T, control_modifier, false, false, command_modifier));
+  // New window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_N, control_modifier, false, false, command_modifier));
+  // New incognito window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_N, control_modifier, true, false, command_modifier));
+  // Restore tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_T, control_modifier, true, false, command_modifier));
+  // Next tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_TAB, control_modifier, false, false, command_modifier));
+  // Previous tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_TAB, control_modifier, true, false, command_modifier));
+  // Esc.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_ESCAPE, false, false, false, false));
+  // Magic key X to stop the test.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_X, false, false, false, false));
+
+  std::string result;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      browser()->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost(),
+      "report()",
+      &result));
+#if defined(OS_MACOSX)
+  base::ReplaceSubstringsAfterOffset(&result, 0, "MetaLeft", "ControlLeft");
+#endif
+  base::TrimWhitespaceASCII(result, base::TRIM_ALL, &result);
+  ASSERT_EQ(result,
+            "keydown ControlLeft\n"
+            "keydown KeyW\n"
+            "keyup KeyW\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyW\n"
+            "keyup KeyW\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown KeyT\n"
+            "keyup KeyT\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown KeyN\n"
+            "keyup KeyN\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyN\n"
+            "keyup KeyN\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyT\n"
+            "keyup KeyT\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown Tab\n"
+            "keyup Tab\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown Tab\n"
+            "keyup Tab\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown Escape\n"
+            "keyup Escape\n"
+            "keydown KeyX\n"
+            "keyup KeyX");
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserCommandControllerInteractiveTest,
+                       MAYBE(ExitHTMLFullscreenWithEsc)) {

[msw, 2017/06/06 20:32:06]

nit: try to use a consistent naming pattern for these test fixtures ie.
KeyEventsSentToPageWithHtmlTriggeredFullscreen

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+  ASSERT_TRUE(embedded_test_server()->Start());
+  ui_test_utils::NavigateToURL(browser(),
+      embedded_test_server()->GetURL(kFullscreenKeyboardLockHTML));
+  {
+    FullscreenNotificationObserver fullscreen_observer;
+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+          browser(), ui::VKEY_S, false, false, false, false));

[msw, 2017/06/06 20:32:07]

q: Is 's' another magic key to make the page trigger fullscreen? Add a comment.

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+  }
+#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
+  // Close tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_W, control_modifier, false, false, command_modifier));
+  // Close window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_W, control_modifier, true, false, command_modifier));
+  // New tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_T, control_modifier, false, false, command_modifier));
+  // New window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_N, control_modifier, false, false, command_modifier));
+  // New incognito window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_N, control_modifier, true, false, command_modifier));
+  // Restore tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_T, control_modifier, true, false, command_modifier));
+  // Next tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_TAB, control_modifier, false, false, command_modifier));
+  // Previous tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_TAB, control_modifier, true, false, command_modifier));
+  {
+    FullscreenNotificationObserver fullscreen_observer;
+    // Esc.

[msw, 2017/06/06 20:32:06]

nit: "// Esc should exit fullscreen, when ..." (explain the condition under
which Esc will exit fullscreen, like "when the page triggers fullscreen
itself"... I don't know exactly when that's the case...) Maybe also note that
the webpage should not receive the Esc keypress (since it's not in the expected
string)

[Hzj_jie, 2017/06/28 00:34:43]

Done.

+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+        ui::VKEY_ESCAPE, false, false, false, command_modifier));
+  }
+  // Magic key X to stop the test.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_X, false, false, false, false));
+
+  std::string result;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      browser()->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost(),
+      "report()",
+      &result));
+#if defined(OS_MACOSX)
+  base::ReplaceSubstringsAfterOffset(&result, 0, "MetaLeft", "ControlLeft");
+#endif
+  base::TrimWhitespaceASCII(result, base::TRIM_ALL, &result);
+  ASSERT_EQ(result,
+            "keydown KeyS\n"
+            "keyup KeyS\n"
+            "keydown ControlLeft\n"
+            "keydown KeyW\n"
+            "keyup KeyW\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyW\n"
+            "keyup KeyW\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown KeyT\n"
+            "keyup KeyT\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown KeyN\n"
+            "keyup KeyN\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyN\n"
+            "keyup KeyN\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyT\n"
+            "keyup KeyT\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown Tab\n"
+            "keyup Tab\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown Tab\n"
+            "keyup Tab\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown KeyX\n"
+            "keyup KeyX");
+}
+
+IN_PROC_BROWSER_TEST_F(BrowserCommandControllerInteractiveTest,
+                       MAYBE(ExitHTMLFullscreenWithF11)) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  ui_test_utils::NavigateToURL(browser(),
+      embedded_test_server()->GetURL(kFullscreenKeyboardLockHTML));
+  {
+    FullscreenNotificationObserver fullscreen_observer;
+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+          browser(), ui::VKEY_S, false, false, false, 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
+  // Close tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_W, control_modifier, false, false, command_modifier));
+  // Close window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_W, control_modifier, true, false, command_modifier));
+  // New tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_T, control_modifier, false, false, command_modifier));
+  // New window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_N, control_modifier, false, false, command_modifier));
+  // New incognito window.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_N, control_modifier, true, false, command_modifier));
+  // Restore tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_T, control_modifier, true, false, command_modifier));
+  // Next tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_TAB, control_modifier, false, false, command_modifier));
+  // Previous tab.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_TAB, control_modifier, true, false, command_modifier));
+  {
+    FullscreenNotificationObserver fullscreen_observer;
+    // Exit fullscreen.
+#if defined(OS_MACOSX)
+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+          browser(), ui::VKEY_F, true, false, false, true));
+#else
+    ASSERT_TRUE(ui_test_utils::SendKeyPressSync(
+          browser(), ui::VKEY_F11, false, false, false, false));
+#endif
+  }
+  // Magic key X to stop the test.
+  ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(),
+      ui::VKEY_X, false, false, false, false));
+
+  std::string result;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      browser()->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost(),
+      "report()",
+      &result));
+#if defined(OS_MACOSX)
+  base::ReplaceSubstringsAfterOffset(&result, 0, "MetaLeft", "ControlLeft");
+#endif
+  base::TrimWhitespaceASCII(result, base::TRIM_ALL, &result);
+  ASSERT_EQ(result,
+            "keydown KeyS\n"
+            "keyup KeyS\n"
+            "keydown ControlLeft\n"
+            "keydown KeyW\n"
+            "keyup KeyW\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyW\n"
+            "keyup KeyW\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown KeyT\n"
+            "keyup KeyT\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown KeyN\n"
+            "keyup KeyN\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyN\n"
+            "keyup KeyN\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown KeyT\n"
+            "keyup KeyT\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown Tab\n"
+            "keyup Tab\n"
+            "keyup ControlLeft\n"
+            "keydown ControlLeft\n"
+            "keydown ShiftLeft\n"
+            "keydown Tab\n"
+            "keyup Tab\n"
+            "keyup ShiftLeft\n"
+            "keyup ControlLeft\n"
+            "keydown KeyX\n"
+            "keyup KeyX");
+}