Changeset 259630 in webkit

Timestamp:

Apr 7, 2020 1:25:59 AM (4 years ago)

Author:

Doug Kelly

Message:

Add release asserts to KeyframeEffectStack::ensureEffectsAreSorted()
​https://bugs.webkit.org/show_bug.cgi?id=210084
<rdar://problem/61359275>

Reviewed by Ryosuke Niwa.

To ensure any potential problems in KeyframeEffectStack::ensureEffectsAreSorted() are found closer to the
root cause, add several RELEASE_ASSERTs throughout this function (and its associated comparison function).
This should guard against null pointers/null WeakPtrs, as well as other state problems which would be
unexpected for the comparison function used by std::sort.

No new tests; this only adds additional asserts, so there is no change to functionality, and this code is
covered by existing tests.

• animation/KeyframeEffectStack.cpp:

(WebCore::KeyframeEffectStack::ensureEffectsAreSorted):

• animation/WebAnimationUtilities.cpp:

(WebCore::compareAnimationsByCompositeOrder):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• animation/KeyframeEffectStack.cpp (modified) (1 diff)
• animation/WebAnimationUtilities.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-04-07  Doug Kelly  <dougk@apple.com>
+
+        Add release asserts to KeyframeEffectStack::ensureEffectsAreSorted()
+        https://bugs.webkit.org/show_bug.cgi?id=210084
+        <rdar://problem/61359275>
+
+        Reviewed by Ryosuke Niwa.
+
+        To ensure any potential problems in KeyframeEffectStack::ensureEffectsAreSorted() are found closer to the
+        root cause, add several RELEASE_ASSERTs throughout this function (and its associated comparison function).
+        This should guard against null pointers/null WeakPtrs, as well as other state problems which would be
+        unexpected for the comparison function used by std::sort.
+
+        No new tests; this only adds additional asserts, so there is no change to functionality, and this code is
+        covered by existing tests.
+
+        * animation/KeyframeEffectStack.cpp:
+        (WebCore::KeyframeEffectStack::ensureEffectsAreSorted):
+        * animation/WebAnimationUtilities.cpp:
+        (WebCore::compareAnimationsByCompositeOrder):
+
 2020-04-07  Rob Buis  <rbuis@igalia.com>
 

trunk/Source/WebCore/animation/KeyframeEffectStack.cpp

@@ -81 +81 @@
 
     std::sort(m_effects.begin(), m_effects.end(), [&](auto& lhs, auto& rhs) {
+        RELEASE_ASSERT(lhs.get());
+        RELEASE_ASSERT(rhs.get());
+        
         auto* lhsAnimation = lhs->animation();
         auto* rhsAnimation = rhs->animation();
 
-        ASSERT(lhsAnimation);
-        ASSERT(rhsAnimation);
+        RELEASE_ASSERT(lhsAnimation);
+        RELEASE_ASSERT(rhsAnimation);
 
         return compareAnimationsByCompositeOrder(*lhsAnimation, *rhsAnimation, m_cssAnimationList.get());

trunk/Source/WebCore/animation/WebAnimationUtilities.cpp

@@ -85 +85 @@
 
         // We should have found either of those CSS animations in the CSS animations list.
-        ASSERT_NOT_REACHED();
+        RELEASE_ASSERT_NOT_REACHED();
     }
 
     // JS-originated animations sort last based on their position in the global animation list.
     // https://drafts.csswg.org/web-animations-1/#animation-composite-order
+    RELEASE_ASSERT(lhsAnimation.globalPosition() != rhsAnimation.globalPosition() || &lhsAnimation == &rhsAnimation);
     return lhsAnimation.globalPosition() < rhsAnimation.globalPosition();
 }