Patch to return Promise objects from Animation playback methods

return-promises.patch

# HG changeset patch
# Parent  994a8d6eccbcdc6106794705bd77e3ac5f031be2

diff --git a/dom/animation/Animation.cpp b/dom/animation/Animation.cpp
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -391,11 +391,11 @@ Animation::SetPlaybackRate(double aPlayb
 }
 
 // https://drafts.csswg.org/web-animations/#seamlessly-update-the-playback-rate
-void
-Animation::UpdatePlaybackRate(double aPlaybackRate)
+Promise*
+Animation::UpdatePlaybackRate(double aPlaybackRate, ErrorResult& aRv)
 {
   if (mPendingPlaybackRate && mPendingPlaybackRate.value() == aPlaybackRate) {
-    return;
+    return GetReady(aRv);
   }
 
   mPendingPlaybackRate = Some(aPlaybackRate);
@@ -403,7 +403,7 @@ Animation::UpdatePlaybackRate(double aPl
   // If we already have a pending task, there is nothing more to do since the
   // playback rate will be applied then.
   if (Pending()) {
-    return;
+    return GetReady(aRv);
   }
 
   AutoMutationBatchForAnimation mb(*this);
@@ -465,6 +465,8 @@ Animation::UpdatePlaybackRate(double aPl
     MOZ_ASSERT(!rv.Failed(),
                "We should only fail to play when using auto-rewind behavior");
   }
+
+  return GetReady(aRv);
 }
 
 // https://drafts.csswg.org/web-animations/#play-state
@@ -497,6 +499,8 @@ Animation::PlayState() const
 Promise*
 Animation::GetReady(ErrorResult& aRv)
 {
+  MOZ_ASSERT(!aRv.Failed());
+
   nsCOMPtr<nsIGlobalObject> global = GetOwnerGlobal();
   if (!mReady && global) {
     mReady = Promise::Create(global, aRv); // Lazily create on demand
@@ -514,6 +518,8 @@ Animation::GetReady(ErrorResult& aRv)
 Promise*
 Animation::GetFinished(ErrorResult& aRv)
 {
+  MOZ_ASSERT(!aRv.Failed());
+
   nsCOMPtr<nsIGlobalObject> global = GetOwnerGlobal();
   if (!mFinished && global) {
     mFinished = Promise::Create(global, aRv); // Lazily create on demand
@@ -610,18 +616,18 @@ Animation::Pause(ErrorResult& aRv)
 }
 
 // https://drafts.csswg.org/web-animations/#reverse-an-animation
-void
+Promise*
 Animation::Reverse(ErrorResult& aRv)
 {
   if (!mTimeline || mTimeline->GetCurrentTime().IsNull()) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
-    return;
+    return nullptr;
   }
 
   double effectivePlaybackRate = CurrentOrPendingPlaybackRate();
 
   if (effectivePlaybackRate == 0.0) {
-    return;
+    return GetFinished(aRv);
   }
 
   Maybe<double> originalPendingPlaybackRate = mPendingPlaybackRate;
@@ -630,14 +636,14 @@ Animation::Reverse(ErrorResult& aRv)
 
   Play(aRv, LimitBehavior::AutoRewind);
 
-  // If Play() threw, restore state and don't report anything to mutation
-  // observers.
   if (aRv.Failed()) {
     mPendingPlaybackRate = originalPendingPlaybackRate;
+    return nullptr;
   }
 
   // Play(), above, unconditionally calls PostUpdate so we don't need to do
   // it here.
+  return GetFinished(aRv);
 }
 
 // ---------------------------------------------------------------------------
diff --git a/dom/animation/Animation.h b/dom/animation/Animation.h
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -119,8 +119,8 @@ public:
   void Finish(ErrorResult& aRv);
   virtual void Play(ErrorResult& aRv, LimitBehavior aLimitBehavior);
   virtual void Pause(ErrorResult& aRv);
-  void Reverse(ErrorResult& aRv);
-  void UpdatePlaybackRate(double aPlaybackRate);
+  Promise* Reverse(ErrorResult& aRv);
+  Promise* UpdatePlaybackRate(double aPlaybackRate, ErrorResult &aRv);
   bool IsRunningOnCompositor() const;
   IMPL_EVENT_HANDLER(finish);
   IMPL_EVENT_HANDLER(cancel);
@@ -138,16 +138,27 @@ public:
                               ErrorResult& aRv);
   virtual AnimationPlayState PlayStateFromJS() const { return PlayState(); }
   virtual bool PendingFromJS() const { return Pending(); }
-  virtual void PlayFromJS(ErrorResult& aRv)
+  virtual Promise* PlayFromJS(ErrorResult& aRv)
   {
     Play(aRv, LimitBehavior::AutoRewind);
+    if (aRv.Failed()) {
+      return nullptr;
+    }
+    return GetFinished(aRv);
   }
   /**
    * PauseFromJS is currently only here for symmetry with PlayFromJS but
    * in future we will likely have to flush style in
    * CSSAnimation::PauseFromJS so we leave it for now.
    */
-  void PauseFromJS(ErrorResult& aRv) { Pause(aRv); }
+  Promise* PauseFromJS(ErrorResult& aRv)
+  {
+    Pause(aRv);
+    if (aRv.Failed()) {
+      return nullptr;
+    }
+    return GetReady(aRv);
+  }
 
   // Wrapper functions for Animation DOM methods when called from style.
 
diff --git a/dom/webidl/Animation.webidl b/dom/webidl/Animation.webidl
--- a/dom/webidl/Animation.webidl
+++ b/dom/webidl/Animation.webidl
@@ -41,12 +41,13 @@ interface Animation : EventTarget {
   [Throws]
   void finish ();
   [Throws, BinaryName="playFromJS"]
-  void play ();
+  Promise<Animation> play ();
   [Throws, BinaryName="pauseFromJS"]
-  void pause ();
-  void updatePlaybackRate (double playbackRate);
+  Promise<Animation> pause ();
   [Throws]
-  void reverse ();
+  Promise<Animation> updatePlaybackRate (double playbackRate);
+  [Throws]
+  Promise<Animation> reverse ();
 };
 
 // Non-standard extensions
diff --git a/layout/style/nsAnimationManager.cpp b/layout/style/nsAnimationManager.cpp
--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@@ -96,13 +96,13 @@ CSSAnimation::PendingFromJS() const
   return Animation::PendingFromJS();
 }
 
-void
+mozilla::dom::Promise*
 CSSAnimation::PlayFromJS(ErrorResult& aRv)
 {
   // Note that flushing style below might trigger calls to
   // PlayFromStyle()/PauseFromStyle() on this object.
   FlushStyle();
-  Animation::PlayFromJS(aRv);
+  return Animation::PlayFromJS(aRv);
 }
 
 void
diff --git a/layout/style/nsAnimationManager.h b/layout/style/nsAnimationManager.h
--- a/layout/style/nsAnimationManager.h
+++ b/layout/style/nsAnimationManager.h
@@ -86,7 +86,7 @@ public:
   // the playState instead.
   AnimationPlayState PlayStateFromJS() const override;
   bool PendingFromJS() const override;
-  void PlayFromJS(ErrorResult& aRv) override;
+  Promise* PlayFromJS(ErrorResult& aRv) override;
 
   void PlayFromStyle();
   void PauseFromStyle();
diff --git a/layout/style/nsTransitionManager.cpp b/layout/style/nsTransitionManager.cpp
--- a/layout/style/nsTransitionManager.cpp
+++ b/layout/style/nsTransitionManager.cpp
@@ -193,11 +193,11 @@ CSSTransition::PendingFromJS() const
   return Animation::PendingFromJS();
 }
 
