The named CL r562137 has rebaselines for 10.10 so I suspect something went awry

Related stuff:

Issue 831720: tap-to-hide-controls.html is already flaky on Linux/Win
Issue 783154: relates to "double-tap" media controls on Mac
Issue 814964: a _similarly named_ test color-profile-border-radius.html is already marked flaky

disable CL -> https://chromium-review.googlesource.com/#/c/chromium/src/+/1074771

more failures on another bot (GPU): https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.10/47853


Unexpected Failures:
* media/controls/modern/tap-to-hide-controls.html
* virtual/gpu-rasterization/images/color-profile-border-image.html
* virtual/gpu-rasterization/images/gray-scale-png-with-color-profile.html

Not this might be a "real" bug

https://test-results.appspot.com/data/layout_results/WebKit_Mac10_10/47853/layout-test-results/virtual/gpu-rasterization/images/gray-scale-png-with-color-profile-actual.png

has some white lines in it :/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a45ed35b300d0900e5d38c145d8625834c12eefd

commit a45ed35b300d0900e5d38c145d8625834c12eefd
Author: Trent Apted <tapted@chromium.org>
Date: Mon May 28 00:53:56 2018

Widen/Add failed test expectations for Mac (10.10) after skia roll in r562137.

TBR=bsalomon@chromium.org

Bug:  847094 
Change-Id: I237f26b1777de58cef08ebe0effc2b6da54dc299
Reviewed-on: https://chromium-review.googlesource.com/1074771
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562156}
[modify] https://crrev.com/a45ed35b300d0900e5d38c145d8625834c12eefd/third_party/WebKit/LayoutTests/TestExpectations

I've noticed that when I run virtual/gpu-rasterization layout tests locally on ToT with no changes on Macs I get tests that sometimes fail with white lines. It looks similar to the image linked in #3.

I also recently made a Chrome CL that caused layout tests to fail with white lines on the Mac. e.g.: https://chromium-review.googlesource.com/c/chromium/src/+/1080258/2/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/color-profile-image-canvas-expected.png

Locally I don't get these white lines when I run with --use-gl=osmesa or with --additional-driver-flag=--use-gpu-in-tests. It seems to be related to using SwiftShader.

I'm wondering if there is a synchronization/memory coherence issue where SwiftShader has not completely finished rasterization before the image has saved. Maybe there is a missing glFinish() that is required for SwiftShader or something to that effect? Alexis, does this seem plausible?

It sounds plausible to me. Adding capn@ who has better knowledge of synchronization in SwiftShader than I do.

I assume this is Mac specific, i.e. the same tests pass consistently on Windows and Linux? In that case it must be due to the IOSurface extension implementation or usage.

I've only seen this issue on Mac.

Is there a way to disable use of IOSurfaces when running layout tests? I could give it a try locally.

I think --disable-mac-overlays might help remove some of the usage but it might not work with Swiftshader.

IOSurfaces is the interface used by SwiftShader on MacOS, so I don't know of a way of using SwiftShader without IOSurfaces.

I tried --disable-mac-overlays and it didn't resolve the issue. Though, these are pretty simple tests that I would guess are not using overlays.

FWIW, here is a command line that reproduces the white lines pretty consistently for me:

python ./third_party/blink/tools/run_web_tests.py  virtual/gpu-rasterization/images/color-profile --additional-driver-flag=--use-gl=swiftshader --no-retry-failures

Thanks for the info. I should go back to MacOS work within a few days, hopefully. Anyway, you were talking about "a missing glFinish()" earlier. Is that something that can be tried? I'm not sure where that code could be inserted in this case...

The extension spec states that "When a pbuffer is created with type EGL_IOSURFACE_ANGLE, the contents of the associcated IOSurface object are undefined while the pbuffer is bound to a client texture." In other words, it needs to be unbound to synchronize.

Is Chrome doing that consistently before doing anything else with the IOSurface?

Alexis, we have a Surface::sync() method for this, but I don't see it being called when the IOSurface is unbound.

So, I'm not sure about this yet, but you could call:
image[level]->sync();

right before this line here:
https://cs.chromium.org/chromium/src/third_party/swiftshader/src/OpenGL/libGLESv2/Texture.cpp?l=571

