(In reply to David Shin[:dshin] from comment #7) > So Chromium melds this into [snapshot post-layout state steps](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame_view.cc;l=2651;drc=01180e2737b8b04a2a6868eca56df96196ec789c), where a lot of different things are handled as "[snapshot clients](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame.cc;l=4094;drc=01180e2737b8b04a2a6868eca56df96196ec789c)," including [scroll timelines](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.h;l=22-23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). The snapshot clients are [added](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/post_layout_snapshot_client.cc;l=13;drc=01180e2737b8b04a2a6868eca56df96196ec789c) at the [scroll timeline creation time](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.cc;l=23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). > [WebKit](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#147) seemingly collects every timeline (While the set says "updated timelines," any timeline with a [valid source](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/ScrollTimeline.cpp#294) will get added) that has non-null source every refreshdriver tick, to be [updated later](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#262,264) if stale. > > OTOH, the [current spec](https://drafts.csswg.org/scroll-animations-1/#event-loop) 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. If this causes performance issue, we could revisit this.
Bug 2006258 Comment 8 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to David Shin[:dshin] from comment #7) > So Chromium melds this into [snapshot post-layout state steps](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame_view.cc;l=2651;drc=01180e2737b8b04a2a6868eca56df96196ec789c), where a lot of different things are handled as "[snapshot clients](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame.cc;l=4094;drc=01180e2737b8b04a2a6868eca56df96196ec789c)," including [scroll timelines](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.h;l=22-23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). The snapshot clients are [added](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/post_layout_snapshot_client.cc;l=13;drc=01180e2737b8b04a2a6868eca56df96196ec789c) at the [scroll timeline creation time](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.cc;l=23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). > [WebKit](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#147) seemingly collects every timeline (While the set says "updated timelines," any timeline with a [valid source](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/ScrollTimeline.cpp#294) will get added) that has non-null source every refreshdriver tick, to be [updated later](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#262,264) if stale. > > OTOH, the [current spec](https://drafts.csswg.org/scroll-animations-1/#event-loop) 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 this causes performance issue, we could revisit this.
(In reply to David Shin[:dshin] from comment #7) > So Chromium melds this into [snapshot post-layout state steps](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame_view.cc;l=2651;drc=01180e2737b8b04a2a6868eca56df96196ec789c), where a lot of different things are handled as "[snapshot clients](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame.cc;l=4094;drc=01180e2737b8b04a2a6868eca56df96196ec789c)," including [scroll timelines](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.h;l=22-23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). The snapshot clients are [added](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/post_layout_snapshot_client.cc;l=13;drc=01180e2737b8b04a2a6868eca56df96196ec789c) at the [scroll timeline creation time](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.cc;l=23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). > [WebKit](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#147) seemingly collects every timeline (While the set says "updated timelines," any timeline with a [valid source](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/ScrollTimeline.cpp#294) will get added) that has non-null source every refreshdriver tick, to be [updated later](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#262,264) if stale. > > OTOH, the [current spec](https://drafts.csswg.org/scroll-animations-1/#event-loop) 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 issue happens, we could revisit it.
(In reply to David Shin[:dshin] from comment #7) > So Chromium melds this into [snapshot post-layout state steps](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame_view.cc;l=2651;drc=01180e2737b8b04a2a6868eca56df96196ec789c), where a lot of different things are handled as "[snapshot clients](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame.cc;l=4094;drc=01180e2737b8b04a2a6868eca56df96196ec789c)," including [scroll timelines](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.h;l=22-23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). The snapshot clients are [added](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/post_layout_snapshot_client.cc;l=13;drc=01180e2737b8b04a2a6868eca56df96196ec789c) at the [scroll timeline creation time](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/animation/scroll_snapshot_timeline.cc;l=23;drc=01180e2737b8b04a2a6868eca56df96196ec789c). > [WebKit](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#147) seemingly collects every timeline (While the set says "updated timelines," any timeline with a [valid source](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/ScrollTimeline.cpp#294) will get added) that has non-null source every refreshdriver tick, to be [updated later](https://searchfox.org/wubkat/rev/d221d71a6a560cb45bb3e9b23e526b6bcfa9e4b3/Source/WebCore/animation/AnimationTimelinesController.cpp#262,264) if stale. > > OTOH, the [current spec](https://drafts.csswg.org/scroll-animations-1/#event-loop) 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.