Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(12)

Issue 821883002: Mac: More robust "window under location" for interactive_ui_tests (Closed)

Created:
6 years ago by tapted
Modified:
5 years, 12 months ago
Reviewers:
Nico
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, mac-views-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Mac: More robust "window under location" for interactive_ui_tests The interactive_ui_tests for toolkit-views' custom menus (e.g. for combo boxes) would work fine when running tests locally, but refuse pass when run on a trybot. Tracing narrowed the discrepancy down to the "WindowAtCurrentMouseLocation()" function in ui_controls_mac.mm. This was working for windows at some positions on the screen, but not others, and attempting to add a [NSApp activateIgnoringOtherApps:YES] call at the appropriate place (e.g. ShowAndFocusNativeWindow() in interactive_test_utils_mac.mm) had no effect. This CL makes WindowAtCurrentMouseLocation more robust by effectively ignoring the influence of windows in other applications that may be on top of windows created in the test. Instead, iterate through [NSApp orderedWindows] and use NSPointInRect. This allows all the toolkit-views menu interactive_ui_tests to pass (except drag & drop - still working on that). Note that, historically, Mac's SendMouseMoveNotifyWhenDone would just use the keyWindow, but that doesn't work for menus (it's also "wrong" for mouse events). I suspect this will also allow me to de-flake a bunch of disabled, Cocoa interactive_ui_tests that have been accumulating. BUG=403679 Committed: https://crrev.com/1285ca09eaaea056fcbedf0ec621084766449497 Cr-Commit-Position: refs/heads/master@{#309620}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -1 line) Patch
M ui/base/test/ui_controls_mac.mm View 1 chunk +21 lines, -1 line 0 comments Download

Messages

Total messages: 11 (4 generated)
tapted
Hi Nico, could you please take a look?
6 years ago (2014-12-23 05:06:41 UTC) #2
Nico
lgtm, worth a try. Good comments / cl description.
6 years ago (2014-12-23 18:20:12 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/821883002/1
5 years, 12 months ago (2014-12-23 21:45:08 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_gn_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_gn_dbg/builds/28850)
5 years, 12 months ago (2014-12-23 21:54:22 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/821883002/1
5 years, 12 months ago (2014-12-24 06:12:47 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 12 months ago (2014-12-24 06:32:27 UTC) #10
commit-bot: I haz the power
5 years, 12 months ago (2014-12-24 06:33:17 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/1285ca09eaaea056fcbedf0ec621084766449497
Cr-Commit-Position: refs/heads/master@{#309620}

Powered by Google App Engine
This is Rietveld 408576698