Implement composite order with animate preempting css

In this patch, we replace the current implementation of sorting effect
order by comparing effect sequence number with comparing the
HasLowerCompositeOrdering function.

The patch will align the implementation with the spec
(https://www.w3.org/TR/web-animations-1/#the-effect-stack). And as we
are using the same comparison function as the getAnimations, the
rendered result would be the same as getAnimations() result.

Bug: 1050224

Change-Id: I88f151fa78070817d2fa4fd520580ddf6d024e27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2076160
Commit-Queue: Hao Sheng <haozhes@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748492}

third_party/blink/renderer/core/animation/animation.cc

@@ -108,7 +108,7 @@ PseudoPriority ConvertStringtoPriority(const String& pseudo) {
 }
 
 Animation::AnimationClassPriority AnimationPriority(
-    const Animation* animation) {
+    const Animation& animation) {
   // According to the spec:
   // https://drafts.csswg.org/web-animations/#animation-class,
   // CSS tranisiton has a lower composite order than the CSS animation, and CSS
@@ -120,9 +120,9 @@ Animation::AnimationClassPriority AnimationPriority(
   // https://drafts.csswg.org/css-animations-2/#animation-composite-order and
   // https://drafts.csswg.org/css-transitions-2/#animation-composite-order
   Animation::AnimationClassPriority priority;
-  if (animation->IsCSSTransition())
+  if (animation.IsCSSTransition())
     priority = Animation::AnimationClassPriority::kCssTransitionPriority;
-  else if (animation->IsCSSAnimation())
+  else if (animation.IsCSSAnimation())
     priority = Animation::AnimationClassPriority::kCssAnimationPriority;
   else
     priority = Animation::AnimationClassPriority::kDefaultPriority;
@@ -462,8 +462,8 @@ bool Animation::HasLowerCompositeOrdering(
     const Animation* animation1,
     const Animation* animation2,
     CompareAnimationsOrdering compare_animation_type) {
-  AnimationClassPriority priority1 = AnimationPriority(animation1);
-  AnimationClassPriority priority2 = AnimationPriority(animation2);
+  AnimationClassPriority priority1 = AnimationPriority(*animation1);
+  AnimationClassPriority priority2 = AnimationPriority(*animation2);
   if (priority1 != priority2)
     return priority1 < priority2;
 

third_party/blink/renderer/core/animation/effect_stack.cc

@@ -71,11 +71,6 @@ void CopyToActiveInterpolationsMap(
   }
 }
 
-bool CompareSampledEffects(const Member<SampledEffect>& sampled_effect1,
-                           const Member<SampledEffect>& sampled_effect2) {
-  DCHECK(sampled_effect1 && sampled_effect2);
-  return sampled_effect1->SequenceNumber() < sampled_effect2->SequenceNumber();
-}
 
 void CopyNewAnimationsToActiveInterpolationsMap(
     const HeapVector<Member<const InertEffect>>& new_animations,
@@ -91,6 +86,21 @@ void CopyNewAnimationsToActiveInterpolationsMap(
 
 }  // namespace
 
+bool EffectStack::CompareSampledEffects(
+    const Member<SampledEffect>& sampled_effect1,
+    const Member<SampledEffect>& sampled_effect2) {
+  if (sampled_effect1->Effect() && sampled_effect2->Effect()) {
+    Animation* animation1 = sampled_effect1->Effect()->GetAnimation();
+    Animation* animation2 = sampled_effect2->Effect()->GetAnimation();
+    if (animation1 && animation2) {
+      return Animation::HasLowerCompositeOrdering(
+          animation1, animation2,
+          Animation::CompareAnimationsOrdering::kPointerOrder);
+    }
+  }
+  return sampled_effect1->SequenceNumber() < sampled_effect2->SequenceNumber();
+}
+
 EffectStack::EffectStack() = default;
 
 bool EffectStack::HasActiveAnimationsOnCompositor(
@@ -126,9 +136,9 @@ ActiveInterpolationsMap EffectStack::ActiveInterpolations(
   if (effect_stack) {
     HeapVector<Member<SampledEffect>>& sampled_effects =
         effect_stack->sampled_effects_;
+    effect_stack->RemoveRedundantSampledEffects();
     std::sort(sampled_effects.begin(), sampled_effects.end(),
               CompareSampledEffects);
-    effect_stack->RemoveRedundantSampledEffects();
     bool reached_cuttoff = false;
     for (const auto& sampled_effect : sampled_effects) {
       if (reached_cuttoff)

third_party/blink/renderer/core/animation/effect_stack.h

@@ -57,6 +57,8 @@ class CORE_EXPORT EffectStack {
   void Add(SampledEffect* sampled_effect) {
     sampled_effects_.push_back(sampled_effect);
   }
+  static bool CompareSampledEffects(const Member<SampledEffect>&,
+                                    const Member<SampledEffect>&);
   bool IsEmpty() const { return sampled_effects_.IsEmpty(); }
   bool HasActiveAnimationsOnCompositor(const PropertyHandle&) const;
 

third_party/blink/web_tests/external/wpt/css/css-animations/CSSAnimation-compositeOrder.tentative-expected.txt

 This is a testharness.js-based test.
 FAIL Animations are composited by their order in the animation-name property. assert_equals: expected "100px" but got "50px"
+PASS Web-animation replaces CSS animation
 Harness: the test ran to completion.
 

third_party/blink/web_tests/external/wpt/css/css-animations/CSSAnimation-compositeOrder.tentative.html

@@ -36,4 +36,18 @@ promise_test(async t => {
   // in the animation list.
   assert_equals(getComputedStyle(div).marginLeft, '100px');
 }, "Animations are composited by their order in the animation-name property.");
+
+promise_test(async t => {
+  const div = addDiv(t);
+  const animA = div.animate({margin: ["100px","100px"]}, 100000);
+  assert_equals(getComputedStyle(div).marginLeft, '100px');
+  div.style.animation = 'margin50 100s';
+  assert_equals(getComputedStyle(div).marginLeft, '50px');
+  // Wait for animation starts
+  await waitForAnimationFrames(2);
+  assert_equals(getComputedStyle(div).marginLeft, '100px',
+                "A higher-priority animation is not overriden by a more recent"
+                + "one.");
+}, 'Web-animation replaces CSS animation');
+
 </script>