From 394700123601b1145ffdfabc2998e29ca2f530c9 Mon Sep 17 00:00:00 2001
From: Brian Birtles <birtles@gmail.com>
Date: Fri, 8 May 2015 16:17:13 +0900
Subject: [PATCH] Bug 1161320 - Fix conflict between finishing and aborting a
 pause; r=jwatt

Animation::ResumeAt contains an assertion that, when we exit the play-pending
state, checks we either have a resolved start time or a resolved hold time.

That's normally true but if we are aborting a pause on animation that is
finished we can end up with a resolved start time (since we don't clear the
start time when we're aborting a pause) and a resolved hold time (either
because the regular finishing behavior set one, or, because play() applied
auto-rewinding behavior and set it).

In that case we should actually respect the hold time and update the start time
when resuming the animation. However, ResumeAt won't update the start time if it
is already set.

This patch fixes that by clearing the start time in DoPlay in the case where we
are aborting a pause and have a hold time.

--HG--
extra : rebase_source : 83f980d6cbc34375274f30f6527992b4fec7f639
---
 dom/animation/Animation.cpp                   | 16 +++++++++---
 .../file_animation-currenttime.html           | 23 +++++++++++++++++
 layout/style/crashtests/1161320-1.html        | 25 +++++++++++++++++++
 layout/style/crashtests/1161320-2.html        | 25 +++++++++++++++++++
 layout/style/crashtests/crashtests.list       |  2 ++
 5 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100644 layout/style/crashtests/1161320-1.html
 create mode 100644 layout/style/crashtests/1161320-2.html

diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp
index 154aa0aa0266..e15006bdfa1b 100644
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -576,10 +576,18 @@ Animation::DoPlay(LimitBehavior aLimitBehavior)
     return;
   }
 
-  // Clear the start time until we resolve a new one (unless we are aborting
-  // a pending pause operation, in which case we keep the old start time so
-  // that the animation continues moving uninterrupted by the aborted pause).
-  if (!abortedPause) {
+  // Clear the start time until we resolve a new one. We do this except
+  // for the case where we are aborting a pause and don't have a hold time.
+  //
+  // If we're aborting a pause and *do* have a hold time (e.g. because
+  // the animation is finished or we just applied the auto-rewind behavior
+  // above) we should respect it by clearing the start time. If we *don't*
+  // have a hold time we should keep the current start time so that the
+  // the animation continues moving uninterrupted by the aborted pause.
+  //
+  // (If we're not aborting a pause, mHoldTime must be resolved by now
+  //  or else we would have returned above.)
+  if (!mHoldTime.IsNull()) {
     mStartTime.SetNull();
   }
 
diff --git a/dom/animation/test/css-animations/file_animation-currenttime.html b/dom/animation/test/css-animations/file_animation-currenttime.html
index eca14dedbd21..7d4f90c76c66 100644
--- a/dom/animation/test/css-animations/file_animation-currenttime.html
+++ b/dom/animation/test/css-animations/file_animation-currenttime.html
@@ -542,6 +542,29 @@ test(function(t) {
                 'The currentTime of a cancelled animation should be null');
 }, 'Animation.currentTime after cancelling');
 
+async_test(function(t) {
+  var div = addDiv(t, {'class': 'animated-div'});
+  div.style.animation = 'anim 100s';
+
+  var animation = div.getAnimations()[0];
+  animation.ready.then(t.step_func(function() {
+    animation.finish();
+
+    // Initiate a pause then abort it
+    animation.pause();
+    animation.play();
+
+    // Wait to return to running state
+    return animation.ready;
+  })).then(t.step_func(function() {
+    assert_true(animation.currentTime < 100 * 1000,
+                'After aborting a pause when finished, the currentTime should'
+                + ' jump back towards the start of the animation');
+    t.done();
+  }));
+}, 'After aborting a pause when finished, the call to play() should rewind'
+   + ' the current time');
+
 done();
     </script>
   </body>
diff --git a/layout/style/crashtests/1161320-1.html b/layout/style/crashtests/1161320-1.html
new file mode 100644
index 000000000000..3cb3a7b45ef2
--- /dev/null
+++ b/layout/style/crashtests/1161320-1.html
@@ -0,0 +1,25 @@
+<!doctype html>
+<html class="reftest-wait">
+<head>
+<meta charset=utf-8>
+<style>
+@keyframes a { }
+body {
+  animation-name: a;
+}
+</style>
+
+<script>
+function boom()
+{
+  var body = document.body;
+  body.style.animationPlayState = 'paused';
+  window.getComputedStyle(body).animationPlayState;
+  body.style.animationPlayState = 'running';
+  document.documentElement.removeAttribute("class");
+}
+</script>
+</head>
+
+<body onload="setTimeout(boom, 100);"></body>
+</html>
diff --git a/layout/style/crashtests/1161320-2.html b/layout/style/crashtests/1161320-2.html
new file mode 100644
index 000000000000..71db694d013d
--- /dev/null
+++ b/layout/style/crashtests/1161320-2.html
@@ -0,0 +1,25 @@
+<!doctype html>
+<html class="reftest-wait">
+<head>
+<meta charset=utf-8>
+<style>
+@keyframes a { }
+body {
+  animation: a 100s;
+}
+</style>
+
+<script>
+function boom()
+{
+  var anim = document.body.getAnimations()[0];
+  anim.finish();
+  anim.pause();
+  anim.play();
+  document.documentElement.removeAttribute("class");
+}
+</script>
+</head>
+
+<body onload="setTimeout(boom, 100);"></body>
+</html>
diff --git a/layout/style/crashtests/crashtests.list b/layout/style/crashtests/crashtests.list
index 5d8ee5f58160..d45021c256e1 100644
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -113,6 +113,8 @@ load 1074651-1.html
 pref(dom.webcomponents.enabled,true) load 1089463-1.html
 pref(layout.css.expensive-style-struct-assertions.enabled,true) load 1136010-1.html
 load 1153693-1.html
+load 1161320-1.html
+pref(dom.animations-api.core.enabled,true) load 1161320-2.html
 load 1161366-1.html
 load 1163446-1.html
 load large_border_image_width.html