Changeset 259538 in webkit

Timestamp:

Apr 4, 2020, 3:39:18 PM (6 years ago)

Author:

Doug Kelly

Message:

Additional sanity checks in compareAnimationsByCompositeOrder()
​https://bugs.webkit.org/show_bug.cgi?id=209996

Reviewed by Geoffrey Garen.

compareAnimationsByCompositeOrder() is used by std::sort() which requires strict weak ordering.
This adds additional checks to ensure strict weak ordering is maintained, first by ensuring
the transitionProperty string is different before returning that comparison, then by only using
if the animation is a CSSTransition or CSSAnimation if the left hand and right hand sides differ.
This should leave all remaining cases to sort by the global animation list.

No new tests; this should be covered by existing tests and should not change functionality
otherwise.

• animation/WebAnimationUtilities.cpp:

(WebCore::compareAnimationsByCompositeOrder):

Location:

trunk/Source/WebCore

Files:

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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-04-04  Doug Kelly  <dougk@apple.com>
+
+        Additional sanity checks in compareAnimationsByCompositeOrder()
+        https://bugs.webkit.org/show_bug.cgi?id=209996
+
+        Reviewed by Geoffrey Garen.
+
+        compareAnimationsByCompositeOrder() is used by std::sort() which requires strict weak ordering.
+        This adds additional checks to ensure strict weak ordering is maintained, first by ensuring
+        the transitionProperty string is different before returning that comparison, then by only using
+        if the animation is a CSSTransition or CSSAnimation if the left hand and right hand sides differ.
+        This should leave all remaining cases to sort by the global animation list.
+
+        No new tests; this should be covered by existing tests and should not change functionality
+        otherwise.
+
+        * animation/WebAnimationUtilities.cpp:
+        (WebCore::compareAnimationsByCompositeOrder):
+
 2020-04-04  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/animation/WebAnimationUtilities.cpp

@@ -45 +45 @@
     bool rhsIsCSSTransition = rhsHasOwningElement && is<CSSTransition>(rhsAnimation);
     if (lhsIsCSSTransition || rhsIsCSSTransition) {
-        if (lhsIsCSSTransition == rhsIsCSSTransition) {
-            // Sort transitions first by their generation time, and then by transition-property.
-            // https://drafts.csswg.org/css-transitions-2/#animation-composite-order
-            auto& lhsCSSTransition = downcast<CSSTransition>(lhsAnimation);
-            auto& rhsCSSTransition = downcast<CSSTransition>(rhsAnimation);
-            if (lhsCSSTransition.generationTime() != rhsCSSTransition.generationTime())
-                return lhsCSSTransition.generationTime() < rhsCSSTransition.generationTime();
-            return lhsCSSTransition.transitionProperty().utf8() < rhsCSSTransition.transitionProperty().utf8();
-        }
-        return !rhsIsCSSTransition;
+        if (lhsIsCSSTransition != rhsIsCSSTransition)
+            return !rhsIsCSSTransition;
+
+        // Sort transitions first by their generation time, and then by transition-property.
+        // https://drafts.csswg.org/css-transitions-2/#animation-composite-order
+        auto& lhsCSSTransition = downcast<CSSTransition>(lhsAnimation);
+        auto& rhsCSSTransition = downcast<CSSTransition>(rhsAnimation);
+        if (lhsCSSTransition.generationTime() != rhsCSSTransition.generationTime())
+            return lhsCSSTransition.generationTime() < rhsCSSTransition.generationTime();
+        auto lhsCSSTransitionProperty = lhsCSSTransition.transitionProperty().utf8();
+        auto rhsCSSTransitionProperty = rhsCSSTransition.transitionProperty().utf8();
+        if (lhsCSSTransitionProperty != rhsCSSTransitionProperty)
+            return lhsCSSTransitionProperty < rhsCSSTransitionProperty;
     }
 
@@ -61 +64 @@
     bool rhsIsCSSAnimation = rhsHasOwningElement && is<CSSAnimation>(rhsAnimation);
     if (lhsIsCSSAnimation || rhsIsCSSAnimation) {
-        if (lhsIsCSSAnimation == rhsIsCSSAnimation) {
-            // We must have a list of CSS Animations if we have CSS Animations to sort through.
-            ASSERT(cssAnimationList);
-            ASSERT(!cssAnimationList->isEmpty());
+        if (lhsIsCSSAnimation != rhsIsCSSAnimation)
+            return !rhsIsCSSAnimation;
 
-            // https://drafts.csswg.org/css-animations-2/#animation-composite-order
-            // Sort A and B based on their position in the computed value of the animation-name property of the (common) owning element.
-            auto& lhsBackingAnimation = downcast<CSSAnimation>(lhsAnimation).backingAnimation();
-            auto& rhsBackingAnimation = downcast<CSSAnimation>(rhsAnimation).backingAnimation();
+        // We must have a list of CSS Animations if we have CSS Animations to sort through.
+        ASSERT(cssAnimationList);
+        ASSERT(!cssAnimationList->isEmpty());
 
-            for (size_t i = 0; i < cssAnimationList->size(); ++i) {
-                auto& animation = cssAnimationList->animation(i);
-                if (animation == lhsBackingAnimation)
-                    return true;
-                if (animation == rhsBackingAnimation)
-                    return false;
-            }
+        // https://drafts.csswg.org/css-animations-2/#animation-composite-order
+        // Sort A and B based on their position in the computed value of the animation-name property of the (common) owning element.
+        auto& lhsBackingAnimation = downcast<CSSAnimation>(lhsAnimation).backingAnimation();
+        auto& rhsBackingAnimation = downcast<CSSAnimation>(rhsAnimation).backingAnimation();
 
-            // We should have found either of those CSS animations in the CSS animations list.
-            ASSERT_NOT_REACHED();
+        for (size_t i = 0; i < cssAnimationList->size(); ++i) {
+            auto& animation = cssAnimationList->animation(i);
+            if (animation == lhsBackingAnimation)
+                return true;
+            if (animation == rhsBackingAnimation)
+                return false;
         }
-        return !rhsIsCSSAnimation;
+
+        // We should have found either of those CSS animations in the CSS animations list.
+        ASSERT_NOT_REACHED();
     }