fsamuel@, do you know if surface sync is plumbed into spots that this test hits?

The surface out of which we want to read doesn't seem to be ever activated. This is due to the fact that we wait for all compositing stages to complete in display compositor pixel dumps. Without this, the surface is activated on a deadline and the test passes. However, that isn't the right solution since it introduces races. The real question is why the surface dependency is never satisfied. 

The dependency seems to come from surface->activation_dependencies and it isn't known to the surface_manager_. I'm guessing it's the webgl canvas that uses the surface when the patch in OP is applied. 

fsamuel@, do you mind taking a look, since I think you're in a better position to figure out if the patch is missing something or if the existing code is misbehaving somehow. 

I'll take a look. Activation dependencies aren't typically known to SurfaceManager until later. That part is fine. 

Looks like the bug is in the CL. The first op we do is block (forever) on a surface that never gets a CompositorFrame.

I think the solution is to just delete https://cs.chromium.org/chromium/src/cc/layers/surface_layer.cc?q=surface_layer.cc&sq=package:chromium&g=0&l=128

SurfaceLayer::HasDrawableContent()

We shouldn't wait for a valid SurfaceRange. We won't produce any quads if we don't have a valid SurfaceRange but we will get the WillDraw callback.

I'll have a fix ready soon.

This is the fix I believe: https://chromium-review.googlesource.com/c/chromium/src/+/1161338

Passing back to Mounir after chatting offline about the solution.

fsamuel@, I would be interested to understand why this specific test would fail if the issue was as generic as the one we discussed? It sounds that CJ's change is working except for this very specific instance and that's a test that ends up doing reftesting on some text. I would be tempted to simply change the test to use the w3c test harness which would probably avoid the reftest and would allow us to move on. WDYT?

I would need to dig into this further (and I'm low on cycles at the moment). My hunch is turning on --enable-display-compositor-pixel-dump --enable-threaded-compositing turns on surface sync with unlimited deadlines which reveals the bug. 

This is definitely a real bug. 

Ping mlamouri@, should this be a P1, and if so can you provide an update? (Triaging stale P1s in Blink>Infra.)

Ping again: mlamouri@chromium.org can you provide an update on re-prioritization and/or progress?

Thanks!

I don't think merging both path is a P1 (ie. https://chromium-review.googlesource.com/c/chromium/src/+/1152491). We could even close this bug as https://chromium-review.googlesource.com/c/chromium/src/+/1152491 did not land and only make sense in the context of this CL.