[scroll-animations] WPT test scroll-animations/scroll-timelines/scrol…

…l-animation-effect-phases.tentative.html has failures

https://bugs.webkit.org/show_bug.cgi?id=284545
rdar://problem/141356963

Reviewed by NOBODY (OOPS!).

The technique used by this test to determine the phase [0] in which an animation
is, which is not explicitly exposed via the Web Animations API, was not up-to-date
with the fact that progress-based animations remain active at the 100% boundary.

So we update the supporting `assert_phase()` function to deal with such animations
and pass the previously failing assertion.

But going further into this test, we would hit a debug assertion when trying to compute
the unconstrained current time as we get to the end of the procedure to set the timeline
as we move from the scroll timeline to the document timeline.

I filed a spec issue [1] to deal with this and came up with a temporary fix to restore
the timing incompatibility and pass this test.

[0] https://drafts.csswg.org/web-animations-1/#animation-effect-phases-and-states
[1] w3c/csswg-drafts#11761

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/scroll-animation-effect-phases.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/web-animations/testcommon.js:
(assert_phase):
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):

LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/scroll-animation-effect-phases.tentative-expected.txt

@@ -22,7 +22,7 @@ PASS Current times and effect phase at timeline boundary when delay = 250 and en
 PASS Current times and effect phase at timeline start when delay = -125 and endDelay = -125 |
 PASS Current times and effect phase in active range when delay = -125 and endDelay = -125 |
 PASS Current times and effect phase at timeline end when delay = -125 and endDelay = -125 |
-FAIL Playback rate affects whether active phase boundary is inclusive. assert_not_equals: Animation effect is in active phase when current time is 100%. got disallowed value null
+PASS Playback rate affects whether active phase boundary is inclusive.
 PASS Verify that (play -> pause -> play) doesn't change phase/progress.
 PASS Pause in before phase, scroll timeline into active phase, animation should remain in the before phase
 PASS Pause in before phase, set animation current time to be in active range, animation should become active. Scrolling should have no effect.

LayoutTests/imported/w3c/web-platform-tests/web-animations/testcommon.js

@@ -297,11 +297,20 @@ function assert_phase(animation, phase) {
 
   if (phase === 'active') {
     // If the fill mode is 'none', then progress will only be non-null if we
-    // are in the active phase.
+    // are in the active phase, except for progress-based timelines where
+    // currentTime = 100% is still 'active'.
     animation.effect.updateTiming({ fill: 'none' });
-    assert_not_equals(animation.effect.getComputedTiming().progress, null,
-                      'Animation effect is in active phase when current time ' +
-                      `is ${currentTime}.`);
+    if ('ScrollTimeline' in window && animation.timeline instanceof ScrollTimeline) {
+        const isActive = animation.currentTime?.toString() == "100%" ||
+                         animation.effect.getComputedTiming().progress != null;
+        assert_true(isActive,
+                    'Animation effect is in active phase when current time ' +
+                    `is ${currentTime}.`);
+    } else {
+      assert_not_equals(animation.effect.getComputedTiming().progress, null,
+                        'Animation effect is in active phase when current time ' +
+                        `is ${currentTime}.`);
+    }
   } else {
     // The easiest way to distinguish between the 'before' phase and the 'after'
     // phase is to toggle the fill mode. For example, if the progress is null

Source/WebCore/animation/WebAnimation.cpp

@@ -338,8 +338,15 @@ void WebAnimation::setTimeline(RefPtr<AnimationTimeline>&& timeline)
     }
 
     // 10. If the start time of animation is resolved, make animation’s hold time unresolved.
-    if (m_startTime)
+    if (m_startTime) {
+        // FIXME: we may now be in a state where the hold time and start times have
+        // incompatible time units per https://github.com/w3c/csswg-drafts/issues/11761.
+        // Until the spec knows how to handle this case, we ensure the start time matches
+        // the value type of the currently resolved hold time before we make it unresolved.
+        if (m_holdTime && m_holdTime->time().has_value() != m_startTime->time().has_value())
+            m_startTime = m_holdTime;
         m_holdTime = std::nullopt;
+    }
 
     // 11. Run the procedure to update an animation's finished state for animation with the did seek flag set to false,
     // and the synchronously notify flag set to false.