From a0b96280e498d53576ec855d0deea56f508c9e7c Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Thu, 12 Sep 2024 16:53:14 +0100 Subject: [PATCH] LibWeb: Move "owning element" of Animation classes into Animation There's no need to have a virtual method here when we can just store the owning element pointer on the Animation instead. --- Userland/Libraries/LibWeb/Animations/Animation.cpp | 1 + Userland/Libraries/LibWeb/Animations/Animation.h | 6 +++++- Userland/Libraries/LibWeb/CSS/CSSAnimation.cpp | 14 ++++---------- Userland/Libraries/LibWeb/CSS/CSSAnimation.h | 7 ------- Userland/Libraries/LibWeb/CSS/CSSTransition.cpp | 9 ++++----- Userland/Libraries/LibWeb/CSS/CSSTransition.h | 6 ------ 6 files changed, 14 insertions(+), 29 deletions(-) diff --git a/Userland/Libraries/LibWeb/Animations/Animation.cpp b/Userland/Libraries/LibWeb/Animations/Animation.cpp index f7e86073973..d218c5c40b4 100644 --- a/Userland/Libraries/LibWeb/Animations/Animation.cpp +++ b/Userland/Libraries/LibWeb/Animations/Animation.cpp @@ -1358,6 +1358,7 @@ void Animation::visit_edges(Cell::Visitor& visitor) visitor.visit(m_timeline); visitor.visit(m_current_ready_promise); visitor.visit(m_current_finished_promise); + visitor.visit(m_owning_element); } } diff --git a/Userland/Libraries/LibWeb/Animations/Animation.h b/Userland/Libraries/LibWeb/Animations/Animation.h index 134346e0cab..41be019afc1 100644 --- a/Userland/Libraries/LibWeb/Animations/Animation.h +++ b/Userland/Libraries/LibWeb/Animations/Animation.h @@ -101,7 +101,8 @@ public: virtual bool is_css_animation() const { return false; } virtual bool is_css_transition() const { return false; } - virtual JS::GCPtr owning_element() const { return {}; } + JS::GCPtr owning_element() const { return m_owning_element; } + void set_owning_element(JS::GCPtr value) { m_owning_element = value; } virtual AnimationClass animation_class() const { return AnimationClass::None; } virtual Optional class_specific_composite_order(JS::NonnullGCPtr) const { return {}; } @@ -192,6 +193,9 @@ private: // https://www.w3.org/TR/web-animations-1/#pending-pause-task TaskState m_pending_pause_task { TaskState::None }; + // https://www.w3.org/TR/css-animations-2/#owning-element-section + JS::GCPtr m_owning_element; + Optional m_pending_finish_microtask_id; Optional m_saved_play_time; diff --git a/Userland/Libraries/LibWeb/CSS/CSSAnimation.cpp b/Userland/Libraries/LibWeb/CSS/CSSAnimation.cpp index 88dd049d7c3..0ec4ab3805d 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSAnimation.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSAnimation.cpp @@ -26,11 +26,11 @@ Optional CSSAnimation::class_specific_composite_order(JS::NonnullGCPtrm_owning_element); + VERIFY(!owning_element() == !other->owning_element()); // Within the set of CSS Animations with an owning element, two animations A and B are sorted in composite order // (first to last) as follows: - if (m_owning_element) { + if (owning_element()) { // 1. If the owning element of A and B differs, sort A and B by tree order of their corresponding owning elements. // With regard to pseudo-elements, the sort order is as follows: // - element @@ -40,7 +40,7 @@ Optional CSSAnimation::class_specific_composite_order(JS::NonnullGCPtrm_owning_element.ptr()) { + if (owning_element().ptr() != other->owning_element().ptr()) { // FIXME: Sort by tree order return {}; } @@ -57,7 +57,7 @@ Optional CSSAnimation::class_specific_composite_order(JS::NonnullGCPtr create(JS::Realm&); - JS::GCPtr owning_element() const override { return m_owning_element; } - void set_owning_element(JS::GCPtr value) { m_owning_element = value; } - FlyString const& animation_name() const { return id(); } virtual Animations::AnimationClass animation_class() const override; @@ -32,12 +29,8 @@ private: explicit CSSAnimation(JS::Realm&); virtual void initialize(JS::Realm&) override; - virtual void visit_edges(Cell::Visitor&) override; virtual bool is_css_animation() const override { return true; } - - // https://www.w3.org/TR/css-animations-2/#owning-element-section - JS::GCPtr m_owning_element; }; } diff --git a/Userland/Libraries/LibWeb/CSS/CSSTransition.cpp b/Userland/Libraries/LibWeb/CSS/CSSTransition.cpp index 7d964288a59..8a913915b0f 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSTransition.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSTransition.cpp @@ -32,13 +32,13 @@ Optional CSSTransition::class_specific_composite_order(JS::NonnullGCPtrm_owning_element) + if (!owning_element() && !other->owning_element()) return global_animation_list_order() - other->global_animation_list_order(); // 2. Otherwise, if only one of A or B has an owning element, let the animation with an owning element sort first. - if (m_owning_element && !other->m_owning_element) + if (owning_element() && !other->owning_element()) return -1; - if (!m_owning_element && other->m_owning_element) + if (!owning_element() && other->owning_element()) return 1; // 3. Otherwise, if the owning element of A and B differs, sort A and B by tree order of their corresponding owning @@ -50,7 +50,7 @@ Optional CSSTransition::class_specific_composite_order(JS::NonnullGCPtrm_owning_element.ptr()) { + if (owning_element().ptr() != other->owning_element().ptr()) { // FIXME: Actually sort by tree order return {}; } @@ -88,7 +88,6 @@ void CSSTransition::initialize(JS::Realm& realm) void CSSTransition::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_owning_element); visitor.visit(m_cached_declaration); } diff --git a/Userland/Libraries/LibWeb/CSS/CSSTransition.h b/Userland/Libraries/LibWeb/CSS/CSSTransition.h index 4aac633c3b7..8eac75a3c8a 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSTransition.h +++ b/Userland/Libraries/LibWeb/CSS/CSSTransition.h @@ -22,9 +22,6 @@ public: StringView transition_property() const { return string_from_property_id(m_transition_property); } - JS::GCPtr owning_element() const override { return m_owning_element; } - void set_owning_element(JS::GCPtr value) { m_owning_element = value; } - JS::GCPtr cached_declaration() const { return m_cached_declaration; } void set_cached_declaration(JS::GCPtr declaration) { m_cached_declaration = declaration; } @@ -44,9 +41,6 @@ private: // https://drafts.csswg.org/css-transitions-2/#transition-generation size_t m_transition_generation; - // https://drafts.csswg.org/css-transitions-2/#owning-element - JS::GCPtr m_owning_element; - JS::GCPtr m_cached_declaration; };