Bug 1361234 - Restore test_deferred_start.html to actually testing delay; r=hiro
authorBrian Birtles <birtles@gmail.com>
Tue, 02 May 2017 16:40:23 +0900
changeset 406506 4317e4c2650579f892a2d64bceda9a19b09eda47
parent 406505 dca52e4ed76197ec3e3629e659020c3d5c8d81bb
child 406507 438c1291a77ab7b753fac99123ea3a7de6b57fb2
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1361234
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1361234 - Restore test_deferred_start.html to actually testing delay; r=hiro Once upon a time[1] a test was added to test_deferred_start.html to test the code path where we establish the start time to pass to the compositor when resolving a pending animation. Later[2], however, we encountered intermittent failures on B2G so we made it stop waiting on animation.ready and use waitForPaints instead. There were two problems with this, however. Firstly, waiting for paints often means that extra paints are processed such that we end up updating the start time on the layer using a different code path and masking any potential bugs in the code path under test. Secondly, when we made this change we replaced: return animation.ready.then(function() { /* test code */ }) with: return waitForPaints(function() { /* test code */ }) And sadly that means that 'test code' never runs. Of course, what we meant to write was: return waitForPaints().then(function() { /* test code */ }) As a result, when we later broke the code path under test no one noticed. This patch restores the test so that it tests what it intends to (and currently fails, at least most of the time). [1] https://hg.mozilla.org/mozilla-central/rev/79cac8c71159 [2] https://hg.mozilla.org/mozilla-central/rev/986b18fdfdba [3] https://hg.mozilla.org/mozilla-central/rev/b66b75c2d042101b954e6423438cc07955c2b9bd MozReview-Commit-ID: 1iMWLQP6zae
dom/animation/test/mochitest.ini
dom/animation/test/mozilla/file_deferred_start.html
dom/animation/test/testcommon.js
--- a/dom/animation/test/mochitest.ini
+++ b/dom/animation/test/mochitest.ini
@@ -96,16 +96,17 @@ support-files =
 [css-transitions/test_event-dispatch.html]
 [css-transitions/test_keyframeeffect-getkeyframes.html]
 [css-transitions/test_pseudoElement-get-animations.html]
 [css-transitions/test_setting-effect.html]
 [document-timeline/test_document-timeline.html]
 [document-timeline/test_request_animation_frame.html]
 [mozilla/test_cubic_bezier_limits.html]
 [mozilla/test_deferred_start.html]
+skip-if = true # Bug 1361234 (mostly fails)
 [mozilla/test_disable_animations_api_core.html]
 [mozilla/test_disabled_properties.html]
 [mozilla/test_discrete-animations.html]
 [mozilla/test_document-timeline-origin-time-range.html]
 [mozilla/test_hide_and_show.html]
 [mozilla/test_set-easing.html]
 [mozilla/test_spacing_property_order.html]
 [mozilla/test_transform_limits.html]
--- a/dom/animation/test/mozilla/file_deferred_start.html
+++ b/dom/animation/test/mozilla/file_deferred_start.html
@@ -1,18 +1,14 @@
 <!doctype html>
 <meta charset=utf-8>
 <script src="../testcommon.js"></script>
 <script src="/tests/SimpleTest/paint_listener.js"></script>
 <style>
 @keyframes empty { }
-@keyframes animTransform {
-  from { transform: translate(0px); }
-  to { transform: translate(100px); }
-}
 .target {
   /* Element needs geometry to be eligible for layerization */
   width: 100px;
   height: 100px;
   background-color: white;
 }
 </style>
 <body>
@@ -83,39 +79,53 @@ promise_test(function(t) {
 // SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh here since that takes
 // us through a different code path.
 promise_test(function(t) {
   // This test only applies to compositor animations
   if (!isOMTAEnabled()) {
     return;
   }
 
-  // Setup animation
-  var div = addDiv(t);
+  const div = addDiv(t);
   div.classList.add('target');
-  div.style.animation = 'animTransform 100s -50s forwards';
-  var animation = div.getAnimations()[0];
 
-  return waitForPaints(function() {
+  // As with the above test, any stray paints can cause this test to produce
+  // a false negative (that is, pass when it should fail). To avoid this we
+  // first wait for the document to load, then wait until it is idle, then wait
+  // for paints and only then do we commence the test. Even doing that, this
+  // test can sometimes pass when it should not due to a stray paint. Most of
+  // the time, however, it will correctly fail so hopefully even if we do
+  // occasionally produce a false negative on one platform, another platform
+  // will fail as expected.
+  return waitForDocLoad().then(() => waitForIdle())
+  .then(() => waitForPaints())
+  .then(() => {
+    div.animate({ transform: [ 'translate(0px)', 'translate(100px)' ] },
+                { duration: 400 * MS_PER_SEC,
+                  delay: -200 * MS_PER_SEC });
+    return waitForPaints();
+  }).then(() => {
     var transformStr =
       SpecialPowers.DOMWindowUtils.getOMTAStyle(div, 'transform');
 
     var matrixComponents =
       transformStr.startsWith('matrix(')
       ? transformStr.substring('matrix('.length, transformStr.length-1)
                     .split(',')
                     .map(component => Number(component))
       : [];
     assert_equals(matrixComponents.length, 6,
                   'Got a valid transform matrix on the compositor'
                   + ' (got: "' + transformStr + '")');
 
-    // If the delay has been applied correctly we should be at least
-    // half-way through the animation
-    assert_true(matrixComponents[4] >= 50,
-                'Animation is at least half-way through on the compositor'
-                + ' (got translation of ' + matrixComponents[4] + ')');
+    // If the delay has been applied we should be about half-way through
+    // the animation. However, if we applied it twice we will be at the
+    // end of the animation already so check that we are roughly half way
+    // through.
+    const translateX = matrixComponents[4];
+    assert_between_inclusive(translateX, 40, 75,
+        'Animation is about half-way through on the compositor');
   });
 }, 'Starting an animation with a delay starts from the correct point');
 
 done();
 </script>
 </body>
--- a/dom/animation/test/testcommon.js
+++ b/dom/animation/test/testcommon.js
@@ -230,16 +230,25 @@ function waitForAnimationFrames(frameCou
         window.requestAnimationFrame(handleFrame); // wait another frame
       }
     }
     window.requestAnimationFrame(handleFrame);
   });
 }
 
 /**
+ * Promise wrapper for requestIdleCallback.
+ */
+function waitForIdle() {
+  return new Promise(resolve => {
+    requestIdleCallback(resolve);
+  });
+}
+
+/**
  * Wrapper that takes a sequence of N animations and returns:
  *
  *   Promise.all([animations[0].ready, animations[1].ready, ... animations[N-1].ready]);
  */
 function waitForAllAnimations(animations) {
   return Promise.all(animations.map(function(animation) {
     return animation.ready;
   }));