These tests are flaky on Mac WK1: web-platform-tests/intersection-observer/bounding-box.html web-platform-tests/intersection-observer/display-none.html web-platform-tests/intersection-observer/containing-block.html My suspicion is that the Timer-based scheduling we're using doesn't always generate updates when the tests expect, since the tests are written in a way that assumes HTMLEventLoop-driven scheduling.
Created attachment 348417 [details] Suppress flaky failures
Comment on attachment 348417 [details] Suppress flaky failures Clearing flags on attachment: 348417 Committed r235471: <https://trac.webkit.org/changeset/235471>
What's interesting about these three test is they all involve changes to style which change intersection, and in the flaky failures we're not getting an intersection update at the point where the test expects one. For changes to style, we first need a style update to happen, which then triggers a layout update, which then schedules an intersection update, and that in turn schedules notifications. The WPTs expect layout and style to be updated right after the next rAF, and intersections to be updated right after that. One thing we can do to reduce the delay for notifications is to fire them immediately after the intersection update rather than scheduled on zero-second Timer (since the intersection update itself is already scheduled using a Timer). These tests are also flaking on WK2, so I'm going to generalize the suppression until we have a fix landed and the tests are no longer flaking.
Created attachment 348432 [details] Generalize suppression
Comment on attachment 348432 [details] Generalize suppression Clearing flags on attachment: 348432 Committed r235479: <https://trac.webkit.org/changeset/235479>
Given the description above I suspect these flaky results reflect a real bug that could adversely affect interoperability with other browser engines. Even though these timing properties are not well specified, I think they could possibly be important for compatibility, not just for the test harness.
(In reply to Darin Adler from comment #6) > Given the description above I suspect these flaky results reflect a real bug > that could adversely affect interoperability with other browser engines. > Even though these timing properties are not well specified, I think they > could possibly be important for compatibility, not just for the test harness. Yes, trying to make the timing of these events more predictable (and more consistent with other browsers) seems worth doing. The approach I'm considering is scheduling these updates as part of the layer update lifecycle (e.g., scheduling them right after flushLayers()). At that point we know that layout and style are up-to-date, so it's safe to compute intersections.
Created attachment 351259 [details] Patch
Comment on attachment 351259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351259&action=review > Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp:258 > +#if ENABLE(INTERSECTION_OBSERVER) > + m_webPage.updateIntersectionObservations(); > +#endif Would be nice to add a "willDisplayPage" bottleneck to WebPage and/or Page and move the #ifdef lower down.
Created attachment 352494 [details] Patch for landing
(In reply to Simon Fraser (smfr) from comment #9) > Comment on attachment 351259 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351259&action=review > > > Source/WebKit/WebProcess/WebPage/AcceleratedDrawingArea.cpp:258 > > +#if ENABLE(INTERSECTION_OBSERVER) > > + m_webPage.updateIntersectionObservations(); > > +#endif > > Would be nice to add a "willDisplayPage" bottleneck to WebPage and/or Page > and move the #ifdef lower down. Done, added willDisplayPage to WebPage and Page and moved the #if down to Page.
Comment on attachment 352494 [details] Patch for landing Clearing flags on attachment: 352494 Committed r237218: <https://trac.webkit.org/changeset/237218>
All reviewed patches have been landed. Closing bug.
<rdar://problem/45337305>