LibWeb: Store Animations in Animatable instead of AnimationEffects

This is closer to what the spec instructs us to do, and matches how
associations are maintained in the timelines. Also note that the removed
destructor logic is not necessary since we visit the associated
animations anyways.

Userland/Libraries/LibWeb/Animations/Animatable.cpp

@@ -63,9 +63,9 @@ Vector<JS::NonnullGCPtr<Animation>> Animatable::get_animations(Web::Animations::
     // The returned list is sorted using the composite order described for the associated animations of effects in
     // §5.4.2 The effect stack.
     if (!m_is_sorted_by_composite_order) {
-        quick_sort(m_associated_effects, [](JS::NonnullGCPtr<AnimationEffect>& a, JS::NonnullGCPtr<AnimationEffect>& b) {
+        quick_sort(m_associated_animations, [](JS::NonnullGCPtr<Animation>& a, JS::NonnullGCPtr<Animation>& b) {
-            auto& a_effect = verify_cast<KeyframeEffect>(*a);
+            auto& a_effect = verify_cast<KeyframeEffect>(*a->effect());
-            auto& b_effect = verify_cast<KeyframeEffect>(*b);
+            auto& b_effect = verify_cast<KeyframeEffect>(*b->effect());
             return KeyframeEffect::composite_order(a_effect, b_effect) < 0;
         });
         m_is_sorted_by_composite_order = true;
@@ -75,29 +75,29 @@ Vector<JS::NonnullGCPtr<Animation>> Animatable::get_animations(Web::Animations::
     (void)options;
 
     Vector<JS::NonnullGCPtr<Animation>> relevant_animations;
-    for (auto& effect : m_associated_effects) {
+    for (auto const& animation : m_associated_animations) {
-        if (auto animation = effect->associated_animation(); animation && animation->is_relevant())
+        if (animation->is_relevant())
             relevant_animations.append(*animation);
     }
 
     return relevant_animations;
 }
 
-void Animatable::associate_with_effect(JS::NonnullGCPtr<AnimationEffect> effect)
+void Animatable::associate_with_animation(JS::NonnullGCPtr<Animation> animation)
 {
-    m_associated_effects.append(effect);
+    m_associated_animations.append(animation);
     m_is_sorted_by_composite_order = false;
 }
 
-void Animatable::disassociate_with_effect(JS::NonnullGCPtr<AnimationEffect> effect)
+void Animatable::disassociate_with_animation(JS::NonnullGCPtr<Animation> animation)
 {
-    m_associated_effects.remove_first_matching([&](auto element) { return effect == element; });
+    m_associated_animations.remove_first_matching([&](auto element) { return animation == element; });
 }
 
 void Animatable::visit_edges(JS::Cell::Visitor& visitor)
 {
-    for (auto const& effect : m_associated_effects)
+    for (auto const& animation : m_associated_animations)
-        visitor.visit(effect);
+        visitor.visit(animation);
     visitor.visit(m_cached_animation_name_source);
     visitor.visit(m_cached_animation_name_animation);
 }

Userland/Libraries/LibWeb/Animations/Animatable.h

@@ -30,8 +30,8 @@ public:
     WebIDL::ExceptionOr<JS::NonnullGCPtr<Animation>> animate(Optional<JS::Handle<JS::Object>> keyframes, Variant<Empty, double, KeyframeAnimationOptions> options = {});
     Vector<JS::NonnullGCPtr<Animation>> get_animations(GetAnimationsOptions options = {});
 
-    void associate_with_effect(JS::NonnullGCPtr<AnimationEffect> effect);
+    void associate_with_animation(JS::NonnullGCPtr<Animation>);
-    void disassociate_with_effect(JS::NonnullGCPtr<AnimationEffect> effect);
+    void disassociate_with_animation(JS::NonnullGCPtr<Animation>);
 
     JS::GCPtr<CSS::CSSStyleDeclaration const> cached_animation_name_source() const { return m_cached_animation_name_source; }
     void set_cached_animation_name_source(JS::GCPtr<CSS::CSSStyleDeclaration const> value) { m_cached_animation_name_source = value; }
@@ -43,7 +43,7 @@ protected:
     void visit_edges(JS::Cell::Visitor&);
 
 private:
-    Vector<JS::NonnullGCPtr<AnimationEffect>> m_associated_effects;
+    Vector<JS::NonnullGCPtr<Animation>> m_associated_animations;
     bool m_is_sorted_by_composite_order { true };
     JS::GCPtr<CSS::CSSStyleDeclaration const> m_cached_animation_name_source;
     JS::GCPtr<Animations::Animation> m_cached_animation_name_animation;

Userland/Libraries/LibWeb/Animations/Animation.cpp

@@ -352,7 +352,7 @@ void Animation::set_replace_state(Bindings::AnimationReplaceState value)
     if (value == Bindings::AnimationReplaceState::Removed) {
         // Remove the associated effect from its target, if applicable
         if (m_effect && m_effect->target())
-            m_effect->target()->disassociate_with_effect(*m_effect);
+            m_effect->target()->disassociate_with_animation(*this);
 
         // Remove this animation from its timeline
         m_timeline->disassociate_with_animation(*this);

Userland/Libraries/LibWeb/Animations/KeyframeEffect.cpp

@@ -723,11 +723,13 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<KeyframeEffect>> KeyframeEffect::construct_
 
 void KeyframeEffect::set_target(DOM::Element* target)
 {
+    if (auto animation = this->associated_animation()) {
         if (m_target_element)
-        m_target_element->disassociate_with_effect(*this);
+            m_target_element->disassociate_with_animation(*animation);
+        if (target)
+            target->associate_with_animation(*animation);
+    }
     m_target_element = target;
-    if (m_target_element)
-        m_target_element->associate_with_effect(*this);
 }
 
 WebIDL::ExceptionOr<void> KeyframeEffect::set_pseudo_element(Optional<String> pseudo_element)
@@ -853,12 +855,6 @@ KeyframeEffect::KeyframeEffect(JS::Realm& realm)
 {
 }
 
-KeyframeEffect::~KeyframeEffect()
-{
-    if (m_target_element)
-        m_target_element->disassociate_with_effect(*this);
-}
-
 void KeyframeEffect::initialize(JS::Realm& realm)
 {
     Base::initialize(realm);

Userland/Libraries/LibWeb/Animations/KeyframeEffect.h

@@ -105,7 +105,7 @@ public:
 
 private:
     KeyframeEffect(JS::Realm&);
-    virtual ~KeyframeEffect() override;
+    virtual ~KeyframeEffect() override = default;
 
     virtual void initialize(JS::Realm&) override;
     virtual void visit_edges(Cell::Visitor&) override;