Fix CommandsApiTest flakiness for SingleProcessMash

This CL fixes CommandsApiTest.PageActionOverrideChromeShortcut
flakiness.

This is flaky because the focus of view (OmniboxViewViews in this case)
is asynchronous in Mash, it gets done asynchronously after ctrl-l
is marked as processed. And actually InputMethodMus marks keyevents
before focus (pending keyevents) as done without processing
(see https://chromium.googlesource.com/chromium/src/+/master/ui/aura/mus/input_method_mus.cc#172)

Considering the test scenario, I believe the test case
should wait for the omnibox getting focused.

BUG=894911
TEST=interactive_ui_tests --enable-features=SingleProcessMash

Change-Id: I1e64370924be606badd44ec9fd99c4a39759d28a
Reviewed-on: https://chromium-review.googlesource.com/c/1281907Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600197}

chrome/browser/extensions/extension_keybinding_apitest.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/browser_action_test_util.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views_mode_controller.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -350,6 +351,8 @@ IN_PROC_BROWSER_TEST_F(CommandsApiTest, PageActionOverrideChromeShortcut) {
                                               control_is_modifier, false, false,
                                               command_is_modifier));
 
+  ui_test_utils::WaitUntilViewFocused(browser(), VIEW_ID_OMNIBOX);
+
   // Activate the shortcut.
   ASSERT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_P,
                                               control_is_modifier, false, false,

chrome/test/base/interactive_test_utils.h

@@ -92,6 +92,9 @@ void ClickOnView(const Browser* browser, ViewID vid);
 // Makes focus shift to the given View without clicking it.
 void FocusView(const Browser* browser, ViewID vid);
 
+// Wait until the view identified by |view_id| in |browser| gets focused.
+void WaitUntilViewFocused(const Browser* browser, ViewID view_id);
+
 // A collection of utilities that are used from interactive_ui_tests. These are
 // separated from ui_test_utils.h to ensure that browser_tests don't use them,
 // since they depend on focus which isn't possible for sharded test.

chrome/test/base/interactive_test_utils_aura.cc

@@ -4,10 +4,47 @@
 
 #include "chrome/test/base/interactive_test_utils_aura.h"
 
+#include "base/run_loop.h"
 #include "build/build_config.h"
 #include "chrome/test/base/interactive_test_utils.h"
+#include "ui/aura/client/focus_change_observer.h"
+#include "ui/aura/client/focus_client.h"
 #include "ui/aura/window.h"
 
+namespace {
+
+class FocusWaiter : public aura::client::FocusChangeObserver {
+ public:
+  explicit FocusWaiter(aura::Window* window) : window_(window) {
+    aura::client::GetFocusClient(window_)->AddObserver(this);
+  }
+
+  ~FocusWaiter() override {
+    aura::client::GetFocusClient(window_)->RemoveObserver(this);
+  }
+
+  void WaitUntilFocus() {
+    if (window_->HasFocus())
+      return;
+    run_loop_.Run();
+  }
+
+ private:
+  // aura::client::FocusChangeObserver:
+  void OnWindowFocused(aura::Window* gained_focus,
+                       aura::Window* lost_focus) override {
+    if (gained_focus == window_)
+      run_loop_.QuitWhenIdle();
+  }
+
+  aura::Window* window_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(FocusWaiter);
+};
+
+}  // namespace
+
 namespace ui_test_utils {
 
 #if !defined(OS_WIN)
@@ -30,4 +67,9 @@ bool ShowAndFocusNativeWindowAura(gfx::NativeWindow window) {
   return true;
 }
 
+void WaitUntilWindowFocused(aura::Window* window) {
+  FocusWaiter waiter(window);
+  waiter.WaitUntilFocus();
+}
+
 }  // namespace ui_test_utils

chrome/test/base/interactive_test_utils_aura.h

@@ -16,6 +16,9 @@ namespace ui_test_utils {
 void HideNativeWindowAura(gfx::NativeWindow window);
 bool ShowAndFocusNativeWindowAura(gfx::NativeWindow window);
 
+// Wait until |window| gets focused.
+void WaitUntilWindowFocused(aura::Window* window);
+
 }  // namespace ui_test_utils
 
 #endif  // CHROME_TEST_BASE_INTERACTIVE_TEST_UTILS_AURA_H_

chrome/test/base/interactive_test_utils_views.cc

@@ -12,6 +12,45 @@
 #include "ui/base/ui_features.h"
 #include "ui/views/focus/focus_manager.h"
 
+#if defined(USE_AURA)
+#include "chrome/test/base/interactive_test_utils_aura.h"
+#endif
+
+namespace {
+
+class FocusWaiter : public views::FocusChangeListener {
+ public:
+  explicit FocusWaiter(views::View* view) : view_(view) {
+    view_->GetFocusManager()->AddFocusChangeListener(this);
+  }
+  ~FocusWaiter() override {
+    view_->GetFocusManager()->RemoveFocusChangeListener(this);
+  }
+
+  void WaitUntilFocused() {
+    if (view_->HasFocus())
+      return;
+    run_loop_.Run();
+  }
+
+ private:
+  // views::FocusChangeListener:
+  void OnWillChangeFocus(views::View* focused_before,
+                         views::View* focused_now) override {}
+  void OnDidChangeFocus(views::View* focused_before,
+                        views::View* focused_now) override {
+    if (focused_now == view_)
+      run_loop_.QuitWhenIdle();
+  }
+
+  views::View* view_;
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(FocusWaiter);
+};
+
+}  // namespace
+
 namespace ui_test_utils {
 
 bool IsViewFocused(const Browser* browser, ViewID vid) {
@@ -50,4 +89,18 @@ gfx::Point GetCenterInScreenCoordinates(const views::View* view) {
   return center;
 }
 
+void WaitUntilViewFocused(const Browser* browser, ViewID vid) {
+#if defined(USE_AURA)
+  BrowserWindow* browser_window = browser->window();
+  DCHECK(browser_window);
+  gfx::NativeWindow window = browser_window->GetNativeWindow();
+  DCHECK(window);
+  WaitUntilWindowFocused(window);
+#endif
+
+  FocusWaiter waiter(
+      BrowserView::GetBrowserViewForBrowser(browser)->GetViewByID(vid));
+  waiter.WaitUntilFocused();
+}
+
 }  // namespace ui_test_utils