Changeset 243263 in webkit

Timestamp:

Mar 20, 2019, 5:14:04 PM (7 years ago)

Author:

graouts@webkit.org

Message:

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• animation/AnimationTimeline.cpp (modified) (3 diffs)
• animation/AnimationTimeline.h (modified) (1 diff)
• animation/DocumentTimeline.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/animation/AnimationTimeline.cpp

@@ -57 +57 @@
 void AnimationTimeline::forgetAnimation(WebAnimation* animation)
 {
-    m_allAnimations.remove(animation);
+    m_allAnimations.removeFirst(animation);
 }
 
@@ -63 +63 @@
 {
     if (m_animations.add(&animation)) {
-        m_allAnimations.add(&animation);
+        m_allAnimations.append(makeWeakPtr(&animation));
         auto* timeline = animation.timeline();
         if (timeline && timeline != this)
@@ -493 +493 @@
     animation.cancelFromStyle();
     removeAnimation(animation);
-    m_allAnimations.remove(&animation);
+    m_allAnimations.removeFirst(&animation);
 }
 

trunk/Source/WebCore/animation/AnimationTimeline.h

@@ -78 +78 @@
     explicit AnimationTimeline();
 
-    ListHashSet<WebAnimation*> m_allAnimations;
+    Vector<WeakPtr<WebAnimation>> m_allAnimations;
     ListHashSet<RefPtr<WebAnimation>> m_animations;
     HashMap<Element*, PropertyToTransitionMap> m_elementToCompletedCSSTransitionByCSSPropertyID;

trunk/Source/WebCore/animation/DocumentTimeline.cpp

@@ -133 +133 @@
     // 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;
 
@@ -140 +140 @@
             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());
     }