-void
+mozilla::dom::Promise*
 CSSTransition::PlayFromJS(ErrorResult& aRv)
 {
   FlushStyle();
-  Animation::PlayFromJS(aRv);
+  return Animation::PlayFromJS(aRv);
 }
 
 void
diff --git a/layout/style/nsTransitionManager.h b/layout/style/nsTransitionManager.h
--- a/layout/style/nsTransitionManager.h
+++ b/layout/style/nsTransitionManager.h
@@ -139,7 +139,7 @@ public:
   // Animation interface overrides
   AnimationPlayState PlayStateFromJS() const override;
   bool PendingFromJS() const override;
-  void PlayFromJS(ErrorResult& aRv) override;
+  Promise* PlayFromJS(ErrorResult& aRv) override;
 
   // A variant of Play() that avoids posting style updates since this method
   // is expected to be called whilst already updating style.
diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -358708,12 +358708,24 @@
      {}
     ]
    ],
+   "web-animations/interfaces/Animation/reverse.html": [
+    [
+     "/web-animations/interfaces/Animation/reverse.html",
+     {}
+    ]
+   ],
    "web-animations/interfaces/Animation/startTime.html": [
     [
      "/web-animations/interfaces/Animation/startTime.html",
      {}
     ]
    ],
+   "web-animations/interfaces/Animation/updatePlaybackRate.html": [
+    [
+     "/web-animations/interfaces/Animation/updatePlaybackRate.html",
+     {}
+    ]
+   ],
    "web-animations/interfaces/AnimationEffectTiming/delay.html": [
     [
      "/web-animations/interfaces/AnimationEffectTiming/delay.html",
@@ -485306,7 +485318,7 @@
    "support"
   ],
   "css/css-fonts/font-feature-settings-serialization-001.html": [
-   "d1032e08aee1e9a7d1309ad94bd5633535ce9315",
+   "bf557e7f93663a36dab3ea358401b16c2e88811a",
    "testharness"
   ],
   "css/css-fonts/font-features-across-space-1-ref.html": [
@@ -563122,7 +563134,7 @@
    "support"
   ],
   "interfaces/dom.idl": [
-   "52236516620dac45213fa06dc169f0c02e63a0c5",
+   "773c449a2f9a6bd9e35d0dd8a4c2e1eaa0266150",
    "support"
   ],
   "interfaces/fullscreen.idl": [
@@ -591550,7 +591562,7 @@
    "testharness"
   ],
   "web-animations/interfaces/Animation/idlharness.html": [
-   "d61aa2d95ea31809a275183408e822c8c1eec87d",
+   "caa3bfee88cc510f17a4a96bcc4df2c8260ca11f",
    "testharness"
   ],
   "web-animations/interfaces/Animation/oncancel.html": [
@@ -591562,7 +591574,7 @@
    "testharness"
   ],
   "web-animations/interfaces/Animation/pause.html": [
-   "f044cc7ac09c38f127e399f7d6f9a0714046aa8e",
+   "813c828c80ad583eaa2d44bf19d8f147f06a0297",
    "testharness"
   ],
   "web-animations/interfaces/Animation/pending.html": [
@@ -591570,17 +591582,25 @@
    "testharness"
   ],
   "web-animations/interfaces/Animation/play.html": [
-   "54edbdd6c0e1953f8d0e2bfbb92bfe318114ab74",
+   "1a4b47685c577c6b385ec5fec92e024cb7371eb0",
    "testharness"
   ],
   "web-animations/interfaces/Animation/ready.html": [
    "bd4a18205791b2b0271a6266dba3ebc8482c835b",
    "testharness"
   ],
+  "web-animations/interfaces/Animation/reverse.html": [
+   "5ec018a6ec68e1c58db405a1b33789141351e91b",
+   "testharness"
+  ],
   "web-animations/interfaces/Animation/startTime.html": [
    "01f669542434f03d37e9f148a4f3135fe3122d46",
    "testharness"
   ],
+  "web-animations/interfaces/Animation/updatePlaybackRate.html": [
+   "050c2aa2030d49329bb14eb97258187b613e5d04",
+   "testharness"
+  ],
   "web-animations/interfaces/AnimationEffectTiming/delay.html": [
    "4de5b0a692d645961de27df67efa8257adb0a031",
    "testharness"
diff --git a/testing/web-platform/tests/web-animations/interfaces/Animation/idlharness.html b/testing/web-platform/tests/web-animations/interfaces/Animation/idlharness.html
--- a/testing/web-platform/tests/web-animations/interfaces/Animation/idlharness.html
+++ b/testing/web-platform/tests/web-animations/interfaces/Animation/idlharness.html
@@ -27,10 +27,10 @@ interface Animation : EventTarget {
              attribute EventHandler             oncancel;
     void cancel ();
     void finish ();
-    void play ();
-    void pause ();
-    void updatePlaybackRate (double playbackRate);
-    void reverse ();
+    Promise<Animation> play ();
+    Promise<Animation> pause ();
+    Promise<Animation> updatePlaybackRate (double playbackRate);
+    Promise<Animation> reverse ();
 };
 </script>
 <script>
diff --git a/testing/web-platform/tests/web-animations/interfaces/Animation/pause.html b/testing/web-platform/tests/web-animations/interfaces/Animation/pause.html
--- a/testing/web-platform/tests/web-animations/interfaces/Animation/pause.html
+++ b/testing/web-platform/tests/web-animations/interfaces/Animation/pause.html
@@ -94,5 +94,7 @@ promise_test(t => {
     });
 }, 'pause() on a finished animation');
 
