LibWeb: Rework AnimationTimeline's monotonically increasing property
Some checks are pending
CI / macOS, arm64, Sanitizer, Clang (push) Waiting to run
CI / Linux, x86_64, Fuzzers, Clang (push) Waiting to run
CI / Linux, x86_64, Sanitizer, GNU (push) Waiting to run
CI / Linux, x86_64, Sanitizer, Clang (push) Waiting to run
Package the js repl as a binary artifact / Linux, arm64 (push) Waiting to run
Package the js repl as a binary artifact / macOS, arm64 (push) Waiting to run
Package the js repl as a binary artifact / Linux, x86_64 (push) Waiting to run
Run test262 and test-wasm / run_and_update_results (push) Waiting to run
Lint Code / lint (push) Waiting to run
Label PRs with merge conflicts / auto-labeler (push) Waiting to run
Push notes / build (push) Waiting to run

Our previous implementation kept track of an AnimationTimeline being
monotonically increasing, by looking at new time values coming in and
setting `m_monotonically_increasing` to `false` whenever a new value
is before the previous known time value.

As far as I can tell, the spec doesn't really ask us to do so: it just
defines 'monotonically increasing' as a property of a timeline, i.e. it
guarantees that returned time values from `::current_time()` are always
greater than or equal to the last returned value.

This fixes a common crash seen when the last render opportunity lies
before the document's origin time, and `::set_current_time()` was
invoked with a negative value. This was especially visible in the
`Text/input/wpt-import/css/cssom/CSSStyleSheet-constructable.html` test.
This commit is contained in:
Jelle Raaijmakers 2025-07-29 10:15:12 +02:00 committed by Jelle Raaijmakers
commit aa563706ca
Notes: github-actions[bot] 2025-07-30 12:39:08 +00:00
5 changed files with 30 additions and 18 deletions

View file

