Hey kainino@

We are seeing a failure in webkit_layout_tests, where you landed a change to video rendering:
https://chromium-review.googlesource.com/c/chromium/src/+/1164163

Where the produced images doesn't match expectations.

Could you help confirm if this is your change?

This looks like it only failed once - so I guess it's flaky? (If it's my change, it would make sense if it were flaky.)

However, if it is flaky, it also is possible that the culprit change was in an earlier commit.

I'm trying to look at the diff, but it doesn't make sense:
https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=1648edf32fc4d9c07c6fe7d427159992f35aa015&as=video-fixed-background-expected.png
and
https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=0ae9103f3cf2ad076b98b24d58f9b3acaa14cf64&as=video-fixed-background-actual.png
look the same to me, but the diff shows something:
https://isolateserver.appspot.com/browse?namespace=default-gzip&digest=e0361cf9a2f830a035f24a4abcca1adadc26b3f1&as=video-fixed-background-diff.png

(expected and actual are the same image with the same alpha channel, while diff is also the same image - but with a 0.5 alpha channel.)

So I'm wondering if the change was color related, or something that changed how the PNG files are encoded or diffed?

e.g. this CL changed pow to powf (seems like an unlikely candidate to me, though):
https://chromium-review.googlesource.com/c/chromium/src/+/1176591

Looks like there has been a few rare flakes on a different linux-dcheck-off-rel bot:

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests%20(with%20patch)&tests=virtual%2Fandroid%2Ffullscreen%2Fvideo-fixed-background.html

So it could be a rare flake, or as you said an unrelated change.

I'm removing Sheriff-Chromium flag as the bot is green again. If it looks like this isn't a reproducible flake feel free to close it out.

I'll take another look at the build history in a few days and see if it's flaked again.

The NextAction date has arrived: 2018-08-24

Seems to still be flaking: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-xenial-rel/2674

Speculatively reverting to see if this flake disappears:
https://chromium-review.googlesource.com/c/chromium/src/+/1192129

Here's the dashboard I'm looking at:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=site_per_process_webkit_layout_tests&tests=virtual%2Fandroid%2Ffullscreen%2Fvideo-fixed-background.html

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

commit de9497de9793e055da360fcadcf5e26da9f3262b
Author: Kai Ninomiya <kainino@chromium.org>
Date: Mon Aug 27 23:49:04 2018

Revert "Use unverified sync tokens in PaintCanvasVideoRenderer"

This reverts commit 228446ba19c6a6f6cd7b61402002b882523cfe07.

Reason for revert: Potentially causing flakes in virtual/android/fullscreen/video-fixed-background.html

Bug: 876466

Original change's description:
> Use unverified sync tokens in PaintCanvasVideoRenderer
> 
> PaintCanvasVideoRenderer::CopyVideoFrameTexturesToGLTexture
> unnecessarily used verified sync tokens to pass textures between
> Skia's context and WebGL's context. Verification isn't needed
> because these two clients are being used synchronously from the
> same process and thread.
> 
> Changing these sync tokens to be unverified gives us a nice
> speedup - on a simple mp4-to-WebGL case on Linux, the texImage2D
> blocking time drops from ~1.25 ms to ~0.70ms (~80% speedup).
> 
> Bug:  871417 
> Change-Id: I38c246fdb49588eccc87211fa06a2abf00e55c5c
> Reviewed-on: https://chromium-review.googlesource.com/1164163
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Commit-Queue: Kai Ninomiya <kainino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584840}

TBR=sandersd@chromium.org,kainino@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  871417 
Change-Id: I51d3e5f9308eadf08babec4e1c99b1d51c4a3b90
Reviewed-on: https://chromium-review.googlesource.com/1192129
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586500}
[modify] https://crrev.com/de9497de9793e055da360fcadcf5e26da9f3262b/media/renderers/paint_canvas_video_renderer.cc

The NextAction date has arrived: 2018-08-29

It does seem to have stopped flaking, but the last flake was in "Chromium: r586607 to r586618" while my revert landed in r586500. I'll try relanding this after the branch to see what happens.

The NextAction date has arrived: 2018-08-31

Latest flake is on r588208 to r588217. Definitely not suspecting my change in this, although it's still possible there was another unrelated regression around the same time. I don't know offhand if it's possible to bisect this flake, but that would be the preferred next step.

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

commit c9fce27cac9d96085bd8bd29e345dc2c6b378a25
Author: Kai Ninomiya <kainino@chromium.org>
Date: Mon Sep 03 01:04:41 2018

Reland "Use unverified sync tokens in PaintCanvasVideoRenderer"

This reverts commit de9497de9793e055da360fcadcf5e26da9f3262b.

Reason for revert: flake continued after revert,
so this change wasn't responsible.

Original change's description:
> Revert "Use unverified sync tokens in PaintCanvasVideoRenderer"
> 
> This reverts commit 228446ba19c6a6f6cd7b61402002b882523cfe07.
> 
> Reason for revert: Potentially causing flakes in virtual/android/fullscreen/video-fixed-background.html
> 
> Bug: 876466
> 
> Original change's description:
> > Use unverified sync tokens in PaintCanvasVideoRenderer
> > 
> > PaintCanvasVideoRenderer::CopyVideoFrameTexturesToGLTexture
> > unnecessarily used verified sync tokens to pass textures between
> > Skia's context and WebGL's context. Verification isn't needed
> > because these two clients are being used synchronously from the
> > same process and thread.
> > 
> > Changing these sync tokens to be unverified gives us a nice
> > speedup - on a simple mp4-to-WebGL case on Linux, the texImage2D
> > blocking time drops from ~1.25 ms to ~0.70ms (~80% speedup).
> > 
> > Bug:  871417 
> > Change-Id: I38c246fdb49588eccc87211fa06a2abf00e55c5c
> > Reviewed-on: https://chromium-review.googlesource.com/1164163
> > Reviewed-by: Dan Sanders <sandersd@chromium.org>
> > Commit-Queue: Kai Ninomiya <kainino@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#584840}
> 
> TBR=sandersd@chromium.org,kainino@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug:  871417 
> Change-Id: I51d3e5f9308eadf08babec4e1c99b1d51c4a3b90
> Reviewed-on: https://chromium-review.googlesource.com/1192129
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>
> Commit-Queue: Kai Ninomiya <kainino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#586500}

TBR=sandersd@chromium.org,kainino@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 876466,  871417 
Change-Id: Ic5c2206901239695b8289f0fa3e63f925094444d
Reviewed-on: https://chromium-review.googlesource.com/1200487
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588308}
[modify] https://crrev.com/c9fce27cac9d96085bd8bd29e345dc2c6b378a25/media/renderers/paint_canvas_video_renderer.cc

My change wasn't the issue. Actually, I just figured out how to get some slightly older results out of the flakiness dashboard. The first flake I can see was Linux%20Tests/71853 (r584213), but there is no data before that.

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=site_per_process_webkit_layout_tests&showAllRuns=true&tests=virtual%2Fandroid%2Ffullscreen%2Fvideo-fixed-background.html