Refactor out foreground_help to fix BringBrowserWindowToFront on Windows

Refactor out foreground_helper so that regular Chromium tests can use it. Use
it in ShowAndFocusNativeWindow to make sure the browser window is in front,
both when tests ask and when trying to send keyboard events. This should reduce
some flakiness of related tests, and certainly makes it easier to debug those
tests.

The API for SendKeyPress events looks like it changed long ago to add a target
window, only the Windows code simply dropped that on the floor. By returning
false when the event wouldn't go to the right place, it allows for
ui_test_utils::SendKeyPressSync to bring the browser window to the front and
try again.

BUG=106489
TEST=Run browser_tests or interactive_ui_tests and bring another window to the
front. For the tests in question, the browser should now pop in front when it
needs to for the test to complete properly.


Review URL: http://codereview.chromium.org/9431050

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@123771 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 177 insertions, 110 deletions

diff --git a/chrome/browser/automation/ui_controls_internal.h b/chrome/browser/automation/ui_controls_internal.h
index e0f5a78..f16de36 100644
--- a/chrome/browser/automation/ui_controls_internal.h
+++ b/chrome/browser/automation/ui_controls_internal.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -18,7 +18,8 @@ void ClickTask(MouseButton button, int state, const base::Closure& followup);
 #if defined(OS_WIN)
 // A utility functions for windows to send key or mouse events and
 // run the task.
