For example; addAsyncAnimTest("play_filling_forwards", aOptions, function*() { // Start a long animation with a forwards fill e.style = "animation: anim 100s forwards"; // The animation should cause the creation of a single Animation. var animations = e.getAnimations(); is(animations.length, 1, "getAnimations().length after animation start"); // Wait for the single MutationRecord for the Animation addition to // be delivered. yield waitForFrame(); assert_records([{ added: animations, changed: [], removed: [] }], "records after animation start"); // Wait until the animation is ready yield animations[0].ready; // Seek to the end animations[0].finish(); // Wait for the single MutationRecord for the Animation change to // be delivered. yield waitForFrame(); assert_records([{ added: [], changed: animations, removed: [] }], "records after finish()"); // Since we are filling forwards, calling play() should produce a // change record since the animation remains relevant. animations[0].play(); // Wait for the single MutationRecord for the Animation change to // be delivered. yield waitForFrame(); assert_records([{ added: [], changed: animations, removed: [] }], "records after play()"); // Cancel the animation. e.style = ""; // Wait for the single removal notification. yield waitForFrame(); assert_records([{ added: [], changed: [], removed: animations }], "records after animation end"); }); This test can be rewritten something like this; var div = addDiv(t, { style: "animation: anim 100s forwards" }); var observer = setupSynchronousObserver(...); var animation = e.getAnimations()[0]; assert_equals_records(observer.takeRecords(), [{ added: [anim], changed: [], removed: [] }], "records after animation is added"); animation.finish(); flushComputedStyle(e); // I am not sure this is necessary. animation.play(); assert_equals_record(observer.takeRecord(),...); ...

I had to take this for bug 1416966.

Comment on attachment 8935182 [details] Bug 1416693 - Make transition test cases that don't require asynchronous. https://reviewboard.mozilla.org/r/206044/#review211718 Looks good. Thanks. ::: dom/animation/test/chrome/test_animation_observers_sync.html:973 (Diff revision 1) > + addDiv(t, { style: "transition-duration: 100s; " + > + "transition-property: color, background-color, line-height" + > + "background-color: yellow; line-height: 16px" }); > + var observer = > + setupSynchronousObserver(t, > + aOptions.subtree ? div.parentNode : div, > + aOptions.subtree); > + getComputedStyle(div).transitionProperty; > + > + div.style.backgroundColor = "lime"; > + div.style.color = "blue"; > + div.style.lineHeight = "24px"; We didn't set the initial color value, but this should be fine.

Comment on attachment 8935183 [details] Bug 1416693 - Make animation test cases that don't require asynchronous. https://reviewboard.mozilla.org/r/206046/#review211726 ::: dom/animation/test/chrome/test_animation_observers_sync.html:1224 (Diff revision 1) > + // Finish and pause. > + animations[0].finish(); > + animations[0].pause(); > + > + assert_equals(animations[0].playState, "paused", > + "playState after finishing and pausing"); > + > + // We should have two MutationRecords for the Animation changes: > + // one for the finish, one for the pause. > + assert_equals_records(observer.takeRecords(), > + [{ added: [], changed: animations, removed: [] }, > + { added: [], changed: animations, removed: [] }], > + "records after finish() and pause()"); > + > + // Call finish() again. > + animations[0].finish(); > + > + assert_equals(animations[0].playState, "finished", > + "playState after finishing from paused state"); > + > + assert_equals_records(observer.takeRecords(), > + [{ added: [], changed: animations, removed: [] }], > + "records after finish() and pause()"); I have a question about this: From the original test, we call finish() and pause(), and then wait for "aniamtion ready" (i.e Wait for the pause to complete), and then call finish(), that's why we call this test "finish_from_pause". But now I cannot see the "animation ready" things, so this test now looks like the same as the test below (i.e. "finish_from_pause_pending"). Should we update this or remove this test? Or maybe I miss something. :) Thanks.

(In reply to Boris Chiou [:boris] (away Dec 16–31) from comment #6) > Comment on attachment 8935183 [details] > Bug 1416693 - Make animation test cases that don't require asynchronous. > > https://reviewboard.mozilla.org/r/206046/#review211726 > > ::: dom/animation/test/chrome/test_animation_observers_sync.html:1224 > (Diff revision 1) > > + // Finish and pause. > > + animations[0].finish(); > > + animations[0].pause(); > > + > > + assert_equals(animations[0].playState, "paused", > > + "playState after finishing and pausing"); > > + > > + // We should have two MutationRecords for the Animation changes: > > + // one for the finish, one for the pause. > > + assert_equals_records(observer.takeRecords(), > > + [{ added: [], changed: animations, removed: [] }, > > + { added: [], changed: animations, removed: [] }], > > + "records after finish() and pause()"); > > + > > + // Call finish() again. > > + animations[0].finish(); > > + > > + assert_equals(animations[0].playState, "finished", > > + "playState after finishing from paused state"); > > + > > + assert_equals_records(observer.takeRecords(), > > + [{ added: [], changed: animations, removed: [] }], > > + "records after finish() and pause()"); > > I have a question about this: > From the original test, we call finish() and pause(), and then wait for > "aniamtion ready" (i.e Wait for the pause to complete), and then call > finish(), that's why we call this test "finish_from_pause". But now I cannot > see the "animation ready" things, so this test now looks like the same as > the test below (i.e. "finish_from_pause_pending"). Should we update this or > remove this test? Or maybe I miss something. :) Thanks. Oh right, indeed. That one was an overkill. I will leave the test case in test_animation_observers_async.html. And unfortunately the test case is one of the failure test cases caused by bug 1416966.:/ I have to solve it somehow in that bug. Anyway, thanks for the review!

Cool. Thanks for this info. :)

A final try; https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5dac4f3bf845d0c63c6a95d19a57d4267be0ed7

:glob, would you mind clearing remaining open issue count in this change https://reviewboard.mozilla.org/r/206046/diff/2#index_header ? There is no open issue but the count stays 1.

issue count fixed.

Thank you!

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ec95322bbbd9 Make transition test cases that don't require asynchronous. r=boris https://hg.mozilla.org/integration/autoland/rev/2566f76fae2d Make animation test cases that don't require asynchronous. r=boris

https://hg.mozilla.org/mozilla-central/rev/ec95322bbbd9 https://hg.mozilla.org/mozilla-central/rev/2566f76fae2d

Too late for Beta58. Mark 58 won't fix.