Deflake ui_controls::SendMouseMove on Windows.

On Windows, SendMouseMove must transform a point in screen coordinates
into the range (0..2^16-1) for consumption by SendInput. Windows then
converts this back to screen coordinates. This change rounds up rather
than down in the initial conversion so that Windows lands on the correct
point when it seemly truncates.

Previously, we'd see off-by-one errors from time to time. While this may
seem insignificant at first blush, it's problematic when SendMouseMove
is called to move the mouse to a point where it already resides. No
events are generated by SendInput in this case, so the hook proc will
never be called. This shouldn't be a problem since SendMouseMove detects
this case and short-circuits the request. With the ob1, however,
SendMouseMove thinks it needs to move the mouse by one point, yet the
bad math results in a call to SendInput to move the mouse to its current
position. This makes tests timeout and all of us sad.

BUG=390564,639940,710436,726659,728488,764415
R=mathp@chromium.org, sky@chromium.org

Change-Id: I1daebe8281db0a799ef7266c3c1765de65580f03
Reviewed-on: https://chromium-review.googlesource.com/686898Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506574}

chrome/browser/autofill/autofill_interactive_uitest.cc

@@ -552,8 +552,7 @@ class AutofillInteractiveTest : public InProcessBrowserTest {
 
 // Test that basic form fill is working.
 // Flakily times out on ChromeOS http://crbug.com/585885
-// Flakily fails on Windows http://crbug.com/639940
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_BasicFormFill DISABLED_BasicFormFill
 #else
 #define MAYBE_BasicFormFill BasicFormFill
@@ -633,8 +632,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_AutofillSelectViaTab) {
 }
 
 // crbug.com/516052
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_AutofillViaClick DISABLED_AutofillViaClick
 #else
 #define MAYBE_AutofillViaClick AutofillViaClick
@@ -665,14 +663,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_AutofillViaClick) {
 }
 
 // Makes sure that the first click does *not* activate the popup.
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_WIN)
-#define MAYBE_DontAutofillForFirstClick DISABLED_DontAutofillForFirstClick
-#else
-#define MAYBE_DontAutofillForFirstClick DontAutofillForFirstClick
-#endif
-IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
-                       MAYBE_DontAutofillForFirstClick) {
+IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, DontAutofillForFirstClick) {
   CreateTestProfile();
 
   // Load the test page.
@@ -694,14 +685,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 
 // Makes sure that clicking outside the focused field doesn't activate
 // the popup.
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_WIN)
-#define MAYBE_DontAutofillForOutsideClick DISABLED_DontAutofillForOutsideClick
-#else
-#define MAYBE_DontAutofillForOutsideClick DontAutofillForOutsideClick
-#endif
-IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
-                       MAYBE_DontAutofillForOutsideClick) {
+IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, DontAutofillForOutsideClick) {
   CreateTestProfile();
 
   // Load the test page.
@@ -726,8 +710,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 // Test that a field is still autofillable after the previously autofilled
 // value is deleted.
 // TODO(crbug.com/603488) Test is timing out flakily on CrOS.
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_OnDeleteValueAfterAutofill DISABLED_OnDeleteValueAfterAutofill
 #else
 #define MAYBE_OnDeleteValueAfterAutofill OnDeleteValueAfterAutofill
@@ -803,8 +786,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 
 // Test that a JavaScript oninput event is fired after auto-filling a form.
 // Flakily times out on ChromeOS http://crbug.com/585885
-// Flaky on Windows. http://crbug.com/726659
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_OnInputAfterAutofill DISABLED_OnInputAfterAutofill
 #else
 #define MAYBE_OnInputAfterAutofill OnInputAfterAutofill
@@ -883,8 +865,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_OnInputAfterAutofill) {
 
 // Test that a JavaScript onchange event is fired after auto-filling a form.
 // Flaky on CrOS.  http://crbug.com/578095
-// Flaky on Windows. http://crbug.com/726659
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_OnChangeAfterAutofill DISABLED_OnChangeAfterAutofill
 #else
 #define MAYBE_OnChangeAfterAutofill OnChangeAfterAutofill
@@ -962,8 +943,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_OnChangeAfterAutofill) {
 }
 
 // Flakily times out on ChromeOS http://crbug.com/585885
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_InputFiresBeforeChange DISABLED_InputFiresBeforeChange
 #else
 #define MAYBE_InputFiresBeforeChange InputFiresBeforeChange
@@ -1049,8 +1029,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_InputFiresBeforeChange) {
 
 // Test that we can autofill forms distinguished only by their |id| attribute.
 // Flaky on CrOS.  http://crbug.com/578095
-// Flaky on Windows. http://crbug.com/726659
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_AutofillFormsDistinguishedById \
   DISABLED_AutofillFormsDistinguishedById
 #else
@@ -1083,8 +1062,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 // (duplicated for "confirmation"); or variants that are hot-swapped via
 // JavaScript, with only one actually visible at any given time.
 // Flakily times out on ChromeOS http://crbug.com/585885
-// Flaky on Windows. http://crbug.com/726659
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_AutofillFormWithRepeatedField \
   DISABLED_AutofillFormWithRepeatedField
 #else
@@ -1136,8 +1114,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 }
 
 // TODO(crbug.com/603488) Test is timing out flakily on CrOS.
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_AutofillFormWithNonAutofillableField \
   DISABLED_AutofillFormWithNonAutofillableField
 #else
