DumpRenderTree crashes under WebAnimation::isRelevant when running imported/mozilla...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 00:14:04 +0000 (00:14 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 00:14:04 +0000 (00:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196028
<rdar://problem/46842707>

Reviewed by Dean Jackson.

Instead of keeping a ListHashSet of raw pointers, we are now using a Vector of WeakPtrs.

* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::forgetAnimation):
(WebCore::AnimationTimeline::animationTimingDidChange):
(WebCore::AnimationTimeline::cancelDeclarativeAnimation):
* animation/AnimationTimeline.h:
* animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::getAnimations const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243263 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/animation/AnimationTimeline.cpp
Source/WebCore/animation/AnimationTimeline.h
Source/WebCore/animation/DocumentTimeline.cpp

index b68f6b5..c0af9fd 100644 (file)
@@ -1,3 +1,21 @@
+2019-03-20  Antoine Quint  <graouts@apple.com>
+
+        DumpRenderTree crashes under WebAnimation::isRelevant when running imported/mozilla/css-transitions/test_document-get-animations.html in GuardMalloc
+        https://bugs.webkit.org/show_bug.cgi?id=196028
+        <rdar://problem/46842707>
+
+        Reviewed by Dean Jackson.
+
+        Instead of keeping a ListHashSet of raw pointers, we are now using a Vector of WeakPtrs.
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::forgetAnimation):
+        (WebCore::AnimationTimeline::animationTimingDidChange):
+        (WebCore::AnimationTimeline::cancelDeclarativeAnimation):
+        * animation/AnimationTimeline.h:
+        * animation/DocumentTimeline.cpp:
+        (WebCore::DocumentTimeline::getAnimations const):
+
 2019-03-20  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Remove the SVG tear off objects for SVGColorAnimator
index faad34c..5612926 100644 (file)
@@ -56,13 +56,13 @@ AnimationTimeline::~AnimationTimeline()
 
 void AnimationTimeline::forgetAnimation(WebAnimation* animation)
 {
-    m_allAnimations.remove(animation);
+    m_allAnimations.removeFirst(animation);
 }
 
 void AnimationTimeline::animationTimingDidChange(WebAnimation& animation)
 {
     if (m_animations.add(&animation)) {
-        m_allAnimations.add(&animation);
+        m_allAnimations.append(makeWeakPtr(&animation));
         auto* timeline = animation.timeline();
         if (timeline && timeline != this)
             timeline->removeAnimation(animation);
@@ -492,7 +492,7 @@ void AnimationTimeline::cancelDeclarativeAnimation(DeclarativeAnimation& animati
 {
     animation.cancelFromStyle();
     removeAnimation(animation);
-    m_allAnimations.remove(&animation);
+    m_allAnimations.removeFirst(&animation);
 }
 
 } // namespace WebCore
index 0f318bf..12d1ece 100644 (file)
@@ -77,7 +77,7 @@ public:
 protected:
     explicit AnimationTimeline();
 
-    ListHashSet<WebAnimation*> m_allAnimations;
+    Vector<WeakPtr<WebAnimation>> m_allAnimations;
     ListHashSet<RefPtr<WebAnimation>> m_animations;
     HashMap<Element*, PropertyToTransitionMap> m_elementToCompletedCSSTransitionByCSSPropertyID;
 
index 3018cbb..7e2f1c4 100644 (file)
@@ -132,19 +132,19 @@ Vector<RefPtr<WebAnimation>> DocumentTimeline::getAnimations() const
 
     // First, let's get all qualifying animations in their right group.
     for (const auto& animation : m_allAnimations) {
-        if (!animation->isRelevant() || animation->timeline() != this || !is<KeyframeEffect>(animation->effect()))
+        if (!animation || !animation->isRelevant() || animation->timeline() != this || !is<KeyframeEffect>(animation->effect()))
             continue;
 
         auto* target = downcast<KeyframeEffect>(animation->effect())->target();
         if (!target || !target->isDescendantOf(*m_document))
             continue;
 
-        if (is<CSSTransition>(animation) && downcast<CSSTransition>(animation)->owningElement())
-            cssTransitions.append(animation);
-        else if (is<CSSAnimation>(animation) && downcast<CSSAnimation>(animation)->owningElement())
-            cssAnimations.append(animation);
+        if (is<CSSTransition>(animation.get()) && downcast<CSSTransition>(animation.get())->owningElement())
+            cssTransitions.append(animation.get());
+        else if (is<CSSAnimation>(animation.get()) && downcast<CSSAnimation>(animation.get())->owningElement())
+            cssAnimations.append(animation.get());
         else
-            webAnimations.append(animation);
+            webAnimations.append(animation.get());
     }
 
     // Now sort CSS Transitions by their composite order.