Content Shell allows native events to mess with layout tests on Mac |
|||
Issue descriptionSee https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11?numbuilds=100 Sample failures: https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_11/23064/layout-test-results/results.html Flakiness started in build 23036 (https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/23036) --- hbos@ suspected this was caused by https://chromium-review.googlesource.com/642368, so he reverted it in https://chromium-review.googlesource.com/c/chromium/src/+/646086. But 1) the failure mode is completely unrelated to the Skia change (which should only affect gradients, subtly) and 2) there have been two more failure instances so far, *after the revert*: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/23064 https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/23067 So something else is going on. --- Staring closely at the pixel differences, it looks like the bad version is rendered with some unwanted mouse focus on random input elements. E.g.: https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_11/23067/layout-test-results/fast/forms/calendar-picker/calendar-picker-appearance-zoom125-diffs.html https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_11/23067/layout-test-results/fast/forms/color/color-suggestion-picker-appearance-zoom200-diffs.html https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Mac10_11/23067/layout-test-results/fast/forms/suggestion-picker/week-suggestion-picker-appearance-diffs.html Almost like there's some spurious mouse move event messing with the controls - which made me suspect https://chromium-review.googlesource.com/643626, also present in the initial flake blame. Eventually I was able to trigger the flakes on an MBP. Here's the crazy repro: * start running the forms layout tests: third_party/WebKit/Tools/Scripts/run-webkit-tests --num-retries=0 fast/forms/ * start circling the mouse cursor in the upper-left quadrant of the screen * observe tests failing randomly (I'm getting 2-7 failures per run) This repros on an MBP (10.12), and the failures go away when a) I leave the mouse alone, somewhere to the right or b) I revert https://chromium-review.googlesource.com/643626 :)
,
Aug 31 2017
I can repro consistently on my MBP, as long as I keep moving the mouse in the upper-left area for the duration of the test run.
,
Aug 31 2017
Well injecting any input into a layout test will always make them fail. That is why on linux you run them on a hidden xdisplay.
,
Aug 31 2017
Re. c#3, I'm not sure how we run layout tests on Mac to comment in general - but the behavior seems to have changed with your patch. I also don't think the movement is strictly required, it just makes the repro easier. I've seen it happen even when I leave the mouse static in that area (which I'm guessing is what's going on with the build slave).
,
Sep 1 2017
I hacked together a quick prototype of discarding native input inside render_widget_host_view_mac.mm and it seemed to work correctly for these tests. Anyone have any thoughts how I should implement this? I was wondering if I should implement a RenderWidgetHostViewMacDelegate when in shell mode and layout tests are enabled and toss out all native input. The RenderWidgetHostViewMacDelegate class has a handleEvent interface that I can provide an implementation of easily. Anyone know how this doesn't happen on Windows?
,
Sep 1 2017
Change uploaded here: https://chromium-review.googlesource.com/c/chromium/src/+/647341 Sending it to the trybots hopefully all is well with the change.
,
Sep 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bff20002785d6966a69d0ccf62f33229b042d403 commit bff20002785d6966a69d0ccf62f33229b042d403 Author: Dave Tapuska <dtapuska@chromium.org> Date: Fri Sep 01 20:45:17 2017 Support discarding native events when running layout test. Native events sent from the OS can cause layout tests to fail. We started correctly delivering them (in change https://chromium-review.googlesource.com/643626) with correct values and flakiness was discovered. BUG= 761070 Change-Id: Ibdc7b288c4890d44a02996cefebac2a7f687a038 Reviewed-on: https://chromium-review.googlesource.com/647341 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Dave Tapuska <dtapuska@chromium.org> Cr-Commit-Position: refs/heads/master@{#499285} [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/chrome/browser/ui/cocoa/tab_contents/chrome_web_contents_view_delegate_mac.h [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/chrome/browser/ui/cocoa/tab_contents/chrome_web_contents_view_delegate_mac.mm [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/browser/web_contents/web_contents_view_mac.mm [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/public/browser/web_contents_view_delegate.cc [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/public/browser/web_contents_view_delegate.h [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/shell/BUILD.gn [add] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/shell/browser/renderer_host/shell_render_widget_host_view_mac_delegate.h [add] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/shell/browser/renderer_host/shell_render_widget_host_view_mac_delegate.mm [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/shell/browser/shell_web_contents_view_delegate.h [modify] https://crrev.com/bff20002785d6966a69d0ccf62f33229b042d403/content/shell/browser/shell_web_contents_view_delegate_mac.mm
,
Sep 2 2017
Awesome, the flakes are all gone, thanks! |
|||
►
Sign in to add a comment |
|||
Comment 1 by dtapu...@chromium.org
, Aug 31 2017