Added hold_phase to Animation

Prior to this change, effects are always given the current phase of the
timeline associated with them. This fails in certain situations such as
pausing and then scrolling, setting current time with an inactive
timeline, and many other scenarios where hold_time is set.

In order to fix this, we have introduced hold_phase. Anytime hold_time
is updated, hold_phase will also be updated with the appropriate phase.
Sometimes the phase is set explicitly and other times we pull from the
timeline current time.

Animation effects will be given the animation hold_phase if it is
populated, otherwise it will calculate its current phase (similar to how
current time is calculated).

The WebAnimations spec is being updated to reflect this change.

Bug: 1046833
Change-Id: I4bf1e42eaab684c18829a79acc1ab8911ec893af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135336
Commit-Queue: Jordan Taylor <jortaylo@microsoft.com>
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797835}

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

@@ -309,7 +309,8 @@ void Animation::setCurrentTime(base::Optional<double> new_current_time,
 
   // Synchronously resolve pending pause task.
   if (pending_pause_) {
-    hold_time_ = MillisecondsToSeconds(new_current_time.value());
+    SetHoldTimeAndPhase(MillisecondsToSeconds(new_current_time.value()),
+                        TimelinePhase::kActive);
     ApplyPendingPlaybackRate();
     start_time_ = base::nullopt;
     pending_pause_ = false;
@@ -339,13 +340,15 @@ void Animation::SetCurrentTimeInternal(double new_current_time) {
 
   base::Optional<double> previous_start_time = start_time_;
   base::Optional<double> previous_hold_time = hold_time_;
+  base::Optional<TimelinePhase> previous_hold_phase = hold_phase_;
 
   // Update either the hold time or the start time.
   if (hold_time_ || !start_time_ || !timeline_ || !timeline_->IsActive() ||
-      playback_rate_ == 0)
-    hold_time_ = new_current_time;
-  else
+      playback_rate_ == 0) {
+    SetHoldTimeAndPhase(new_current_time, TimelinePhase::kActive);
+  } else {
     start_time_ = CalculateStartTime(new_current_time);
+  }
 
   // Preserve invariant that we can only set a start time or a hold time in the
   // absence of an active timeline.
@@ -355,10 +358,26 @@ void Animation::SetCurrentTimeInternal(double new_current_time) {
   // Reset the previous current time.
   previous_current_time_ = base::nullopt;
 
-  if (previous_start_time != start_time_ || previous_hold_time != hold_time_)
+  if (previous_start_time != start_time_ || previous_hold_time != hold_time_ ||
+      previous_hold_phase != hold_phase_)
     SetOutdated();
 }
 
+void Animation::SetHoldTimeAndPhase(
+    base::Optional<double> new_hold_time /* in seconds */,
+    TimelinePhase new_hold_phase) {
+  // new_hold_time must be valid, unless new_hold_phase is inactive.
+  DCHECK(new_hold_time ||
+         (!new_hold_time && new_hold_phase == TimelinePhase::kInactive));
+  hold_time_ = new_hold_time;
+  hold_phase_ = new_hold_phase;
+}
+
+void Animation::ResetHoldTimeAndPhase() {
+  hold_time_ = base::nullopt;
+  hold_phase_ = base::nullopt;
+}
+
 base::Optional<double> Animation::startTime() const {
   return start_time_
              ? base::make_optional(SecondsToMilliseconds(start_time_.value()))
@@ -393,10 +412,22 @@ base::Optional<double> Animation::currentTime() const {
   return SecondsToMilliseconds(current_time);
 }
 
+bool Animation::ValidateHoldTimeAndPhase() const {
+  return (hold_phase_ && hold_time_) ||
+         ((!hold_phase_ || hold_phase_ == TimelinePhase::kInactive) &&
+          !hold_time_);
+}
+
 base::Optional<double> Animation::CurrentTimeInternal() const {
+  DCHECK(ValidateHoldTimeAndPhase());
   return hold_time_ ? hold_time_ : CalculateCurrentTime();
 }
 
+TimelinePhase Animation::CurrentPhaseInternal() const {
+  DCHECK(ValidateHoldTimeAndPhase());
+  return hold_phase_ ? hold_phase_.value() : CalculateCurrentPhase();
+}
+
 base::Optional<double> Animation::UnlimitedCurrentTime() const {
   return CalculateAnimationPlayState() == kPaused || !start_time_
              ? CurrentTimeInternal()
@@ -615,7 +646,7 @@ void Animation::CommitPendingPlay(double ready_time) {
       start_time_ = ready_time;
     } else {
       start_time_ = ready_time - hold_time_.value() / playback_rate_;
-      hold_time_ = base::nullopt;
+      ResetHoldTimeAndPhase();
     }
   } else if (start_time_ && pending_playback_rate_) {
     // B: If animation’s start time is resolved and animation has a pending
@@ -634,7 +665,7 @@ void Animation::CommitPendingPlay(double ready_time) {
         (ready_time - start_time_.value()) * playback_rate_;
     ApplyPendingPlaybackRate();
     if (playback_rate_ == 0) {
-      hold_time_ = current_time_to_match;
+      SetHoldTimeAndPhase(current_time_to_match, CalculateCurrentPhase());
       start_time_ = ready_time;
     } else {
       start_time_ = ready_time - current_time_to_match / playback_rate_;
@@ -665,8 +696,10 @@ void Animation::CommitPendingPause(double ready_time) {
   // 2. If animation’s start time is resolved and its hold time is not resolved,
   //    let animation’s hold time be the result of evaluating
   //    (ready time - start time) × playback rate.
-  if (start_time_ && !hold_time_)
-    hold_time_ = (ready_time - start_time_.value()) * playback_rate_;
+  if (start_time_ && !hold_time_) {
+    SetHoldTimeAndPhase((ready_time - start_time_.value()) * playback_rate_,
+                        CalculateCurrentPhase());
+  }
 
   // 3. Apply any pending playback rate on animation.
   // 4. Make animation’s start time unresolved.
@@ -721,6 +754,12 @@ base::Optional<double> Animation::CalculateCurrentTime() const {
   return (timeline_time.value() - start_time_.value()) * playback_rate_;
 }
 
+TimelinePhase Animation::CalculateCurrentPhase() const {
+  if (!start_time_ || !timeline_)
+    return TimelinePhase::kInactive;
+  return timeline_->Phase();
+}
+
 // https://drafts.csswg.org/web-animations/#setting-the-start-time-of-an-animation
 void Animation::setStartTime(base::Optional<double> start_time_ms,
                              ExceptionState& exception_state) {
@@ -739,11 +778,13 @@ void Animation::setStartTime(base::Optional<double> start_time_ms,
   // This preserves the invariant that when we don’t have an active timeline it
   // is only possible to set either the start time or the animation’s current
   // time.
-  if (!timeline_time && start_time_ms)
-    hold_time_ = base::nullopt;
+  if (!timeline_time && start_time_ms) {
+    ResetHoldTimeAndPhase();
+  }
 
   // 3. Let previous current time be animation’s current time.
   base::Optional<double> previous_current_time = CurrentTimeInternal();
+  TimelinePhase previous_current_phase = CurrentPhaseInternal();
 
   // 4. Apply any pending playback rate on animation.
   ApplyPendingPlaybackRate();
@@ -770,10 +811,11 @@ void Animation::setStartTime(base::Optional<double> start_time_ms,
   //      Set animation’s hold time to previous current time even if previous
   //      current time is unresolved.
   if (start_time_) {
-    if (playback_rate_ != 0)
-      hold_time_ = base::nullopt;
+    if (playback_rate_ != 0) {
+      ResetHoldTimeAndPhase();
+    }
   } else {
-    hold_time_ = previous_current_time;
+    SetHoldTimeAndPhase(previous_current_time, previous_current_phase);
   }
 
   // 7. If animation has a pending play task or a pending pause task, cancel
@@ -1035,10 +1077,11 @@ void Animation::pause(ExceptionState& exception_state) {
   base::Optional<double> current_time = CurrentTimeInternal();
   if (!current_time) {
     if (playback_rate_ >= 0) {
-      if (has_finite_timeline)
+      if (has_finite_timeline) {
         start_time_ = 0;
-      else
-        hold_time_ = 0;
+      } else {
+        SetHoldTimeAndPhase(0, TimelinePhase::kActive);
+      }
     } else {
       if (EffectEnd() == std::numeric_limits<double>::infinity()) {
         exception_state.ThrowDOMException(
@@ -1046,10 +1089,11 @@ void Animation::pause(ExceptionState& exception_state) {
             "Cannot play reversed Animation with infinite target effect end.");
         return;
       }
-      if (has_finite_timeline)
+      if (has_finite_timeline) {
         start_time_ = EffectEnd();
-      else
-        hold_time_ = EffectEnd();
+      } else {
+        SetHoldTimeAndPhase(EffectEnd(), TimelinePhase::kActive);
+      }
     }
   }
 
@@ -1160,10 +1204,11 @@ void Animation::PlayInternal(AutoRewind auto_rewind,
   if (effective_playback_rate > 0 && auto_rewind == AutoRewind::kEnabled &&
       (!current_time || current_time < 0 || current_time >= EffectEnd())) {
     performed_seek = true;
-    if (has_finite_timeline)
+    if (has_finite_timeline) {
       start_time_ = 0;
-    else
-      hold_time_ = 0;
+    } else {
+      SetHoldTimeAndPhase(0, TimelinePhase::kActive);
+    }
   } else if (effective_playback_rate < 0 &&
              auto_rewind == AutoRewind::kEnabled &&
              (!current_time || current_time <= 0 ||
@@ -1175,21 +1220,23 @@ void Animation::PlayInternal(AutoRewind auto_rewind,
       return;
     }
     performed_seek = true;
-    if (has_finite_timeline)
+    if (has_finite_timeline) {
       start_time_ = EffectEnd();
-    else
-      hold_time_ = EffectEnd();
+    } else {
+      SetHoldTimeAndPhase(EffectEnd(), TimelinePhase::kActive);
+    }
   } else if (effective_playback_rate == 0 && !current_time) {
     performed_seek = true;
-    if (has_finite_timeline)
+    if (has_finite_timeline) {
       start_time_ = 0;
-    else
-      hold_time_ = 0;
+    } else {
+      SetHoldTimeAndPhase(0, TimelinePhase::kActive);
+    }
   }
   // TODO(crbug.com/1081267): Update based on upcoming spec change.
   // https://github.com/w3c/csswg-drafts/pull/5059
   if (performed_seek && has_finite_timeline) {
-    hold_time_ = base::nullopt;
+    ResetHoldTimeAndPhase();
     ApplyPendingPlaybackRate();
   }
 
@@ -1299,7 +1346,7 @@ void Animation::finish(ExceptionState& exception_state) {
     start_time_ = CalculateStartTime(new_current_time);
 
   if (pending_pause_ && start_time_) {
-    hold_time_ = base::nullopt;
+    ResetHoldTimeAndPhase();
     pending_pause_ = false;
     if (ready_promise_)
       ResolvePromiseMaybeAsync(ready_promise_.Get());
@@ -1338,20 +1385,29 @@ void Animation::UpdateFinishedState(UpdateType update_type,
     // boundary. The value of previous current time is used to retain this
     // value.
     double playback_rate = EffectivePlaybackRate();
+    base::Optional<double> hold_time;
+    TimelinePhase hold_phase;
     if (playback_rate > 0 && unconstrained_current_time >= EffectEnd()) {
-      hold_time_ = did_seek ? unconstrained_current_time
+      hold_time = did_seek ? unconstrained_current_time
                            : Max(previous_current_time_, EffectEnd());
+      hold_phase = did_seek ? TimelinePhase::kActive : CalculateCurrentPhase();
+
+      SetHoldTimeAndPhase(hold_time, hold_phase);
     } else if (playback_rate < 0 && unconstrained_current_time <= 0) {
-      hold_time_ = did_seek ? unconstrained_current_time
+      hold_time = did_seek ? unconstrained_current_time
                            : Min(previous_current_time_, 0);
+      hold_phase = did_seek ? TimelinePhase::kActive : CalculateCurrentPhase();
+
       // Hack for resolving precision issue at zero.
-      if (hold_time_.value() == -0)
-        hold_time_ = 0;
+      if (hold_time.value() == -0)
+        hold_time = 0;
+
+      SetHoldTimeAndPhase(hold_time, hold_phase);
     } else if (playback_rate != 0) {
       // Update start time and reset hold time.
       if (did_seek && hold_time_)
         start_time_ = CalculateStartTime(hold_time_.value());
-      hold_time_ = base::nullopt;
+      ResetHoldTimeAndPhase();
     }
   }
 
@@ -1844,7 +1900,7 @@ bool Animation::Update(TimingUpdateReason reason) {
 
   if (content_) {
     base::Optional<double> inherited_time;
-    base::Optional<TimelinePhase> timeline_phase;
+    TimelinePhase inherited_phase = TimelinePhase::kInactive;
 
     if (!idle) {
       inherited_time = CurrentTimeInternal();
@@ -1852,10 +1908,10 @@ bool Animation::Update(TimingUpdateReason reason) {
       if (inherited_time == 0 && EffectivePlaybackRate() < 0)
         inherited_time = -1;
 
-      timeline_phase = timeline_->Phase();
+      inherited_phase = CurrentPhaseInternal();
     }
 
-    content_->UpdateInheritedTime(inherited_time, timeline_phase, reason);
+    content_->UpdateInheritedTime(inherited_time, inherited_phase, reason);
 
     // After updating the animation time if the animation is no longer current
     // blink will no longer composite the element (see
@@ -1977,7 +2033,7 @@ void Animation::cancel() {
     pending_pause_ = pending_play_ = false;
   }
 
-  hold_time_ = base::nullopt;
+  ResetHoldTimeAndPhase();
   start_time_ = base::nullopt;
 
   // Apply changes synchronously.
@@ -2093,7 +2149,7 @@ void Animation::PauseForTesting(double pause_time) {
   is_paused_for_testing_ = true;
   pending_pause_ = false;
   pending_play_ = false;
-  hold_time_ = pause_time;
+  SetHoldTimeAndPhase(pause_time, TimelinePhase::kActive);
   start_time_ = base::nullopt;
 }
 

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

@@ -291,6 +291,7 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   void AddedEventListener(const AtomicString& event_type,
                           RegisteredEventListener&) override;
   base::Optional<double> CurrentTimeInternal() const;
+  TimelinePhase CurrentPhaseInternal() const;
   virtual AnimationEffect::EventDelegate* CreateEventDelegate(
       Element* target,
       const AnimationEffect::EventDelegate* old_event_delegate) {
@@ -299,6 +300,11 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
 
  private:
   void SetCurrentTimeInternal(double new_current_time);
+  void SetHoldTimeAndPhase(
+      base::Optional<double> new_hold_time /* in seconds */,
+      TimelinePhase new_hold_phase);
+  void ResetHoldTimeAndPhase();
+  bool ValidateHoldTimeAndPhase() const;
 
   void ClearOutdated();
   void ForceServiceOnNextFrame();
@@ -314,6 +320,7 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
 
   base::Optional<double> CalculateStartTime(double current_time) const;
   base::Optional<double> CalculateCurrentTime() const;
+  TimelinePhase CalculateCurrentPhase() const;
 
   void BeginUpdatingState();
   void EndUpdatingState();
@@ -377,6 +384,7 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   base::Optional<double> pending_playback_rate_;
   base::Optional<double> start_time_;
   base::Optional<double> hold_time_;
+  base::Optional<TimelinePhase> hold_phase_;
   base::Optional<double> previous_current_time_;
 
   unsigned sequence_number_;

third_party/blink/web_tests/external/wpt/scroll-animations/scroll-animation-effect-phases.tentative.html

@@ -151,4 +151,167 @@
       );
     }
   }
+
+  function createKeyframeEffectOpacity(test){
+    return new KeyframeEffect(
+      createDiv(test),
+      {
+        opacity: [0.3, 0.7]
+      },
+      {
+        duration: 1000
+      }
+    );
+  }
+
+  function verifyTimelineBeforePhase(animation){
+    assert_equals(animation.timeline.phase, "before");
+    assert_equals(animation.timeline.currentTime, 0);
+    assert_equals(animation.currentTime, 0);
+    assert_equals(
+      animation.effect.getComputedTiming().localTime,
+      0,
+      "effect local time in timeline before phase");
+  }
+
+  function verifyEffectBeforePhase(animation){
+    // progress == null AND opacity == 1 implies we are in the effect before
+    // phase
+    assert_equals(
+      animation.effect.getComputedTiming().progress,
+      null
+    );
+    assert_equals(
+      window.getComputedStyle(animation.effect.target).getPropertyValue("opacity"),
+      "1"
+    );
+  }
+
+  promise_test(async t => {
+    const animation = new Animation(
+      createKeyframeEffectOpacity(t),
+      createScrollTimelineWithOffsets(t, "20%", "80%")
+    );
+
+    const scroller = animation.timeline.scrollSource;
+    const maxScroll = scroller.scrollHeight - scroller.clientHeight;
+
+    animation.play();
+    await animation.ready;
+
+    verifyTimelineBeforePhase(animation);
+    verifyEffectBeforePhase(animation);
+
+    animation.pause();
+    await waitForNextFrame();
+
+    verifyTimelineBeforePhase(animation);
+    verifyEffectBeforePhase(animation);
+
+    animation.play();
+    await waitForNextFrame();
+
+    verifyTimelineBeforePhase(animation);
+    verifyEffectBeforePhase(animation);
+  }, 'Verify that (play -> pause -> play) doesn\'t change phase/progress.');
+
+  promise_test(async t => {
+    const animation = new Animation(
+      createKeyframeEffectOpacity(t),
+      createScrollTimelineWithOffsets(t, "20%", "80%")
+    );
+
+    const scroller = animation.timeline.scrollSource;
+    const maxScroll = scroller.scrollHeight - scroller.clientHeight;
+
+    animation.play();
+    await animation.ready;
+
+    verifyTimelineBeforePhase(animation);
+    verifyEffectBeforePhase(animation);
+
+    animation.pause();
+    await waitForNextFrame();
+
+    verifyTimelineBeforePhase(animation);
+    verifyEffectBeforePhase(animation);
+
+    // Scrolling should not cause the animation effect to change.
+    scroller.scrollTop = 0.5 * maxScroll;
+    await waitForNextFrame();
+
+    // Check timeline phase
+    assert_equals(animation.timeline.phase, "active");
+    assert_equals(animation.timeline.currentTime, 500);
+    assert_equals(animation.currentTime, 0);
+    assert_equals(
+      animation.effect.getComputedTiming().localTime,
+      0,
+      "effect local time"
+    );
+
+    // Make sure the effect is still in the before phase even though the
+    // timeline is not.
+    verifyEffectBeforePhase(animation);
+  }, 'Pause in before phase, scroll timeline into active phase, animation ' +
+  'should remain in the before phase');
+
+promise_test(async t => {
+  const animation = new Animation(
+    createKeyframeEffectOpacity(t),
+    createScrollTimelineWithOffsets(t, "20%", "80%")
+  );
+  const scroller = animation.timeline.scrollSource;
+  const maxScroll = scroller.scrollHeight - scroller.clientHeight;
+
+  animation.play();
+  await animation.ready;
+
+  // Causes the timeline to be inactive
+  scroller.style.overflow = "visible";
+  await waitForNextFrame();
+  await waitForNextFrame();
+
+  // Check timeline phase
+  assert_equals(animation.timeline.phase, "inactive");
+  assert_equals(animation.timeline.currentTime, null);
+  assert_equals(animation.currentTime, null);
+  assert_equals(
+    animation.effect.getComputedTiming().localTime,
+    null,
+    "effect local time with inactive timeline"
+  );
+
+  verifyEffectBeforePhase(animation);
+
+  // Setting the current time while timeline is inactive should cause hold phase
+  // and hold time to be populated
+  animation.currentTime = 500;
+  await waitForNextFrame();
+  await waitForNextFrame();
+
+  // Check timeline phase
+  assert_equals(animation.timeline.phase, "inactive");
+  assert_equals(animation.timeline.currentTime, null);
+  assert_equals(animation.currentTime, 500);
+  assert_equals(
+    animation.effect.getComputedTiming().localTime,
+    500,
+    "effect local time after setting animation current time"
+  );
+
+  // Check effect phase
+  // progress == 0.5 AND opacity == 0.5 shows we are in the effect active phase
+  assert_equals(
+    animation.effect.getComputedTiming().progress,
+    0.5,
+    "effect progress"
+  );
+  assert_equals(
+    window.getComputedStyle(animation.effect.target).getPropertyValue("opacity"),
+    "0.5",
+    "effect opacity after setting animation current time"
+  );
+}, 'Make scroller inactive, then set current time to an in range time');
+
 </script>
\ No newline at end of file