@@ -1189,8 +1166,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 }
 
 // Flakily fails on ChromeOS (crbug.com/646576).
-// Flaky on Windows (crbug.com/726659).
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_DynamicFormFill DISABLED_DynamicFormFill
 #else
 #define MAYBE_DynamicFormFill DynamicFormFill
@@ -1309,8 +1285,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_AutofillAfterReload) {
 // Test that filling a form sends all the expected events to the different
 // fields being filled.
 // Flakily fails on ChromeOS (crbug.com/646576).
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_AutofillEvents DISABLED_AutofillEvents
 #else
 #define MAYBE_AutofillEvents AutofillEvents
@@ -1506,13 +1481,12 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_AutofillAfterTranslate) {
 // The high level key presses execute the following: Select the first text
 // field, invoke the autofill popup list, select the first profile within the
 // list, and commit to the profile to populate the form.
-// Flakily times out on windows (https://crbug.com/390564), and on CrOS
-// (https://crbug.com/516052).
-#if defined(OS_WIN) || defined(OS_CHROMEOS)
+// Flakily times out on CrOS (https://crbug.com/516052).
+#if defined(OS_CHROMEOS)
 #define MAYBE_ComparePhoneNumbers DISABLED_ComparePhoneNumbers
 #else
 #define MAYBE_ComparePhoneNumbers ComparePhoneNumbers
-#endif  // defined(OS_WIN) || defined(OS_CHROMEOS)
+#endif  // defined(OS_CHROMEOS)
 IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_ComparePhoneNumbers) {
   AutofillProfile profile;
   profile.SetRawInfo(NAME_FIRST, ASCIIToUTF16("Bob"));
@@ -1550,8 +1524,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest, MAYBE_ComparePhoneNumbers) {
 // Test that Autofill does not fill in read-only fields.
 // Flaky on the official cros-trunk. crbug.com/516052
 // Also flaky on ChromiumOS generally. crbug.com/585885
-// Also flaky on Windows. crbug.com/726659
-#if defined(OFFICIAL_BUILD) || defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OFFICIAL_BUILD) || defined(OS_CHROMEOS)
 #define MAYBE_NoAutofillForReadOnlyFields DISABLED_NoAutofillForReadOnlyFields
 #else
 #define MAYBE_NoAutofillForReadOnlyFields NoAutofillForReadOnlyFields
@@ -1716,8 +1689,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveTest,
 // is interacting with the form.  This is a regression test for
 // http://crbug.com/160476
 // Flakily times out on ChromeOS http://crbug.com/585885
-// Flaky on Windows http://crbug.com/728488
-#if defined(OS_CHROMEOS) || defined(OS_WIN)
+#if defined(OS_CHROMEOS)
 #define MAYBE_DisableAutocompleteWhileFilling \
   DISABLED_DisableAutocompleteWhileFilling
 #else
@@ -1766,15 +1738,7 @@ class AutofillInteractiveIsolationTest : public AutofillInteractiveTest {
   }
 };
 
