I have investigated this and the cause seems to be because layout tests run with disabled thread compositing [1]. Removing the flag from layout tests will make the test pass.

[1] https://cs.chromium.org/chromium/src/content/shell/app/shell_main_delegate.cc?rcl=a7328d1a9ecaeefd6815533193ae350cbf657588&l=212

thanks for looking into this!

it seems like non-threaded compositing does some unexpected things.  for example, the red border only shows up around the (blank) video element in threaded mode.  doesn't matter if css reflection is enabled or not.

i also tried running the test through content_shell rather than the test runner, with threaded mode on and off.  with threaded mode off, it failed 585 / 585 times due to pixel mis-matches.  however, with threaded mode on, it failed 114 / 586 times.

i wasn't able to get the test to pass either way through the test runner script, though.  i even modified render_thread_impl to always enable threaded compositing.

i will look at this more after the new year.

turns out that this call controls whether the test fails or not:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/PaintLayer.cpp?rcl=5b2de20541d2b08bf64e81804b538eb4d03a4e21&l=3134

conveniently, it's marked with a TODO to avoid it since it has side-effects.  :)

over to fs for help understanding what's happening.  the call in question (c#4) was added in https://codereview.chromium.org/2490163002 .

TL;DR: computing IntersectionGeometry in HTMLMediaElement in StartPlayerLoad causes a test failure when the reflection filter is used, due to the side-effects caused by the call to LastFilterEffect() .

I haven't looked any deeper into this, but if you would want to do something like this (use IntersectionGeometry), you'd need to ensure that the document lifecycle is sufficiently updated, or stale data will be used. StartPlayerLoad is not unlikely to invalidate layout itself (due to mutations of the "display state"), so it doesn't feel like a great place for doing something like that (whatever it now happens to be that you want to do...)

This appears to be caused by the same underlying issue as issue 658305 - the added IntersectionGeometry call ends up computing and caching a FilterEffect chain on the PaintLayer using a LayoutObject/Box that is empty. The reflection is a part of the cached FilterEffect and has the LayoutBox dimensions (0x0) "baked" into it. This cached FilterEffect chain is then kept around (not invalidated) despite the dimensions of the box and layer changing.
I verified that a WIP CL (w/ minor tweak to handle box-reflect) for said issue also fixes this issue, so marking as dupe. Comment 6 still applies though.

thanks for the information!  i had a feeling that i was breaking some contract or other.

is posting the call to IntersectionGeometry, rather than calling it in StartPlayerLoading, sufficient to make it safe?

Delaying the call doesn't really help as such, what is required for correctness is to ensure that the document lifecycle is sufficiently advanced (probably at least CompositingClean.) That is best done somewhere "well-defined" - which is often most easily achieved by posting a task. That can still trigger the above however, so it's not really a guarantee against that.

fs: thanks for this information.  after quite a bit of fiddling, i have an additional question.

TL;DR: for my application (heuristic ordering of media players for resource allocation), stale data often is better than waiting for proper data.  if i avoid computing the intersection for elements with LayoutObject::HasFilterInducingProperty, would it be safe?

it's entirely fine if the data is sometimes wrong -- from local testing, it's good enough quite often for my needs.

I guess it's one of those "in theory, no - in practice, most of the time" type situation. Maybe an approach similar to what some other media element heuristic/statistic thing uses (which is essentially polling IIRC) would work? Maybe they can even use the same mechanism? (If using an IntersectionObserver proper is deemed to costly that is.)

the problem i've run into is that the document lifecycle is often < kCompositingClean from the start of HTMLMediaElement's ctor to the time i need to consume the data.

That makes it sound like you'd need to force a lifecycle update to get any data at all. Forcing the lifecycle is more often than not not a good idea (maybe even "never".) Thinking about how something fits (or doesn't) with the document lifecycle is fairly important, and may avoid further complications down the line. Using an IntersectionObserver would have the advantage of "observing" the lifecycle rather than forcing it in one way or another.

> to force a lifecycle update to get any data at all

hrm, it seems that you're right.  the stale data i was getting was still later than i need it.

it seems like there isn't a good solution here.  we don't want to delay player load, since it impacts user experience.  however, blink just doesn't always know where on a page the element is by the time player load starts.

thanks for taking the time to explain it.  i appreciate it.