@ -560,7 +560,7 @@ WebIDL::ExceptionOr<void> Animation::play()
return play_an_animation(AutoRewind::Yes);
}
// https://www.w3.org/TR/web-animations-1/#play-an-animation
// https://drafts.csswg.org/web-animations-1/#playing-an-animation-section
WebIDL::ExceptionOr<void> Animation::play_an_animation(AutoRewind auto_rewind)
{
// 1. Let aborted pause be a boolean flag that is true if animation has a pending pause task, and false otherwise.
@ -666,16 +666,17 @@ WebIDL::ExceptionOr<void> Animation::play_an_animation(AutoRewind auto_rewind)
// 12. Schedule a task to run as soon as animation is ready. The task shall perform the following steps:
//
// Note: Steps omitted, set run_pending_play_task()
// Note: Steps omitted, see run_pending_play_task()
//
// So long as the above task is scheduled but has yet to run, animation is described as having a pending play
// task. While the task is running, however, animation does not have a pending play task.
//
// If a user agent determines that animation is immediately ready, it may schedule the above task as a microtask
// such that it runs at the next microtask checkpoint, but it must not perform the task synchronously.
// FIXME: Below actions should only happen when the animation is actually ready.
m_pending_play_task = TaskState::Scheduled;
if (m_timeline)
m_saved_play_time = m_timeline->current_time().value();
m_saved_play_time = m_timeline->current_time();
// 13. Run the procedure to update an animations finished state for animation with the did seek flag set to false,
// and the synchronously notify flag set to false.

View file

@ -13,23 +13,31 @@ namespace Web::Animations {
GC_DEFINE_ALLOCATOR(AnimationTimeline);
// https://drafts.csswg.org/web-animations-1/#dom-animationtimeline-currenttime
Optional<double> AnimationTimeline::current_time() const
{
// Returns the current time for this timeline or null if this timeline is inactive.
if (is_inactive())
return {};
return m_current_time;
}
void AnimationTimeline::set_current_time(Optional<double> value)
{
if (value == m_current_time)
return;
if (m_is_monotonically_increasing && m_current_time.has_value()) {
if (!value.has_value() || value.value() < m_current_time.value())
m_is_monotonically_increasing = false;
if (m_is_monotonically_increasing && m_current_time.has_value() && (!value.has_value() || *value < *m_current_time)) {
dbgln("AnimationTimeline::set_current_time({}): monotonically increasing timeline can only move forward", value);
return;
}
m_current_time = value;
// The loop might modify the content of m_associated_animations, so let's iterate over a copy.
auto temporary_copy = GC::RootVector<GC::Ref<Animation>>(vm().heap());
temporary_copy.extend(m_associated_animations.values());
for (auto& animation : temporary_copy) {
for (auto& animation : temporary_copy)
animation->notify_timeline_time_did_change();
}
}
void AnimationTimeline::set_associated_document(GC::Ptr<DOM::Document> document)
@ -41,10 +49,10 @@ void AnimationTimeline::set_associated_document(GC::Ptr<DOM::Document> document)
m_associated_document = document;
}
// https://www.w3.org/TR/web-animations-1/#inactive-timeline
// https://drafts.csswg.org/web-animations-1/#timeline
bool AnimationTimeline::is_inactive() const
{
// A timeline is considered to be inactive when its time value is unresolved.
// A timeline is considered to be inactive when its time value is unresolved, and active otherwise.
return !m_current_time.has_value();
}

View file

@ -16,7 +16,7 @@ class AnimationTimeline : public Bindings::PlatformObject {
GC_DECLARE_ALLOCATOR(AnimationTimeline);
public:
Optional<double> current_time() const { return m_current_time; }
Optional<double> current_time() const;
virtual void set_current_time(Optional<double>);
GC::Ptr<DOM::Document> associated_document() const { return m_associated_document; }
@ -43,8 +43,8 @@ protected:
// https://www.w3.org/TR/web-animations-1/#dom-animationtimeline-currenttime
Optional<double> m_current_time {};
// https://www.w3.org/TR/web-animations-1/#monotonically-increasing-timeline
bool m_is_monotonically_increasing { true };
// https://drafts.csswg.org/web-animations-1/#monotonically-increasing-timeline
bool m_is_monotonically_increasing { false };
// https://www.w3.org/TR/web-animations-1/#timeline-associated-with-a-document
GC::Ptr<DOM::Document> m_associated_document {};

View file

@ -51,20 +51,22 @@ Optional<double> DocumentTimeline::convert_a_timeline_time_to_an_origin_relative
return timeline_time.value() + m_origin_time;
}
// https://www.w3.org/TR/web-animations-1/#origin-time
// https://drafts.csswg.org/web-animations-1/#document-timeline
void DocumentTimeline::set_current_time(Optional<double> current_time)
{
// A document timeline is a type of timeline that is associated with a document and whose current time is calculated
// as a fixed offset from the now timestamp provided each time the update animations and send events procedure is
// run. This fixed offset is referred to as the document timelines origin time.
// run. This fixed offset is equal to the current time of the default document timeline when this timelines current
// time was zero, and is thus referred to as the document timelines origin time.
if (!current_time.has_value())
Base::set_current_time({});
else
Base::set_current_time(current_time.value() - m_origin_time);
// https://drafts.csswg.org/web-animations-1/#ref-for-active-timeline
// After a document timeline becomes active, it is monotonically increasing.
if (!is_inactive())
VERIFY(is_monotonically_increasing());
if (!m_is_monotonically_increasing && !is_inactive())
m_is_monotonically_increasing = true;
}
// https://www.w3.org/TR/web-animations-1/#document-timelines

View file

@ -29,6 +29,7 @@ InternalAnimationTimeline::InternalAnimationTimeline(JS::Realm& realm)
: AnimationTimeline(realm)
{
m_current_time = 0.0;
m_is_monotonically_increasing = true;
auto& document = as<HTML::Window>(HTML::relevant_global_object(*this)).associated_document();
document.associate_with_timeline(*this);