-#if defined(OS_WIN)
-// Flaky on Windows 7 in debug build. http://crbug.com/710436
-#define MAYBE_SimpleCrossSiteFill DISABLED_SimpleCrossSiteFill
-#else
-#define MAYBE_SimpleCrossSiteFill SimpleCrossSiteFill
-#endif
-IN_PROC_BROWSER_TEST_F(AutofillInteractiveIsolationTest,
-                       MAYBE_SimpleCrossSiteFill) {
-
+IN_PROC_BROWSER_TEST_F(AutofillInteractiveIsolationTest, SimpleCrossSiteFill) {
   CreateTestProfile();
 
   // Main frame is on a.com, iframe is on b.com.
@@ -1820,7 +1784,7 @@ IN_PROC_BROWSER_TEST_F(AutofillInteractiveIsolationTest,
 // form is inside an OOPIF.
 // Flaky on Windows http://crbug.com/728488
 #if defined(OS_WIN)
-#define MAYBE_CrossSitePaymentForms DISABLED_MAYBE_CrossSitePaymentForms
+#define MAYBE_CrossSitePaymentForms DISABLED_CrossSitePaymentForms
 #else
 #define MAYBE_CrossSitePaymentForms CrossSitePaymentForms
 #endif

chrome/browser/ui/send_mouse_move_uitest_win.cc

+// 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 <windows.h>
+
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "ui/base/test/ui_controls.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
+#include "ui/gfx/geometry/rect.h"
+
+class SendMouseMoveUITest : public InProcessBrowserTest {
+ protected:
+  SendMouseMoveUITest() = default;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SendMouseMoveUITest);
+};
+
+// This test positions the mouse at every point on the screen. It is not meant
+// to be run on the bots, as it takes too long. Run it manually as needed to
+// verify ui_controls::SendMouseMoveNotifyWhenDone with:
+//   interactive_ui_tests.exe --single_process --gtest_also_run_disabled_tests \
+//       --gtest_filter=SendMouseMoveUITest.DISABLED_Fullscreen
+IN_PROC_BROWSER_TEST_F(SendMouseMoveUITest, DISABLED_Fullscreen) {
+  // Make the browser fullscreen so that we can position the mouse anywhere on
+  // the display.
+  chrome::ToggleFullscreenMode(browser());
+
+  display::Screen* const screen = display::Screen::GetScreen();
+  const gfx::Rect screen_bounds = screen->GetPrimaryDisplay().bounds();
+  for (long scan_y = screen_bounds.y(),
+            bound_y = scan_y + screen_bounds.height();
+       scan_y < bound_y; ++scan_y) {
+    for (long scan_x = screen_bounds.x(),
+              bound_x = scan_x + screen_bounds.width();
+         scan_x < bound_x; ++scan_x) {
+      SCOPED_TRACE(testing::Message()
+                   << "(" << scan_x << ", " << scan_y << ")");
+      // Move the pointer.
+      base::RunLoop run_loop;
+      EXPECT_TRUE(ui_controls::SendMouseMoveNotifyWhenDone(
+          scan_x, scan_y, run_loop.QuitClosure()));
+      run_loop.Run();
+
+      // Check it two ways.
+      POINT cursor_pos = {};
+      ASSERT_NE(::GetCursorPos(&cursor_pos), FALSE);
+      EXPECT_EQ(gfx::Point(cursor_pos), gfx::Point(scan_x, scan_y));
+      EXPECT_EQ(screen->GetCursorScreenPoint(), gfx::Point(scan_x, scan_y));
+    }
+  }
+}

chrome/test/BUILD.gn

@@ -483,6 +483,7 @@ if (!is_android) {
       "../browser/ui/search/instant_uitest_base.cc",
       "../browser/ui/search/instant_uitest_base.h",
       "../browser/ui/search/local_ntp_uitest.cc",
+      "../browser/ui/send_mouse_move_uitest_win.cc",
       "../browser/ui/startup/startup_browser_creator_interactive_uitest.cc",
       "../browser/ui/translate/translate_bubble_test_utils.h",
       "../browser/ui/views/accessibility/navigation_accessibility_uitest_win.cc",

ui/base/test/ui_controls_internal_win.cc

@@ -4,6 +4,8 @@
 
 #include "ui/base/test/ui_controls_internal_win.h"
 
+#include <cmath>
+
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
@@ -300,30 +302,40 @@ bool SendMouseMoveImpl(long screen_x,
   POINT current_pos;
   ::GetCursorPos(&current_pos);
   if (screen_x == current_pos.x && screen_y == current_pos.y) {
-    if (!task.is_null())
+    if (task)
       base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, task);
     return true;
   }
 
-  INPUT input = { 0 };
-
-  int screen_width = ::GetSystemMetrics(SM_CXSCREEN) - 1;
-  int screen_height  = ::GetSystemMetrics(SM_CYSCREEN) - 1;
-  LONG pixel_x  = static_cast<LONG>(screen_x * (65535.0f / screen_width));
-  LONG pixel_y = static_cast<LONG>(screen_y * (65535.0f / screen_height));
-
-  input.type = INPUT_MOUSE;
+  // Get the max screen coordinate for use in computing the normalized absolute
+  // coordinates required by SendInput.
+  int max_x = ::GetSystemMetrics(SM_CXSCREEN) - 1;
+  int max_y = ::GetSystemMetrics(SM_CYSCREEN) - 1;
+
+  // Clamp the inputs.
+  if (screen_x < 0)
+    screen_x = 0;
+  else if (screen_x > max_x)
+    screen_x = max_x;
+  if (screen_y < 0)
+    screen_y = 0;
+  else if (screen_y > max_y)
+    screen_y = max_y;
+
+  // Form the input data containing the normalized absolute coordinates.
+  INPUT input = {INPUT_MOUSE};
+  input.mi.dx = static_cast<LONG>(std::ceil(screen_x * (65535.0 / max_x)));
+  input.mi.dy = static_cast<LONG>(std::ceil(screen_y * (65535.0 / max_y)));
   input.mi.dwFlags = MOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_MOVE;
-  input.mi.dx = pixel_x;
-  input.mi.dy = pixel_y;
 
-  scoped_refptr<InputDispatcher> dispatcher(
-      !task.is_null() ? new InputDispatcher(task, WM_MOUSEMOVE) : NULL);
+  scoped_refptr<InputDispatcher> dispatcher;
+  if (task)
+    dispatcher = base::MakeRefCounted<InputDispatcher>(task, WM_MOUSEMOVE);
 
-  if (!::SendInput(1, &input, sizeof(INPUT)))
+  if (!::SendInput(1, &input, sizeof(input)))
     return false;
 
-  if (dispatcher.get())
+  if (dispatcher)
     dispatcher->AddRef();
 
   return true;