getAnimations cleanup with owning_element

This patch uses owning_element of animations to sorting animations in
getAnimations(), aligning the implementation with the spec:
https://drafts.csswg.org/css-transitions-2/#animation-composite-order


Bug: 1070460
Change-Id: I1bc627ffecc8450e1f2ad1cff0fefc12ac3971da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2139359Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Commit-Queue: Hao Sheng <haozhes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763473}

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

@@ -100,14 +100,14 @@ double Min(base::Optional<double> a, double b) {
   return b;
 }
 
-PseudoPriority ConvertStringtoPriority(const String& pseudo) {
-  if (pseudo.IsNull())
+PseudoPriority ConvertPseudoIdtoPriority(const PseudoId& pseudo) {
+  if (pseudo == kPseudoIdNone)
     return PseudoPriority::kNone;
-  if (pseudo == "::marker")
+  if (pseudo == kPseudoIdMarker)
     return PseudoPriority::kMarker;
-  if (pseudo == "::before")
+  if (pseudo == kPseudoIdBefore)
     return PseudoPriority::kBefore;
-  if (pseudo == "::after")
+  if (pseudo == kPseudoIdAfter)
     return PseudoPriority::kAfter;
   return PseudoPriority::kOther;
 }
@@ -159,6 +159,14 @@ void RecordCompositorAnimationFailureReasons(
     }
   }
 }
