2006258 - Process stale timelines in HTML event loop

Status: ASSIGNED

(Core :: CSS Transitions and Animations, enhancement)

Description

[Boris Chiou [:boris]]

Split the stale timelines part from Bug 1817051 because we may need to check more things in the patch.

Comment 1

[Boris Chiou [:boris]]

Attachment: Bug 2006258 - Update stale timelines. r?#layout-scroll-driven-animation-reviewers

Per spec, we need an extra steps in HTML event loop to update the stale
timelines after we update the animation frame callbacks and resize
observer callbacks.

The wpt for stale timelines rely on the DOM APIs which we don't support
now, so this patch doesn't update any wpt expectation. For now we just
follow the spec to try to update the stale timelines.

Comment 2

[Boris Chiou [:boris]]

The implementation of WebKit:

1. Use m_updatedScrollTimelines to hold a list of scroll timelines which just updated during updateAnimationsAndSendEvents()
2. Then call runPostRenderingUpdateAnimationTasks() after layoutIfNeeded() and updateResizeObservations().
3. This function checks m_updatedScrollTimelines and try to update them if they are staled, which means the offset or the range is changed.
4. May do style update if any of the associated animations have KeyframeEffect.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I've rebased D269665 and revised it. I am going to upload it now.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2006258 - Return a boolean from UpdateCachedCurrentTime to indicate whether the cached value changed. r?#layout-scroll-driven-animation-reviewers

This lets callers detect staleness without having to snapshot fields
around the call, and makes the comparison subclass-aware (ViewTimeline
now compares its full CurrentTimeData via operator==).

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2006258 - Return a boolean from UpdateCachedCurrentTime to indicate whether the cached value changed. r?#layout-scroll-driven-animation-reviewers

This lets callers detect staleness without having to snapshot fields
around the call, and makes the comparison subclass-aware (ViewTimeline
now compares its full CurrentTimeData via operator==).

Comment 7

[David Shin[:dshin]]

So Chromium melds this into snapshot post-layout state steps, where a lot of different things are handled as "snapshot clients," including scroll timelines. The snapshot clients are added at the scroll timeline creation time.
WebKit seemingly collects every timeline (While the set says "updated timelines," any timeline with a valid source will get added) that has non-null source every refreshdriver tick, to be updated later if stale.

OTOH, the current spec seems more involved... I think it says to collect into stale timelines:

• Any newly-created timelines during the RO cycle (i.e. layout flush then gather & send RO events)
• Any existing timelines whose named timeline range changed as a result of the RO cycle

... But computing this set basically requires iterating through all existing scroll timelines anyway?

Comment 8

[Boris Chiou [:boris]]

(In reply to David Shin[:dshin] from comment #7)

So Chromium melds this into snapshot post-layout state steps, where a lot of different things are handled as "snapshot clients," including scroll timelines. The snapshot clients are added at the scroll timeline creation time.
WebKit seemingly collects every timeline (While the set says "updated timelines," any timeline with a valid source will get added) that has non-null source every refreshdriver tick, to be updated later if stale.

OTOH, the current spec seems more involved... I think it says to collect into stale timelines:

• Any newly-created timelines during the RO cycle (i.e. layout flush then gather & send RO events)
• Any existing timelines whose named timeline range changed as a result of the RO cycle

... But computing this set basically requires iterating through all existing scroll timelines anyway?

For now, yes. Or, we probably need to set something like a dirty flag for the timelines changed ONLY In RO cycle, or a more elegant way like Chromium does. This sounds a lot of work. I'm fine with checking all existing scroll timelines for now. Fortunately we do some early returns in UpdateCachedCurrentTime() to make this pass slightly quicker, and it is unlikely to return true in the common case. If any performance issues happen, we could revisit it.