Regression: Search+Key pops up app launcher

chrome/browser/chromeos/accessibility/sticky_keys_browsertest.cc

 // Copyright 2013 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 "ash/shell.h"
 #include "ash/sticky_keys/sticky_keys_controller.h"
 #include "ash/sticky_keys/sticky_keys_overlay.h"
 #include "ash/system/tray/system_tray.h"
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/events/test/event_generator.h"
 #include "ui/gfx/native_widget_types.h"
 
 namespace chromeos {
 
 class StickyKeysBrowserTest : public InProcessBrowserTest {
  protected:
   StickyKeysBrowserTest() {}
   virtual ~StickyKeysBrowserTest() {}
 
   void EnableStickyKeys() {
     AccessibilityManager::Get()->EnableStickyKeys(true);
   }
 
   void DisableStickyKeys() {
     AccessibilityManager::Get()->EnableStickyKeys(false);
   }
 
   ash::SystemTray* GetSystemTray() {
     return ash::Shell::GetInstance()->GetPrimarySystemTray();
   }
 
   void SendKeyPress(ui::KeyboardCode key) {

[dmazzoni, 2014/11/20 21:32:38]

Your changes are fine, but I don't like that some tests use
ui_test_utils::SendKeyPress... and others use EventGenerator. If EventGenerator
is better, can we just always use that?

Can you just delete all of the SendKeyPress functions and can we call
EventGenerator functions instead, everywhere here?

Alternatively, could you leave the tests alone, and instead rewrite the
SendKeyPress helper functions using EventGenerator?

[afakhry, 2014/11/21 02:15:03]

Done.

     gfx::NativeWindow root_window =
         ash::Shell::GetInstance()->GetPrimaryRootWindow();
     ASSERT_TRUE(
         ui_test_utils::SendKeyPressToWindowSync(root_window,
                                                 key,
                                                 false, // control
                                                 false, // shift
                                                 false, // alt
                                                 false)); // command
   }
 
+  ui::test::EventGenerator& GetEventGenerator() {

[dmazzoni, 2014/11/20 21:32:38]

Please just move this to SetUp

[afakhry, 2014/11/21 02:15:03]

Moved to SetUpOnMainThread() since SetUp() caused it to crash by not having
event_generator_ initialized in time.

[afakhry, 2014/11/21 02:15:03]

Done.

+    if (!event_generator_) {
+      event_generator_.reset(
+          new ui::test::EventGenerator(browser()->window()->GetNativeWindow()));
+    }
+    return *event_generator_.get();
+  }
+
   content::NotificationRegistrar registrar_;
+  scoped_ptr<ui::test::EventGenerator> event_generator_;
 
   DISALLOW_COPY_AND_ASSIGN(StickyKeysBrowserTest);
 };
 
 IN_PROC_BROWSER_TEST_F(StickyKeysBrowserTest, OpenTrayMenu) {
   EnableStickyKeys();
 
   // Open system tray bubble with shortcut.
   SendKeyPress(ui::VKEY_MENU); // alt key.
   SendKeyPress(ui::VKEY_SHIFT);
@@ skipping 78 matching lines @@
 
 IN_PROC_BROWSER_TEST_F(StickyKeysBrowserTest, SearchLeftOmnibox) {
   EnableStickyKeys();
 
   OmniboxView* omnibox =
       browser()->window()->GetLocationBar()->GetOmniboxView();
 
   // Give the omnibox focus.
   omnibox->ShowURL();
 
+  ui::test::EventGenerator& generator = GetEventGenerator();
+
+  // Make sure that the AppList is not erronously displayed and the omnibox
+  // doesn't lost focus
+  EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility());
+  EXPECT_TRUE(omnibox->GetNativeView()->HasFocus());
+
   // Type 'foo'.
-  SendKeyPress(ui::VKEY_F);
-  SendKeyPress(ui::VKEY_O);
-  SendKeyPress(ui::VKEY_O);
+  generator.PressKey(ui::VKEY_F, ui::EF_NONE);

[dmazzoni, 2014/11/20 21:32:38]

Just call generator_.PressKey, then there's no need to call GetEventGenerator
from each test.

+  generator.ReleaseKey(ui::VKEY_F, ui::EF_NONE);
+  generator.PressKey(ui::VKEY_O, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_O, ui::EF_NONE);
+  generator.PressKey(ui::VKEY_O, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_O, ui::EF_NONE);
 
   // Verify the location of the caret.
   size_t start, end;
   omnibox->GetSelectionBounds(&start, &end);
   ASSERT_EQ(3U, start);
   ASSERT_EQ(3U, end);
 
+  EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility());
+  EXPECT_TRUE(omnibox->GetNativeView()->HasFocus());
+
   // Hit Home by sequencing Search (left Windows) and Left (arrow).
-  SendKeyPress(ui::VKEY_LWIN);
-  SendKeyPress(ui::VKEY_LEFT);
+  generator.PressKey(ui::VKEY_LWIN, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_LWIN, ui::EF_NONE);
+  generator.PressKey(ui::VKEY_LEFT, ui::EF_NONE);
+  generator.ReleaseKey(ui::VKEY_LEFT, ui::EF_NONE);
+
+  EXPECT_FALSE(ash::Shell::GetInstance()->GetAppListTargetVisibility());
+  EXPECT_TRUE(omnibox->GetNativeView()->HasFocus());
 
   // Verify caret moved to the beginning.
   omnibox->GetSelectionBounds(&start, &end);
   ASSERT_EQ(0U, start);
   ASSERT_EQ(0U, end);
 }
 
 IN_PROC_BROWSER_TEST_F(StickyKeysBrowserTest, OverlayShown) {
   const ui::KeyboardCode modifier_keys[] = { ui::VKEY_CONTROL,
                                              ui::VKEY_SHIFT,
@@ skipping 26 matching lines @@
   EXPECT_TRUE(sticky_keys_overlay->is_visible());
   DisableStickyKeys();
   EXPECT_FALSE(controller->GetOverlayForTest());
   for (auto key_code : modifier_keys) {
     SendKeyPress(key_code);
     EXPECT_FALSE(controller->GetOverlayForTest());
   }
 }
 
 }  // namespace chromeos