It should make sure the texture is ready when Chromium calls eglReleaseTexImage(). I'm not 100% sure Chromium always properly calls eglReleaseTexImage(), though, but it's worth a try.

I made a change to Skia to call glFinish after every draw and that resolves the issue but obviously is not landable. I reverted that and made Alexis' suggested change and that unfortunately does not fix the issue. So perhaps Chrome is not using eglReleaseTexImage().

Yeah, I also just tried this locally. Adding a finish after the draw calls in SwiftShader solves the issue. We have to find out where the IOSurface is read within Chromium that requires some synchronization.

Chris, looping you in because I think of your name when I hear the word IOSurface :)

Hi Chris, I saw that Chromium uses IOSurfaceIsInUse, so I tried doing the synchronization using IOSurfaceIncrementUseCount / IOSurfaceDecrementUseCount, but that doesn't seem to work (maybe I did it wrong). Do you know of a synchronization mechanism that should work for IOSurfaces?

(Oh, and we currently use IOSurfaceLock / IOSurfaceUnlock in SwiftShader)

Capturing the discussion from email:

My understanding is that we are trying to synchronize access to an IOSurface on the CPU and GPU here?

The mental model for IOSurfaces is that the CPU and GPU are accessing the exact same memory (they're not, but that's the illusion IOSurfaces try to provide). The CPU and GPU are running asynchronously, so access will not be synchronized unless done so explicitly (by a glFinish or sync object or similar).

Layout tests don't use CoreAnimation to draw, so IOSurfaceIsInUse won't come into play ("in use" is a "in use by a process", and the driver doesn't use it).

Adding Hotlist-SheriffBot as this affects basically all virtual/gpu-rasterization/images tests that make all Mac 10.10, 10.11 and 10.12 bots very flaky: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=virtual%2Fgpu-rasterization%2Fimages%2F

Not sure whether it makes sense to disable them individually.

Any chance of rolling back?

P0 as this affects dozens of tests.

I started this and stopped (and did not commit)...

 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/color-profile-iframe.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/color-profile-svg.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/content-url-image-with-alt-text-dynamic-2.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/gray-scale-jpeg-with-color-profile.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/color-profile-background-image-repeat.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/color-profile-image-canvas-pattern.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/color-profile-image-canvas-svg.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/color-profile-image-canvas.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/png-with-color-profile.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/rendering-broken-0px-images-quirk.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/rendering-broken-0px-images.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/rendering-broken-10px-images.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/rendering-broken-16px-images.html [ Pass Failure ]
 crbug.com/849256  [ Mac ] virtual/gpu-rasterization/images/rendering-broken-1px-images.html [ Pass Failure ]
 crbug.com/849258  [ Mac ] virtual/gpu-rasterization/images/imagemap-focus-ring-outline-color-not-inherited-from-map.html [ Pass Failure ]
 crbug.com/849258  [ Mac ] virtual/gpu-rasterization/images/imagemap-focus-ring-outline-color.html [ Pass Failure ]
 crbug.com/849258  [ Mac ] virtual/gpu-rasterization/images/imagemap-overflowing-circle-focus-ring.html [ Pass Failure ]

Should we temporarily disable all tests in third_party/WebKit/LayoutTests/images/ for Mac?

It's not obvious what we'd roll back. The flakiness is caused by non-determinism in the interaction with IOSurfaces that AFAIK has existed since we switched to SwiftShader for running Layout Tests on Mac. Changes to Skia, SwiftShader, and Chrome will continue to nudge the flakiness in different directions until access to IOSurfaces is synchronized.

We'll address this with a brute-force workaround imminently: https://swiftshader-review.googlesource.com/19188

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/904042188c4959fea679bdb31a134ba28a745aee

commit 904042188c4959fea679bdb31a134ba28a745aee
Author: Alexis Hetu <sugoi@google.com>
Date: Mon Jun 04 16:50:37 2018

Fixed synchronization issue on MacOS

Because of the way SwiftShader is integrated into Chromium on MacOS,
a synchronization issue exists when Chromium attempts to use an
IOSurface before SwiftShader is done rendering into it. In order to
solve this, all draw calls that end up rendering into an IOSurface
must be synchronized within SwiftShader and can't yield to Chromium
until rendering is complete.

 Bug chromium:847094 

Change-Id: If2dba4fa998e7437ec414d3b4aff9e8b19ecc128
Reviewed-on: https://swiftshader-review.googlesource.com/19188
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>

[modify] https://crrev.com/904042188c4959fea679bdb31a134ba28a745aee/src/OpenGL/common/Image.cpp
[modify] https://crrev.com/904042188c4959fea679bdb31a134ba28a745aee/src/Renderer/Renderer.cpp
[modify] https://crrev.com/904042188c4959fea679bdb31a134ba28a745aee/src/Renderer/Surface.hpp

 Issue 849258  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c1d1d91f0d74adaef3aff961a313a8b9c6a5aedf

commit c1d1d91f0d74adaef3aff961a313a8b9c6a5aedf
Author: Alexis Hetu <sugoi@google.com>
Date: Mon Jun 04 23:24:57 2018

Roll SwiftShader 419e8a7..57776df

https://swiftshader.googlesource.com/SwiftShader.git/+log/419e8a7..57776df

BUG= chromium:847094   chromium:848238 

TBR=kbr@chromium.org

TEST=bots

CQ_INCLUDE_TRYBOTS=luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_chromium_cfi_rel_ng;luci.chromium.try:android_optional_gpu_tests_rel

Change-Id: Ib1ad80065f888a0aab0f9a4b13143967b0ca5a57
Reviewed-on: https://chromium-review.googlesource.com/1085483
Commit-Queue: Alexis Hétu <sugoi@chromium.org>
Reviewed-by: Alexis Hétu <sugoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564288}
[modify] https://crrev.com/c1d1d91f0d74adaef3aff961a313a8b9c6a5aedf/DEPS
[delete] https://crrev.com/00152a250f55c4fa37bb9ee3fe2ed7344f9648d8/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/video-surface-layer/media/controls-without-preload-expected.png
[delete] https://crrev.com/00152a250f55c4fa37bb9ee3fe2ed7344f9648d8/third_party/WebKit/LayoutTests/platform/mac/virtual/video-surface-layer/media/controls-without-preload-expected.png
[modify] https://crrev.com/c1d1d91f0d74adaef3aff961a313a8b9c6a5aedf/third_party/WebKit/LayoutTests/virtual/gpu-rasterization/images/rgb-png-with-cmyk-color-profile-expected.png

