Update Flaky ToolbarActionViewInteractiveUITests

chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc

Index: chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc
index fc9c6ac02fde22d0b8437e2cd0e291a13ef7db2e..cd4d855b9662c251461d0e164888b42aae3e7ccb 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_action_view_interactive_uitest.cc
@@ -39,20 +39,22 @@ AppMenuButton* GetAppButtonFromBrowser(Browser* browser) {
 
 // Tests clicking on an overflowed toolbar action. This is called when the app
 // menu is open, and handles actually clicking on the action.
-// |button| specifies the mouse button to click with.
-void TestOverflowedToolbarAction(Browser* browser,
-                                 ui_controls::MouseButton button) {
+// |button| specifies the mouse button to click with. Returning the targetted

[Devlin, 2016/08/25 16:42:12]

s/Returning/Returns

[Devlin, 2016/08/25 16:42:12]

typo: targetted

[jonross, 2016/08/25 18:29:27]

Done.

[jonross, 2016/08/25 18:29:27]

Done.

+// ToolbarActionView.
+ToolbarActionView* TestOverflowedToolbarAction(
+    Browser* browser,
+    ui_controls::MouseButton button) {
   // A bunch of plumbing to safely get at the overflowed toolbar action.
   AppMenuButton* app_menu_button = GetAppButtonFromBrowser(browser);
   EXPECT_TRUE(app_menu_button->IsMenuShowing());
   AppMenu* app_menu = app_menu_button->app_menu_for_testing();
-  ASSERT_TRUE(app_menu);
+  EXPECT_TRUE(app_menu);

[Devlin, 2016/08/25 16:42:12]

The reason these were asserts is because if they're false, we segfault.  I think
they should remain asserts. :)

[jonross, 2016/08/25 18:29:27]

I removed these as they interfere with return values.

I've restored them for the early segfaults, and switched to an optional out
param

   ExtensionToolbarMenuView* menu_view =
       app_menu->extension_toolbar_for_testing();
-  ASSERT_TRUE(menu_view);
+  EXPECT_TRUE(menu_view);
   BrowserActionsContainer* overflow_container =
       menu_view->container_for_testing();
-  ASSERT_TRUE(overflow_container);
+  EXPECT_TRUE(overflow_container);
   ToolbarActionView* action_view =
       overflow_container->GetToolbarActionViewAt(0);
   EXPECT_TRUE(action_view->visible());
@@ -63,14 +65,12 @@ void TestOverflowedToolbarAction(Browser* browser,
   ui_controls::SendMouseMove(action_view_loc.x(), action_view_loc.y());
   EXPECT_TRUE(ui_test_utils::SendMouseEventsSync(
       button, ui_controls::DOWN | ui_controls::UP));
+  return action_view;
 }
 
 // Tests the context menu of an overflowed action.
-void TestWhileContextMenuOpen(bool* did_test_while_menu_open,
-                              Browser* browser,
+void TestWhileContextMenuOpen(Browser* browser,
                               ToolbarActionView* context_menu_action) {
-  *did_test_while_menu_open = true;
-
   views::MenuItemView* menu_root = context_menu_action->menu_for_testing();
   ASSERT_TRUE(menu_root);
   ASSERT_TRUE(menu_root->GetSubmenu());
@@ -119,21 +119,11 @@ void TestWhileContextMenuOpen(bool* did_test_while_menu_open,
   // Click on the first menu item (which shares bounds, but overlaps, the second
   // row action).
   ui_controls::SendMouseMove(action_view_loc.x(), action_view_loc.y());
-  ui_controls::SendMouseEventsNotifyWhenDone(
-      ui_controls::LEFT, ui_controls::DOWN | ui_controls::UP, base::Closure());
-
+  EXPECT_TRUE(ui_test_utils::SendMouseEventsSync(
+      ui_controls::LEFT, ui_controls::DOWN | ui_controls::UP));
   // Test resumes in the main test body.
 }
 
-// Posts a task to test the context menu.
-void OnContextMenuWillShow(bool* did_test_while_menu_open,
-                           Browser* browser,
-                           ToolbarActionView* toolbar_action_view) {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::Bind(&TestWhileContextMenuOpen, did_test_while_menu_open,
-                            browser, toolbar_action_view));
-}
-
 }  // namespace
 
 class ToolbarActionViewInteractiveUITest : public ExtensionBrowserTest {
@@ -214,8 +204,17 @@ IN_PROC_BROWSER_TEST_F(ToolbarActionViewInteractiveUITest,
 // TODO(jonross): determine cause of new flake, and restore previous MAYBE
 // conditions. Temporarily disabling due to number of flakes (crbug.com/639010)
 // Tests the context menus of overflowed extension actions.
+
+#if defined(USE_OZONE)
+// ozone bringup - http://crbug.com/401304
+#define MAYBE_TestContextMenuOnOverflowedAction \
+  DISABLED_TestContextMenuOnOverflowedAction
+#else
+#define MAYBE_TestContextMenuOnOverflowedAction \
+  TestContextMenuOnOverflowedAction
+#endif
 IN_PROC_BROWSER_TEST_F(ToolbarActionViewInteractiveUITest,
-                       DISABLED_TestContextMenuOnOverflowedAction) {
+                       MAYBE_TestContextMenuOnOverflowedAction) {
   views::MenuController::TurnOffMenuSelectionHoldForTest();
 
   // Load an extension that has a home page (important for the context menu's
@@ -243,13 +242,6 @@ IN_PROC_BROWSER_TEST_F(ToolbarActionViewInteractiveUITest,
   // Reduce visible count to 0 so that all actions are overflowed.
   ToolbarActionsModel::Get(profile())->SetVisibleIconCount(0);
 
-  // Set a callback for the context menu showing.
-  bool did_test_while_menu_open = false;
-  base::Callback<void(ToolbarActionView*)> context_menu_callback(
-      base::Bind(&OnContextMenuWillShow, &did_test_while_menu_open, browser()));
-  ToolbarActionView::set_context_menu_callback_for_testing(
-      &context_menu_callback);
-
   AppMenuButton* app_menu_button = GetAppButtonFromBrowser(browser());
   // Click on the app button, and then right-click on the first toolbar action.
   gfx::Point app_button_loc =
@@ -257,18 +249,22 @@ IN_PROC_BROWSER_TEST_F(ToolbarActionViewInteractiveUITest,
   ui_controls::SendMouseMove(app_button_loc.x(), app_button_loc.y());
   EXPECT_TRUE(ui_test_utils::SendMouseEventsSync(
       ui_controls::LEFT, ui_controls::DOWN | ui_controls::UP));
-
   base::RunLoop().RunUntilIdle();
-  TestOverflowedToolbarAction(browser(), ui_controls::RIGHT);
+
+  // Right clicks on the action view, this should trigger the context menu.
+  ToolbarActionView* action_view =
+      TestOverflowedToolbarAction(browser(), ui_controls::RIGHT);
   base::RunLoop().RunUntilIdle();
 
-  // Test is continued first in TestOverflowedToolbarAction() to right click on
-  // the action, followed by OnContextMenuWillShow() and
-  // TestWhileContextMenuOpen().
+  // Ensure that the menu actually opened.
+  EXPECT_TRUE(action_view->IsMenuRunning());
 
-  // Make sure we did all the expected tests.
-  EXPECT_TRUE(did_test_while_menu_open);
+  // Triggers the action within the context menu. This should load the extension
+  // webpage, and close the menu.
+  TestWhileContextMenuOpen(browser(), action_view);
+  base::RunLoop().RunUntilIdle();
 
+  EXPECT_FALSE(action_view->IsMenuRunning());
   // We should have navigated to the extension's home page, which is google.com.
   EXPECT_EQ(
       GURL("https://www.google.com/"),
@@ -336,9 +332,8 @@ IN_PROC_BROWSER_TEST_F(ToolbarActionViewInteractiveUITest,
   EXPECT_EQ(nullptr, toolbar_actions_bar->popup_owner());
 }
 
-#if defined(USE_OZONE) || defined(OS_WIN)
+#if defined(USE_OZONE)
 // ozone bringup - http://crbug.com/401304
-// flaky on Windows - http://crbug.com/638692
 #define MAYBE_ActivateOverflowedToolbarActionWithKeyboard \
   DISABLED_ActivateOverflowedToolbarActionWithKeyboard
 #else
@@ -379,11 +374,9 @@ IN_PROC_BROWSER_TEST_F(ToolbarActionViewInteractiveUITest,
                             false);
   // The triggering of the action and subsequent widget destruction occurs on
   // the message loop. Wait for this all to complete.
-  base::RunLoop loop;
-  ui_controls::SendKeyPressNotifyWhenDone(native_window, ui::VKEY_RETURN, false,
-                                          false, false, false,
-                                          loop.QuitClosure());
-  loop.Run();
+  EXPECT_TRUE(ui_test_utils::SendKeyPressToWindowSync(
+      native_window, ui::VKEY_RETURN, false, false, false, false));
+  base::RunLoop().RunUntilIdle();
 
   // The menu should be closed.
   EXPECT_FALSE(app_menu_button->IsMenuShowing());