Title: [259538] trunk/Source/WebCore
- Revision
- 259538
- Author
- [email protected]
- Date
- 2020-04-04 15:39:18 -0700 (Sat, 04 Apr 2020)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (259537 => 259538)
--- trunk/Source/WebCore/ChangeLog 2020-04-04 21:42:23 UTC (rev 259537)
+++ trunk/Source/WebCore/ChangeLog 2020-04-04 22:39:18 UTC (rev 259538)
@@ -1,3 +1,22 @@
+2020-04-04 Doug Kelly <[email protected]>
+
+ 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 <[email protected]>
Add even more logging to try and diagnose <webkit.org/b/209685>
Modified: trunk/Source/WebCore/animation/WebAnimationUtilities.cpp (259537 => 259538)
--- trunk/Source/WebCore/animation/WebAnimationUtilities.cpp 2020-04-04 21:42:23 UTC (rev 259537)
+++ trunk/Source/WebCore/animation/WebAnimationUtilities.cpp 2020-04-04 22:39:18 UTC (rev 259538)
@@ -44,16 +44,19 @@
bool lhsIsCSSTransition = lhsHasOwningElement && is<CSSTransition>(lhsAnimation);
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;
}
// CSS Animations sort next.
@@ -60,28 +63,28 @@
bool lhsIsCSSAnimation = lhsHasOwningElement && is<CSSAnimation>(lhsAnimation);
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();
}
// JS-originated animations sort last based on their position in the global animation list.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