+
+Element* OriginatingElement(Element* owning_element) {
+  if (owning_element->IsPseudoElement()) {
+    return owning_element->parentElement();
+  }
+  return owning_element;
+}
+
 }  // namespace
 
 Animation* Animation::Create(AnimationEffect* effect,
@@ -472,44 +480,53 @@ bool Animation::HasLowerCompositeOrdering(
   // The specs:
   // https://drafts.csswg.org/css-animations-2/#animation-composite-order
   // https://drafts.csswg.org/css-transitions-2/#animation-composite-order
-  if (anim_priority1 != kDefaultPriority && animation1->effect() &&
-      animation2->effect()) {
-    // TODO(crbug.com/1043778): Implement and use OwningElement on CSSAnimation
-    // and CSSTransition.
-    auto* effect1 = DynamicTo<KeyframeEffect>(animation1->effect());
-    auto* effect2 = DynamicTo<KeyframeEffect>(animation2->effect());
-    Element* target1 = effect1->target();
-    Element* target2 = effect2->target();
+  if (anim_priority1 != kDefaultPriority) {
+    Element* owning_element1 = animation1->OwningElement();
+    Element* owning_element2 = animation2->OwningElement();
+
+    // Both animations are either CSS transitions or CSS animations with owning
+    // elements.
+    DCHECK(owning_element1 && owning_element2);
+    Element* originating_element1 = OriginatingElement(owning_element1);
+    Element* originating_element2 = OriginatingElement(owning_element2);
 
     // The tree position comparison would take a longer time, thus affect the
     // performance. We only do it when it comes to getAnimation.
-    if (*target1 != *target2) {
+    if (originating_element1 != originating_element2) {
       if (compare_animation_type == CompareAnimationsOrdering::kTreeOrder) {
-        return target1->compareDocumentPosition(target2) &
+        // Since pseudo elements are compared by their originating element,
+        // they sort before their children.
+        return originating_element1->compareDocumentPosition(
+                   originating_element2) &
                Node::kDocumentPositionFollowing;
       } else {
-        return target1 < target2;
+        return originating_element1 < originating_element2;
       }
     }
 
     // A pseudo-element has a higher composite ordering than its originating
-    // element, but lower than the originating element's children.
+    // element, hence kPseudoIdNone is sorted earliest.
     // Two pseudo-elements sharing the same originating element are sorted
     // as follows:
     // ::marker
     // ::before
     // other pseudo-elements (ordered by selector)
     // ::after
-    const String& pseudo1 = effect1->pseudoElement();
-    const String& pseudo2 = effect2->pseudoElement();
-    PseudoPriority priority1 = ConvertStringtoPriority(pseudo1);
-    PseudoPriority priority2 = ConvertStringtoPriority(pseudo2);
+    const PseudoId pseudo1 = owning_element1->GetPseudoId();
+    const PseudoId pseudo2 = owning_element2->GetPseudoId();
+    PseudoPriority priority1 = ConvertPseudoIdtoPriority(pseudo1);
+    PseudoPriority priority2 = ConvertPseudoIdtoPriority(pseudo2);
 
     if (priority1 != priority2)
       return priority1 < priority2;
-    if (priority1 == kOther && pseudo1 != pseudo2)
-      return CodeUnitCompareLessThan(pseudo1, pseudo2);
 
+    // The following if statement is not reachable, but the implementation
+    // matches the specification for composite ordering
+    if (priority1 == kOther && pseudo1 != pseudo2) {
+      return CodeUnitCompareLessThan(
+          PseudoElement::PseudoElementNameForEvents(pseudo1),
+          PseudoElement::PseudoElementNameForEvents(pseudo2));
+    }
     if (anim_priority1 == kCssAnimationPriority) {
       // When comparing two CSSAnimations with the same owning element, we sort
       // A and B based on their position in the computed value of the

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

@@ -22,7 +22,7 @@ CSSAnimation::CSSAnimation(ExecutionContext* execution_context,
   // The owning_element does not always equal to the target element of an
   // animation. The following spec gives an example:
   // https://drafts.csswg.org/css-animations-2/#owning-element-section
-  owning_element_ = To<KeyframeEffect>(effect())->target();
+  owning_element_ = To<KeyframeEffect>(effect())->EffectTarget();
 }
 
 String CSSAnimation::playState() const {

third_party/blink/web_tests/external/wpt/css/css-animations/Document-getAnimations.tentative-expected.txt

@@ -14,6 +14,7 @@ PASS Finished but not filling CSS Animations are not returned
 PASS Yet-to-start CSS Animations are returned
 PASS CSS Animations canceled via the API are not returned
 PASS CSS Animations canceled and restarted via the API are returned
+PASS pseudo element with replaced target does not affect animation ordering
 PASS CSS Animations targetting (pseudo-)elements should have correct order after sorting
 PASS CSS Animations targetting (pseudo-)elements should have correct order after sorting (::marker)
 Harness: the test ran to completion.

third_party/blink/web_tests/external/wpt/css/css-animations/Document-getAnimations.tentative.html

@@ -18,6 +18,20 @@
 @keyframes animRight {
   to { right: 100px }
 }
+
+@keyframes anim1 {
+  to { left: 100px }
+}
+@keyframes anim2 {
+  to { top: 100px }
+}
+@keyframes anim3 {
+  to { bottom: 100px }
+}
+@keyframes anim4 {
+  to { right: 100px }
+}
+
 </style>
 <div id="log"></div>
 <script>
@@ -285,6 +299,84 @@ test(t => {
                 'returned');
 }, 'CSS Animations canceled and restarted via the API are returned');
 
+test(t => {
+  addStyle(t, {
+      '#parent::after': "content: ''; animation: anim1 100s ; ",
+      '#parent::before': "content: ''; animation: anim2 100s;",
+      '#child::after': "content: ''; animation: anim3 100s;",
+      '#child::before': "content: ''; animation: anim4 100s;",
+    });
+  const parent = addDiv(t, { id: 'parent' });
+  const child = addDiv(t, { id: 'child'});
+  parent.appendChild(child);
+  var animations = document.getAnimations();
+  var expectedAnimations = [
+      [parent, '::before', 'anim2'],
+      [parent, '::after', 'anim1'],
+      [child, '::before', 'anim4'],
+      [child, '::after', 'anim3'],
+    ];
+  pseudoAnimCompare(animations, expectedAnimations)
+
+  // Swap animation1 and aimation3's effect
+  var anim1 = animations[0];
+  var anim3 = animations[2];
+  const temp = anim1.effect;
+  anim1.effect = anim3.effect;
+  anim3.effect = temp;
+
+  animations = document.getAnimations();
+  expectedAnimations = [
+      [child, '::before', 'anim2'],
+      [parent, '::after', 'anim1'],
+      [parent, '::before', 'anim4'],
+      [child, '::after', 'anim3'],
+    ];
+  pseudoAnimCompare(animations, expectedAnimations)
+}, 'pseudo element with replaced target does not affect animation ordering');
+
+function pseudoAnimCompare(animations, expectedAnimations) {
+  assert_equals(
+      animations.length,
+      expectedAnimations.length,
+      'CSS animations on both pseudo-elements and elements are returned'
+    );
+  for (const [index, expected] of expectedAnimations.entries()) {
+      const [element, pseudo, name] = expected;
+      const actual = animations[index];
+      if (pseudo) {
+        assert_equals(
+          actual.effect.target,
+          element,
+          `Animation #${index + 1} has the expected target`
+        );
+        assert_equals(
+          actual.effect.pseudoElement,
+          pseudo,
+          `Animation #${index + 1} has the expected pseudo type`
+        );
+        if (name) {
+          assert_equals(
+            actual.animationName,
+            name,
+            `Animation #${index + 1} has the expected name`
+          );
+        }
+      } else {
+        assert_equals(
+          actual.effect.target,
+          element,
+          `Animation #${index + 1} has the expected target`
+        );
+        assert_equals(
+          actual.effect.pseudoElement,
+          null,
+          `Animation #${index + 1} has a null pseudo type`
+        );
+      }
+    }
+}
+
 function pseudoTest(description, testMarkerPseudos) {
   test(t => {
     // Create two divs with the following arrangement:
@@ -327,40 +419,7 @@ function pseudoTest(description, testMarkerPseudos) {
     }
 
     const animations = document.getAnimations();
-    assert_equals(
-      animations.length,
-      expectedAnimations.length,
-      'CSS animations on both pseudo-elements and elements are returned'
-    );
-
-    for (const [index, expected] of expectedAnimations.entries()) {
-      const [element, pseudo] = expected;
-      const actual = animations[index];
-
-      if (pseudo) {
-        assert_equals(
-          actual.effect.target,
-          element,
-          `Animation #${index + 1} has expected target`
-        );
-        assert_equals(
-          actual.effect.pseudoElement,
-          pseudo,
-          `Animation #${index + 1} has expected pseudo type`
-        );
-      } else {
-        assert_equals(
-          actual.effect.target,
-          element,
-          `Animation #${index + 1} has expected target`
-        );
-        assert_equals(
-          actual.effect.pseudoElement,
-          null,
-          `Animation #${index + 1} has null pseudo type`
-        );
-      }
-    }
+    pseudoAnimCompare(animations, expectedAnimations)
   }, description);
 }
 
@@ -368,5 +427,4 @@ pseudoTest('CSS Animations targetting (pseudo-)elements should have correct '
      + 'order after sorting', false);
 pseudoTest('CSS Animations targetting (pseudo-)elements should have correct '
      + 'order after sorting (::marker)', true);
-
 </script>