Should be fixed now.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f73246ba7034539dbda70502ec21b53b06cd68b3

commit f73246ba7034539dbda70502ec21b53b06cd68b3
Author: Alexis Hetu <sugoi@google.com>
Date: Wed Jun 06 16:56:27 2018

Re-enable some disabled MacOS Layout Tests

Now that synchronization issues have been fixed on MacOS,
most previously disabled layout tests can be re-enabled.

TBR=kbr@chromium.org
Bug: chromium:726075   chromium:646528   chromium:845267  chromium:614910  chromium:845266  chromium:791941  chromium:847094   chromium:849256 

Change-Id: Ifa59c2594c429a94643ea9f19b2e08e974cc6c53
Reviewed-on: https://chromium-review.googlesource.com/1088197
Reviewed-by: Alexis Hétu <sugoi@chromium.org>
Commit-Queue: Alexis Hétu <sugoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564927}
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/color-profile-border-image-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/color-profile-munsell-srgb-to-srgb-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/pixel-crack-image-background-webkit-transform-scale-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/gpu-rasterization/images/color-profile-background-image-cover-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/mac-mac10.12/virtual/gpu-rasterization/images/color-profile-background-image-cover-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/gpu-rasterization/images/color-profile-border-image-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/gpu-rasterization/images/pixel-crack-image-background-webkit-transform-scale-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-munsell-adobe-to-srgb-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-munsell-srgb-to-srgb-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/pixel-crack-image-background-webkit-transform-scale-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-background-image-cover-expected.png
[delete] https://crrev.com/2db1569218f1a28d439fcf3b031b9cce41a2f771/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-clip-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/virtual/gpu-rasterization/images/color-profile-background-image-cover-expected.png
[modify] https://crrev.com/f73246ba7034539dbda70502ec21b53b06cd68b3/third_party/WebKit/LayoutTests/virtual/gpu-rasterization/images/color-profile-clip-expected.png