From f4636a0cf5e8e4c5e5a7267dd26d617d37b12e48 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 23 May 2024 10:24:32 +0200 Subject: [PATCH] LibWeb: Stop spamming animation events on the wrong event target This patch fixes two issues: - Animation events that should go to the target element now do (some were previously being dispatched on the animation itself.) - We update the "previous phase" and "previous iteration" fields of animation effects, so that we can actually detect phase changes. This means we stop thinking animations always just started, something that caused each animation to send 60 animationstart events every second (to the wrong target!) --- .../misc/animation-events-basic.txt | 2 + .../misc/animation-events-basic.html | 46 +++++++++++++++++++ .../Libraries/LibWeb/Animations/Animation.cpp | 9 ++-- Userland/Libraries/LibWeb/DOM/Document.cpp | 23 +++++++--- Userland/Libraries/LibWeb/DOM/Document.h | 3 +- 5 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/WebAnimations/misc/animation-events-basic.txt create mode 100644 Tests/LibWeb/Text/input/WebAnimations/misc/animation-events-basic.html diff --git a/Tests/LibWeb/Text/expected/WebAnimations/misc/animation-events-basic.txt b/Tests/LibWeb/Text/expected/WebAnimations/misc/animation-events-basic.txt new file mode 100644 index 00000000000..9e40aaf85f0 --- /dev/null +++ b/Tests/LibWeb/Text/expected/WebAnimations/misc/animation-events-basic.txt @@ -0,0 +1,2 @@ + animationstart +animationend diff --git a/Tests/LibWeb/Text/input/WebAnimations/misc/animation-events-basic.html b/Tests/LibWeb/Text/input/WebAnimations/misc/animation-events-basic.html new file mode 100644 index 00000000000..b0ccd2dc6c4 --- /dev/null +++ b/Tests/LibWeb/Text/input/WebAnimations/misc/animation-events-basic.html @@ -0,0 +1,46 @@ + + + +
+ + diff --git a/Userland/Libraries/LibWeb/Animations/Animation.cpp b/Userland/Libraries/LibWeb/Animations/Animation.cpp index b04482d872d..a2a654d7f14 100644 --- a/Userland/Libraries/LibWeb/Animations/Animation.cpp +++ b/Userland/Libraries/LibWeb/Animations/Animation.cpp @@ -456,7 +456,7 @@ void Animation::cancel(ShouldInvalidate should_invalidate) Optional scheduled_event_time; if (m_timeline && !m_timeline->is_inactive() && m_timeline->can_convert_a_timeline_time_to_an_origin_relative_time()) scheduled_event_time = m_timeline->convert_a_timeline_time_to_an_origin_relative_time(m_timeline->current_time()); - document->append_pending_animation_event({ cancel_event, *this, scheduled_event_time }); + document->append_pending_animation_event({ cancel_event, *this, *this, scheduled_event_time }); } else { HTML::queue_global_task(HTML::Task::Source::DOMManipulation, realm.global_object(), JS::create_heap_function(heap(), [this, cancel_event]() { dispatch_event(cancel_event); @@ -1127,9 +1127,12 @@ void Animation::update_finished_state(DidSeek did_seek, SynchronouslyNotify sync // animation event queue along with its target, animation. For the scheduled event time, use the result // of converting animation’s associated effect end to an origin-relative time. if (auto document_for_timing = this->document_for_timing()) { - document_for_timing->append_pending_animation_event({ .event = finish_event, + document_for_timing->append_pending_animation_event({ + .event = finish_event, + .animation = *this, .target = *this, - .scheduled_event_time = convert_a_timeline_time_to_an_origin_relative_time(associated_effect_end()) }); + .scheduled_event_time = convert_a_timeline_time_to_an_origin_relative_time(associated_effect_end()), + }); } // Otherwise, queue a task to dispatch finishEvent at animation. The task source for this task is the DOM // manipulation task source. diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 15bdb85c4a1..dcbe736bc09 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -2017,7 +2017,12 @@ void Document::dispatch_events_for_animation_if_necessary(JS::NonnullGCPtr(*animation); - if (auto target = effect->target(); target && target->paintable()) + + JS::GCPtr target = effect->target(); + if (!target) + return; + + if (target->paintable()) target->paintable()->set_needs_display(); auto previous_phase = effect->previous_phase(); @@ -2037,7 +2042,8 @@ void Document::dispatch_events_for_animation_if_necessary(JS::NonnullGCPtractive_time_using_fill(Bindings::FillMode::Both).value()); } } + effect->set_previous_phase(current_phase); + effect->set_previous_current_iteration(current_iteration); } // https://html.spec.whatwg.org/multipage/browsing-the-web.html#scroll-to-the-fragment-identifier @@ -4238,8 +4246,8 @@ void Document::update_animations_and_send_events(Optional const& timesta // 6. Perform a stable sort of the animation events in events to dispatch as follows: auto sort_events_by_composite_order = [](auto const& a, auto const& b) { - auto& a_effect = verify_cast(*a.target->effect()); - auto& b_effect = verify_cast(*b.target->effect()); + auto& a_effect = verify_cast(*a.animation->effect()); + auto& b_effect = verify_cast(*b.animation->effect()); return Animations::KeyframeEffect::composite_order(a_effect, b_effect) < 0; }; @@ -4352,9 +4360,10 @@ void Document::remove_replaced_animations() // timeline with which animation is associated. if (auto document = animation->document_for_timing()) { PendingAnimationEvent pending_animation_event { - remove_event, - animation, - animation->timeline()->convert_a_timeline_time_to_an_origin_relative_time(init.timeline_time), + .event = remove_event, + .animation = animation, + .target = animation, + .scheduled_event_time = animation->timeline()->convert_a_timeline_time_to_an_origin_relative_time(init.timeline_time), }; document->append_pending_animation_event(pending_animation_event); } diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index 41f0f2e9a0e..50b19e7040b 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -599,7 +599,8 @@ public: struct PendingAnimationEvent { JS::NonnullGCPtr event; - JS::NonnullGCPtr target; + JS::NonnullGCPtr animation; + JS::NonnullGCPtr target; Optional scheduled_event_time; }; void append_pending_animation_event(PendingAnimationEvent const&);