-bool SendKeyPressImpl(ui::KeyboardCode key,
+bool SendKeyPressImpl(gfx::NativeWindow window,
+                      ui::KeyboardCode key,
                       bool control,
                       bool shift,
                       bool alt,
diff --git a/chrome/browser/automation/ui_controls_internal_win.cc b/chrome/browser/automation/ui_controls_internal_win.cc
index 39925cf..45ca31a 100644
--- a/chrome/browser/automation/ui_controls_internal_win.cc
+++ b/chrome/browser/automation/ui_controls_internal_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -9,8 +9,8 @@
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
-#include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/keycodes/keyboard_code_conversion_win.h"
+#include "ui/base/keycodes/keyboard_codes.h"
 
 namespace {
 
@@ -166,9 +166,21 @@ bool SendKeyEvent(ui::KeyboardCode key, bool up) {
 namespace ui_controls {
 namespace internal {
 
-bool SendKeyPressImpl(ui::KeyboardCode key,
-                      bool control, bool shift, bool alt,
+bool SendKeyPressImpl(gfx::NativeWindow window,
+                      ui::KeyboardCode key,
+                      bool control,
+                      bool shift,
+                      bool alt,
                       const base::Closure& task) {
+  // SendInput only works as we expect it if one of our windows is the
+  // foreground window already.
+  HWND target_window = (::GetActiveWindow() &&
+                        ::GetWindow(::GetActiveWindow(), GW_OWNER) == window) ?
+                       ::GetActiveWindow() :
+                       window;
+  if (window && ::GetForegroundWindow() != target_window)
+    return false;
+
   scoped_refptr<InputDispatcher> dispatcher(
       !task.is_null() ? new InputDispatcher(task, WM_KEYUP) : NULL);
 
diff --git a/chrome/browser/automation/ui_controls_win.cc b/chrome/browser/automation/ui_controls_win.cc
index 744928f..57ce337 100644
--- a/chrome/browser/automation/ui_controls_win.cc
+++ b/chrome/browser/automation/ui_controls_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -19,7 +19,8 @@ bool SendKeyPress(gfx::NativeWindow window,
                   bool alt,
                   bool command) {
   DCHECK(!command);  // No command key on Windows
-  return internal::SendKeyPressImpl(key, control, shift, alt, base::Closure());
+  return internal::SendKeyPressImpl(window, key, control, shift, alt,
+                                    base::Closure());
 }
 
 bool SendKeyPressNotifyWhenDone(gfx::NativeWindow window,
@@ -30,7 +31,7 @@ bool SendKeyPressNotifyWhenDone(gfx::NativeWindow window,
                                 bool command,
                                 const base::Closure& task) {
   DCHECK(!command);  // No command key on Windows
-  return internal::SendKeyPressImpl(key, control, shift, alt, task);
+  return internal::SendKeyPressImpl(window, key, control, shift, alt, task);
 }
 
 bool SendMouseMove(long x, long y) {
diff --git a/chrome/test/base/ui_test_utils.cc b/chrome/test/base/ui_test_utils.cc
index adc81b0..8f6896e 100644
--- a/chrome/test/base/ui_test_utils.cc
+++ b/chrome/test/base/ui_test_utils.cc
@@ -671,12 +671,20 @@ bool SendKeyPressSync(const Browser* browser,
   if (!GetNativeWindow(browser, &window))
     return false;
 
-  if (!ui_controls::SendKeyPressNotifyWhenDone(
-          window, key, control, shift, alt, command,
-          MessageLoop::QuitClosure())) {
+  bool result;
+  result = ui_controls::SendKeyPressNotifyWhenDone(
+      window, key, control, shift, alt, command, MessageLoop::QuitClosure());
+#if defined(OS_WIN)
+  if (!result && BringBrowserWindowToFront(browser)) {
+    result = ui_controls::SendKeyPressNotifyWhenDone(
+        window, key, control, shift, alt, command, MessageLoop::QuitClosure());
+  }
+#endif
+  if (!result) {
     LOG(ERROR) << "ui_controls::SendKeyPressNotifyWhenDone failed";
     return false;
   }
+
   // Run the message loop. It'll stop running when either the key was received
   // or the test timed out (in which case testing::Test::HasFatalFailure should
   // be set).
diff --git a/chrome/test/base/ui_test_utils_win.cc b/chrome/test/base/ui_test_utils_win.cc
index 68e850c..483ebcc 100644
--- a/chrome/test/base/ui_test_utils_win.cc
+++ b/chrome/test/base/ui_test_utils_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "ui/base/win/foreground_helper.h"
 #include "ui/views/focus/focus_manager.h"
 
 namespace ui_test_utils {
@@ -52,6 +53,11 @@ bool ShowAndFocusNativeWindow(gfx::NativeWindow window) {
   // using Windows API.
   ::ShowWindow(window, SW_SHOW);
 
+  if (GetForegroundWindow() != window) {
+    VLOG(1) << "Forcefully refocusing front window";
+    ui::ForegroundHelper::SetForeground(window);
+  }
+
   // ShowWindow does not necessarily activate the window. In particular if a
   // window from another app is the foreground window then the request to
   // activate the window fails. See SetForegroundWindow for details.
diff --git a/ui/base/win/foreground_helper.cc b/ui/base/win/foreground_helper.cc
new file mode 100644
index 0000000..0b03582
--- /dev/null
+++ b/ui/base/win/foreground_helper.cc
@@ -0,0 +1,84 @@
+// Copyright (c) 2012 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 "ui/base/win/foreground_helper.h"
+
+#include "base/logging.h"
+#include "ui/base/win/window_impl.h"
+
+namespace ui {
+
+// static
+HRESULT ForegroundHelper::SetForeground(HWND window) {
+  DCHECK(::IsWindow(window));
+  ForegroundHelper foreground_helper;
+  return foreground_helper.ForegroundHotKey(window);
+}
+
+HRESULT ForegroundHelper::ForegroundHotKey(HWND window) {
+  // This implementation registers a hot key (F22) and then
+  // triggers the hot key.  When receiving the hot key, we'll
+  // be in the foreground and allowed to move the target window
+  // into the foreground too.
+
+  set_window_style(WS_POPUP);
+  Init(NULL, gfx::Rect());
+
+  static const int kHotKeyId = 0x0000baba;
+  static const int kHotKeyWaitTimeout = 2000;
+
+  // Store the target window into our USERDATA for use in our
+  // HotKey handler.
+  window_ = window;
+  RegisterHotKey(hwnd(), kHotKeyId, 0, VK_F22);
+
+  // If the calling thread is not yet a UI thread, call PeekMessage
+  // to ensure creation of its message queue.
+  MSG msg = {0};
+  PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE);
+
+  // Send the Hotkey.
+  INPUT hotkey = {0};
+  hotkey.type = INPUT_KEYBOARD;
+  hotkey.ki.wVk =  VK_F22;
+  if (1 != SendInput(1, &hotkey, sizeof(hotkey))) {
+    LOG(WARNING) << "Failed to send input";
+    return E_FAIL;
+  }
+
+  // There are scenarios where the WM_HOTKEY is not dispatched by the
+  // the corresponding foreground thread. To prevent us from indefinitely
+  // waiting for the hotkey, we set a timer and exit the loop.
+  SetTimer(hwnd(), kHotKeyId, kHotKeyWaitTimeout, NULL);
+
+  // Loop until we get the key or the timer fires.
+  while (GetMessage(&msg, NULL, 0, 0)) {
+    TranslateMessage(&msg);
+    DispatchMessage(&msg);
+
+    if (WM_HOTKEY == msg.message)
+      break;
+    if (WM_TIMER == msg.message) {
+      SetForegroundWindow(window);
+      break;
+    }
+  }
+
+  UnregisterHotKey(hwnd(), kHotKeyId);
+  KillTimer(hwnd(), kHotKeyId);
+  DestroyWindow(hwnd());
+
+  return S_OK;
+}
+
+  // Handle the registered Hotkey being pressed.
+LRESULT ForegroundHelper::OnHotKey(UINT message,
+                                   WPARAM wparam,
+                                   LPARAM lparam,
+                                   BOOL& handled) {
+  SetForegroundWindow(window_);
+  return 1;
+}
+
+}  // namespace ui
diff --git a/ui/base/win/foreground_helper.h b/ui/base/win/foreground_helper.h
new file mode 100644
index 0000000..a630828
--- /dev/null
+++ b/ui/base/win/foreground_helper.h
@@ -0,0 +1,47 @@
+// Copyright (c) 2012 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.
+
+#ifndef UI_BASE_WIN_FOREGROUND_HELPER_H_
+#define UI_BASE_WIN_FOREGROUND_HELPER_H_
+#pragma once
+
+#include "base/logging.h"
+#include "ui/base/win/window_impl.h"
+
+namespace ui {
+
+// Helper class for moving a window to the foreground.
+// Windows XP and later will not allow a window which is in the background to
+// move to the foreground, unless requested by the current window in the
+// foreground.  For automated testing, we really want some of our windows
+// to be capable of moving to the foreground.
+//
+// This is probably leveraging a windows bug.
+class UI_EXPORT ForegroundHelper : public WindowImpl {
+ public:
+  ForegroundHelper() : window_(NULL) { }
+
+  BEGIN_MSG_MAP_EX(ForegroundHelper)
+    MESSAGE_HANDLER(WM_HOTKEY, OnHotKey)
+  END_MSG_MAP()
+
+  // Brings a window into the foreground.
+  // Can be called from any window, even if the caller is not the
+  // foreground window.
+  static HRESULT SetForeground(HWND window);
+
+ private:
+  HRESULT ForegroundHotKey(HWND window);
+
+  // Handle the registered Hotkey being pressed.
+  LRESULT OnHotKey(UINT message, WPARAM wparam, LPARAM lparam, BOOL& handled);
+
+  HWND window_;
+
+  DISALLOW_COPY_AND_ASSIGN(ForegroundHelper);
+};
+
+}  // namespace ui
+
+#endif  // UI_BASE_WIN_FOREGROUND_HELPER_H_
diff --git a/ui/ui.gyp b/ui/ui.gyp
index 75478f0..00db144 100644
--- a/ui/ui.gyp
+++ b/ui/ui.gyp
@@ -213,6 +213,8 @@
         'base/wayland/events_wayland.cc',
         'base/win/atl_module.h',
         'base/win/events_win.cc',
+        'base/win/foreground_helper.cc',
+        'base/win/foreground_helper.h',
         'base/win/hwnd_util.cc',
         'base/win/hwnd_util.h',
         'base/win/ime_input.cc',
diff --git a/webkit/tools/test_shell/foreground_helper.h b/webkit/tools/test_shell/foreground_helper.h
deleted file mode 100644
index 9466183..0000000
--- a/webkit/tools/test_shell/foreground_helper.h
+++ /dev/null
@@ -1,93 +0,0 @@
-// Copyright (c) 2009 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.
-
-#ifndef WEBKIT_TOOLS_TEST_SHELL_FOREGROUND_HELPER_H_
-#define WEBKIT_TOOLS_TEST_SHELL_FOREGROUND_HELPER_H_
-
-#include "base/logging.h"
-#include "ui/base/win/window_impl.h"
-
-// Helper class for moving a window to the foreground.
-// Windows XP and later will not allow a window which is in the background to
-// move to the foreground, unless requested by the current window in the
-// foreground.  For automated testing, we really want some of our windows
-// to be capable of moving to the foreground.
-//
-// This is probably leveraging a windows bug.
-class ForegroundHelper : public ui::WindowImpl {
- public:
-  BEGIN_MSG_MAP_EX(ForegroundHelper)
-    MESSAGE_HANDLER(WM_HOTKEY, OnHotKey)
-  END_MSG_MAP()
-
-  // Brings a window into the foreground.
-  // Can be called from any window, even if the caller is not the
-  // foreground window.
-  static HRESULT SetForeground(HWND window) {
-    DCHECK(::IsWindow(window));
-    ForegroundHelper foreground_helper;
-    CHECK(foreground_helper.ForegroundHotKey(window) == S_OK);
-    return S_OK;
-  }
-
- private:
-  HRESULT ForegroundHotKey(HWND window) {
-    // This implementation registers a hot key (F22) and then
-    // triggers the hot key.  When receiving the hot key, we'll
-    // be in the foreground and allowed to move the target window
-    // into the foreground too.
-
-    set_window_style(WS_POPUP);
-    Init(NULL, gfx::Rect());
-
-    static const int hotkey_id = 0x0000baba;
-
-    // Store the target window into our USERDATA for use in our
-    // HotKey handler.
-    SetWindowLongPtr(hwnd(), GWLP_USERDATA,
-                     reinterpret_cast<ULONG_PTR>(window));
-    RegisterHotKey(hwnd(), hotkey_id, 0, VK_F22);
-
-    // If the calling thread is not yet a UI thread, call PeekMessage
-    // to ensure creation of its message queue.
-    MSG msg = {0};
-    PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE);
-
-    // Send the Hotkey.
-    INPUT hotkey = {0};
-    hotkey.type = INPUT_KEYBOARD;
-    hotkey.ki.wVk =  VK_F22;
-    if (1 != SendInput(1, &hotkey, sizeof(hotkey)))
-      return E_FAIL;
-
-    // Loop until we get the key.
-    // TODO: It may be possible to get stuck here if the
-    // message gets lost?
-    while (GetMessage(&msg, NULL, 0, 0)) {
-      TranslateMessage(&msg);
-      DispatchMessage(&msg);
-
-      if (WM_HOTKEY == msg.message)
-        break;
-    }
-
-    UnregisterHotKey(hwnd(), hotkey_id);
-    DestroyWindow(hwnd());
-
-    return S_OK;
-  }
-
-  // Handle the registered Hotkey being pressed.
-  LRESULT OnHotKey(UINT message,
-                   WPARAM wparam,
-                   LPARAM lparam,
-                   BOOL& handled) {
-    HWND window = reinterpret_cast<HWND>(GetWindowLongPtr(hwnd(),
-                                                          GWLP_USERDATA));
-    SetForegroundWindow(window);
-    return 1;
-  }
-};
-
-#endif  // WEBKIT_TOOLS_TEST_SHELL_FOREGROUND_HELPER_H_
diff --git a/webkit/tools/test_shell/test_shell.gypi b/webkit/tools/test_shell/test_shell.gypi
index 6f49b9b..74c2e76 100644
--- a/webkit/tools/test_shell/test_shell.gypi
+++ b/webkit/tools/test_shell/test_shell.gypi
@@ -70,7 +70,6 @@
         'accessibility_ui_element.h',
         'drop_delegate.cc',
         'drop_delegate.h',
-        'foreground_helper.h',
         'layout_test_controller.cc',
         'layout_test_controller.h',
         'mock_spellcheck.cc',
diff --git a/webkit/tools/test_shell/test_shell_platform_delegate_win.cc b/webkit/tools/test_shell/test_shell_platform_delegate_win.cc
index cb9c3da..ad1c1e3 100644
--- a/webkit/tools/test_shell/test_shell_platform_delegate_win.cc
+++ b/webkit/tools/test_shell/test_shell_platform_delegate_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -10,8 +10,8 @@
 #include "base/command_line.h"
 #include "base/event_recorder.h"
 #include "base/win/win_util.h"
+#include "ui/base/win/foreground_helper.h"
 #include "ui/gfx/native_theme_win.h"
-#include "webkit/tools/test_shell/foreground_helper.h"
 #include "webkit/tools/test_shell/test_shell.h"
 #include "webkit/tools/test_shell/test_shell_platform_delegate.h"
 
@@ -150,6 +150,6 @@ void TestShellPlatformDelegate::SetWindowPositionForRecording(
   // on build systems where the script invoking us is a background
   // process.  So for this case, make our window the topmost window
   // as well.
-  ForegroundHelper::SetForeground(shell->mainWnd());
+  CHECK(ui::ForegroundHelper::SetForeground(shell->mainWnd()) == S_OK);
   ::SetWindowPos(shell->mainWnd(), HWND_TOP, 0, 0, 600, 800, 0);
 }