Fix crashes with null timelines.

The patch addresses cases where timeline-dependent code was being called without first checking for an active timeline. Bug: 967507, 967509 Change-Id: I4383bf5cdbcc8dd92e52c4e281d89d3efd36bd83 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632615 Commit-Queue: Kevin Ellis <kevers@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#664249}

Fix crashes with null timelines.
The patch addresses cases where timeline-dependent code was being called without first checking for an active timeline. Bug: 967507, 967509 Change-Id: I4383bf5cdbcc8dd92e52c4e281d89d3efd36bd83 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632615 Commit-Queue: Kevin Ellis <kevers@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/master@{#664249}
1a0238cc · Kevin Ellis · Commit Bot · 0a7e84a2 · 1a0238cc · 1a0238cc
Commit 1a0238cc authored May 29, 2019 by Kevin Ellis Committed by Commit Bot May 29, 2019
2 changed files
--- a/third_party/blink/renderer/core/animation/animation.cc
+++ b/third_party/blink/renderer/core/animation/animation.cc
@@ -519,9 +519,11 @@ bool Animation::Affects(const Element& element,

 base::Optional<double> Animation::CalculateStartTime(
    double current_time) const {
-  base::Optional<double> start_time =
-      timeline_->EffectiveTime() - current_time / playback_rate_;
-  DCHECK(!IsNull(start_time.value()));
+  base::Optional<double> start_time;
+  if (timeline_) {
+    start_time = timeline_->EffectiveTime() - current_time / playback_rate_;
+    DCHECK(!IsNull(start_time.value()));
+  }
  return start_time;
 }

@@ -981,7 +983,8 @@ void Animation::SetOutdated() {
 }

 void Animation::ForceServiceOnNextFrame() {
-  timeline_->Wake();
+  if (timeline_)
+    timeline_->Wake();
 }

 CompositorAnimations::FailureReasons

--- a/third_party/blink/web_tests/animations/web-animations/animation-null-timeline.html
+++ b/third_party/blink/web_tests/animations/web-animations/animation-null-timeline.html
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Operating on an animation with a null timeline</title>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<body>
+<script>
+// Ensure that operations on an animation object are well behaved with a
+// null timeline. TODO(crbug.com/967416) Consider moving to WPT once edge cases
+// for handling of null timelines are fully flushed out.
+
+// crbug.com/967509
+test(t => {
+  const effect = new KeyframeEffect(null, null, 1000);
+  const animation = new Animation(effect, null);
+  animation.finish();
+  assert_equals(animation.startTime, null);
+  assert_equals(animation.currentTime, 1000);
+  assert_equals(animation.playState, 'finished');
+}, 'Animation finished with a null timeline');
+
+// crbug.com/967507
+test(t => {
+  const effect = new KeyframeEffect(null, null, 1000);
+  const animation1 = document.body.animate([], 1000);
+  assert_true(!!animation1.effect.target);
+  const animation2 = new Animation(effect, null);
+  assert_false(!!animation2.effect.target);
+  animation2.pause();
+  animation2.currentTime = 500;
+  assert_equals(animation2.playState, 'paused');
+  assert_equals(animation2.currentTime, 500);
+  animation1.effect = effect;
+  assert_false(!!animation1.effect.target);
+  assert_false(!!animation2.effect);
+  assert_equals(animation2.playState, 'idle');
+}, 'Set effect with a null timeline');
+</script>