+// XXX Need tests for returning ready Promise here
+
 </script>
 </body>
diff --git a/testing/web-platform/tests/web-animations/interfaces/Animation/play.html b/testing/web-platform/tests/web-animations/interfaces/Animation/play.html
--- a/testing/web-platform/tests/web-animations/interfaces/Animation/play.html
+++ b/testing/web-platform/tests/web-animations/interfaces/Animation/play.html
@@ -30,5 +30,17 @@ promise_test(t => {
 }, 'play() throws when seeking an infinite-duration animation played in ' +
    'reverse');
 
+test(t => {
+  const animation = createDiv(t).animate(null, 100 * MS_PER_SEC);
+
+  // Restarting after finishing should create a new finished promise
+  animation.finish();
+  const previousFinishedPromise = animation.finished;
+  const result = animation.play();
+
+  assert_equals(result, animation.finished);
+  assert_not_equals(result, previousFinishedPromise);
+}, 'play() returns the current finished promise');
+
 </script>
 </body>
diff --git a/testing/web-platform/tests/web-animations/interfaces/Animation/reverse.html b/testing/web-platform/tests/web-animations/interfaces/Animation/reverse.html
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/web-animations/interfaces/Animation/reverse.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Animation.reverse</title>
+<link rel="help" href="https://drafts.csswg.org/web-animations/#dom-animation-reverse">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../testcommon.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+test(t => {
+  const animation = createDiv(t).animate(null, 100 * MS_PER_SEC);
+
+  // Reversing after finishing should create a new finished promise
+  animation.finish();
+  const previousFinishedPromise = animation.finished;
+  const result = animation.reverse();
+
+  assert_equals(result, animation.finished);
+  assert_not_equals(result, previousFinishedPromise);
+}, 'reverse() returns the current finished promise');
+
+// XXX As above but when playback rate is zero
+
+</script>
+</body>
diff --git a/testing/web-platform/tests/web-animations/interfaces/Animation/updatePlaybackRate.html b/testing/web-platform/tests/web-animations/interfaces/Animation/updatePlaybackRate.html
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/web-animations/interfaces/Animation/updatePlaybackRate.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<title>Animation.updatePlaybackRate</title>
+<link rel="help" href="https://drafts.csswg.org/web-animations-1/#dom-animation-updateplaybackrate">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../../testcommon.js"></script>
+<body>
+<div id="log"></div>
+<script>
+'use strict';
+
+// XXX Need tests for returning ready Promise here
+
+</script>
+</body>