[webkit-reviews] review granted: [Bug 202109] [Web Animations] Coordinate "update animations and send events" procedure across multiple timelines : [Attachment 397183] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Antoine Quint <graouts at apple.com>'s
request for review:
Bug 202109: [Web Animations] Coordinate "update animations and send events"
procedure across multiple timelines
https://bugs.webkit.org/show_bug.cgi?id=202109

Attachment 397183: Patch

https://bugs.webkit.org/attachment.cgi?id=397183&action=review




--- Comment #13 from Dean Jackson <dino at apple.com> ---
Comment on attachment 397183
  --> https://bugs.webkit.org/attachment.cgi?id=397183
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=397183&action=review

> Source/WebCore/ChangeLog:13
> +	   So far, although we did manage multiple animation timelines per
document, we mostly operated under the assumption that there really was a
single timeline.
> +	   In this patch we make the "update animations and send events"
procedure, which is central to the lifecycle of animations, work with multiple
timelines such
> +	   that a single microtask checkpoint is performed even with multiple
timelines, whereas we would perform one per timeline before. To do this, we
move much of
> +	   the logic DocumentTimeline::updateAnimationsAndSendEvents() to
DocumentTimelinesController where each step is run across each timeline, rather
than running
> +	   all steps for each timeline one after the other, respecting the
single microtask checkpoint in the middle of the process.

Why are you wrapping the lines at such a high column? I think you want at least
half this width.

> Source/WebCore/animation/DocumentTimelinesController.cpp:71
> +    Vector<RefPtr<DocumentTimeline>> protectedTimelines;
> +    for (auto& timeline : m_timelines)
> +	   protectedTimelines.append(&timeline);

Vector<RefPtr<DocumentTimeline>> protectedTimelines { m_timelines };