Bug 927349 part 29 - Disable failing browser_555547.js on Windows opt - pushed again without backing out parts 22-29 at the same time; r=me

In https://hg.mozilla.org/integration/mozilla-inbound/rev/3ab2ff9b9f94
I accidentally pushed part 29 along with a backout for parts 22-29. This patch
reverts that change.

If this changset causes problems, the following changesets should be backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/477f46897b0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c345954bce5
https://hg.mozilla.org/integration/mozilla-inbound/rev/642e400b22a9
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bdf7c2279fa
https://hg.mozilla.org/integration/mozilla-inbound/rev/79cac8c71159
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfe757e478d5
https://hg.mozilla.org/integration/mozilla-inbound/rev/eaf834051cbd

browser/components/places/tests/browser/browser.ini

@@ -48,7 +48,7 @@ skip-if = e10s # Bug 933103 - mochitest's EventUtils.synthesizeMouse functions n
 [browser_toolbar_migration.js]
 [browser_library_batch_delete.js]
 [browser_555547.js]
-skip-if = e10s
+skip-if = e10s || (os == "win" && !debug) # Bug 1115276
 [browser_416459_cut.js]
 skip-if = e10s # Bug ?????? - clipboard operations don't seem to work in this test?
 [browser_library_downloads.js]

dom/animation/AnimationPlayer.cpp

@@ -133,25 +133,45 @@ AnimationPlayer::SetSource(Animation* aSource)
 void
 AnimationPlayer::Tick()
 {
+  // FIXME (bug 1112969): Check if we are pending but have lost access to the
+  // pending player tracker. If that's the case we should probably trigger the
+  // animation now.
+
   UpdateSourceContent();
 }
 
 void
 AnimationPlayer::StartNow()
 {
-  // Currently we only expect this method to be called when we are in the
-  // middle of initiating/resuming playback so we should have an unresolved
-  // start time to update and a fixed current time to seek to.
-  MOZ_ASSERT(mStartTime.IsNull() && !mHoldTime.IsNull(),
-             "Resolving the start time but we don't appear to be waiting"
-             " to begin playback");
+  // This method is only expected to be called for an animation that is
+  // waiting to play. We can easily adapt it to handle other states
+  // but it's currently not necessary.
+  MOZ_ASSERT(PlayState() == AnimationPlayState::Pending,
+             "Expected to start a pending player");
+  MOZ_ASSERT(!mHoldTime.IsNull(),
+             "A player in the pending state should have a resolved hold time");
 
   Nullable<TimeDuration> readyTime = mTimeline->GetCurrentTime();
-  // Bug 1096776: Once we support disappearing or inactive timelines we
-  // will need special handling here.
+
+  // FIXME (bug 1096776): If readyTime.IsNull(), we should return early here.
+  // This will leave mIsPending = true but the caller will remove us from the
+  // PendingPlayerTracker if we were added there.
+  // Then, in Tick(), if we have:
+  // - a resolved timeline, and
+  // - mIsPending = true, and
+  // - *no* document or we are *not* in the PendingPlayerTracker
+  // then we should call StartNow.
+  //
+  // For now, however, we don't support inactive/missing timelines so
+  // |readyTime| should be resolved.
   MOZ_ASSERT(!readyTime.IsNull(), "Missing or inactive timeline");
+
   mStartTime.SetValue(readyTime.Value() - mHoldTime.Value());
   mHoldTime.SetNull();
+  mIsPending = false;
+
+  UpdateSourceContent();
+  PostUpdate();
 
   if (mReady) {
     mReady->MaybeResolve(this);
@@ -246,7 +266,27 @@ AnimationPlayer::DoPlay()
   // Clear ready promise. We'll create a new one lazily.
   mReady = nullptr;
 
-  StartNow();
+  mIsPending = true;
+
+  nsIDocument* doc = GetRenderedDocument();
+  if (!doc) {
+    // If we have no rendered document (e.g. because the source content's
+    // target element is orphaned), then treat the animation as ready and
+    // start it immediately. It is probably preferable to make playing
+    // *always* asynchronous (e.g. by setting some additional state that
+    // marks this player as pending and queueing a runnable to resolve the
+    // start time). That situation, however, is currently rare enough that
+    // we don't bother for now.
+    StartNow();
+    return;
+  }
+
+  PendingPlayerTracker* tracker = doc->GetOrCreatePendingPlayerTracker();
+  tracker->AddPlayPending(*this);
+
+  // We may have updated the current time when we set the hold time above
+  // so notify source content.
+  UpdateSourceContent();
 }
 
 void

dom/animation/AnimationPlayer.h

@@ -95,8 +95,27 @@ public:
   void SetSource(Animation* aSource);
   void Tick();
 
-  // Sets the start time of a player that is waiting to play to the current
-  // time of its timeline.
+  /**
+   * Sets the start time of a player that is waiting to play to the current
+   * time of its timeline.
+   *
+   * This will reset the pending flag if the call succeeded. The caller is
+   * responsible for removing the player from the PendingPlayerTracker though.
+   *
+   * Typically, when a player is played, it does not start immediately but is
+   * added to a table of pending players on the document of its source content.
+   * In the meantime it sets its hold time to the time from which should
+   * begin playback.
+   *
+   * When the document finishes painting, any pending players in its table
+   * are started by calling this method.
+   *
+   * This approach means that any setup time required for performing the
+   * initial paint of an animation such as layerization is not deducted from
+   * the running time of the animation. Without this we can easily drop the
+   * first few frames of an animation, or, on slower devices, the whole
+   * animation.
+   */
   void StartNow();
   void Cancel();
 

dom/animation/test/chrome/test_running_on_compositor.html

@@ -44,10 +44,7 @@ var omtaEnabled = SpecialPowers.DOMWindowUtils.layerManagerRemote &&
 div.style.animation = 'anim 100s';
 var player = div.getAnimationPlayers()[0];
 
-// Wait so that animation can be set up.
-// FIXME: When we implement the AnimationPlayer.ready promise we should wait
-// on that here.
-window.requestAnimationFrame(function() {
+player.ready.then(function() {
   is(player.isRunningOnCompositor, omtaEnabled,
      'AnimationPlayer reports that it is running on the compositor'
      + ' during playback');
@@ -55,7 +52,8 @@ window.requestAnimationFrame(function() {
   div.style.animationPlayState = 'paused';
   window.getComputedStyle(div).animationPlayState;
 
-  // FIXME: Likewise, we should wait on AnimationPlayer.ready here too.
+  // FIXME: When we implement deferred pausing (bug 1109390), we should wait
+  // on player.ready here.
   window.requestAnimationFrame(function() {
     is(player.isRunningOnCompositor, false,
        'AnimationPlayer reports that it is NOT running on the compositor'

dom/animation/test/css-animations/test_animation-player-playstate.html

@@ -16,8 +16,7 @@ async_test(function(t) {
   div.style.animation = 'anim 1000s';
 
   var player = div.getAnimationPlayers()[0];
-  assert_equals(player.playState, 'running');
-  // Bug 927349: Check for pending state here
+  assert_equals(player.playState, 'pending');
 
   player.ready.then(t.step_func(function() {
     assert_equals(player.playState, 'running');
@@ -52,8 +51,9 @@ test(function(t) {
   var player = div.getAnimationPlayers()[0];
   div.style.animationPlayState = 'running';
   // This test also checks that calling playState flushes style
-  // Bug 927349: Make this check for 'pending'
-  assert_equals(player.playState, 'running');
+  assert_equals(player.playState, 'pending',
+                'Player.playState reports pending after updating'
+                + ' animation-play-state (got: ' + player.playState + ')');
 }, 'Player.playState updates when resumed by setting style');
 
 </script>

dom/animation/test/css-animations/test_animation-player-ready.html

@@ -75,4 +75,62 @@ async_test(function(t) {
   });
 }, 'The ready promise is fulfilled with its AnimationPlayer');
 
+async_test(function(t) {
+  var div = addDiv(t);
+
+  // Set up pending animation
+  div.style.animation = 'abc 100s';
+  var player = div.getAnimationPlayers()[0];
+  assert_equals(player.playState, 'pending', 'Player is initially pending');
+
+  // Set up listeners on ready promise
+  player.ready.then(t.step_func(function() {
+    assert_unreached('ready promise is fulfilled');
+  })).catch(t.step_func(function(err) {
+    assert_equals(err.name, 'AbortError',
+                  'ready promise is rejected with AbortError');
+    assert_equals(player.playState, 'idle',
+                  'Player is idle after animation was cancelled');
+  })).then(t.step_func(function() {
+    t.done();
+  }));
+
+  // Now cancel the animation and flush styles
+  div.style.animation = '';
+  window.getComputedStyle(div).animation;
+
+}, 'ready promise is rejected when an animation is cancelled by resetting'
+   + ' the animation property');
+
+async_test(function(t) {
+  var div = addDiv(t);
+
+  // As before, but this time instead of removing all animations, simply update
+  // the list of animations. At least for Firefox, updating is a different
+  // code path.
+
+  // Set up pending animation
+  div.style.animation = 'abc 100s';
+  var player = div.getAnimationPlayers()[0];
+  assert_equals(player.playState, 'pending', 'Player is initially pending');
+
+  // Set up listeners on ready promise
+  player.ready.then(t.step_func(function() {
+    assert_unreached('ready promise was fulfilled');
+  })).catch(t.step_func(function(err) {
+    assert_equals(err.name, 'AbortError',
+                  'ready promise is rejected with AbortError');
+    assert_equals(player.playState, 'idle',
+                  'Player is idle after animation was cancelled');
+  })).then(t.step_func(function() {
+    t.done();
+  }));
+
+  // Now update the animation and flush styles
+  div.style.animation = 'def 100s';
+  window.getComputedStyle(div).animation;
+
+}, 'ready promise is rejected when an animation is cancelled by updating'
+   + ' the animation property');
+
 </script>

dom/animation/test/css-transitions/test_animation-player-ready.html

@@ -35,4 +35,36 @@ async_test(function(t) {
 }, 'A new ready promise is created each time play() is called'
    + ' the animation property');
 
+async_test(function(t) {
+  var div = addDiv(t);
+
+  // Set up pending transition
+  div.style.transform = 'translate(0px)';
+  window.getComputedStyle(div).transform;
+  div.style.transition = 'transform 100s';
+  div.style.transform = 'translate(10px)';
+  window.getComputedStyle(div).transform;
+
+  var player = div.getAnimationPlayers()[0];
+  assert_equals(player.playState, 'pending', 'Player is initially pending');
+
+  // Set up listeners on ready promise
+  player.ready.then(t.step_func(function() {
+    assert_unreached('ready promise was fulfilled');
+  })).catch(t.step_func(function(err) {
+    assert_equals(err.name, 'AbortError',
+                  'ready promise is rejected with AbortError');
+    assert_equals(player.playState, 'idle',
+                  'Player is idle after transition was cancelled');
+  })).then(t.step_func(function() {
+    t.done();
+  }));
+
+  // Now remove transform from transition-property and flush styles
+  div.style.transitionProperty = 'none';
+  window.getComputedStyle(div).transitionProperty;
+
+}, 'ready promise is rejected when a transition is cancelled by updating'
+   + ' transition-property');
+
 </script>

dom/animation/test/mochitest.ini

@@ -18,3 +18,5 @@ skip-if = buildapp == 'mulet'
 [css-transitions/test_animation-target.html]
 [css-transitions/test_element-get-animation-players.html]
 skip-if = buildapp == 'mulet'
+[mozilla/test_deferred_start.html]
+skip-if = buildapp == 'mulet' || (buildapp == 'b2g' && toolkit != 'gonk') # bug 1113425

dom/animation/test/mozilla/test_deferred_start.html

@@ -0,0 +1,115 @@
+<!doctype html>
+<meta charset=utf-8>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="../testcommon.js"></script>
+<div id="log"></div>
+<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>
+<script>
+'use strict';
+
+function waitForDocLoad() {
+  return new Promise(function(resolve, reject) {
+    if (document.readyState === 'complete') {
+      resolve();
+    } else {
+      window.addEventListener('load', resolve);
+    }
+  });
+}
+
+async_test(function(t) {
+  var div = addDiv(t);
+  var cs = window.getComputedStyle(div);
+
+  // Test that empty animations actually start.
+  //
+  // Normally we tie the start of animations to when their first frame of
+  // the animation is rendered. However, for animations that don't actually
+  // trigger a paint (e.g. because they are empty, or are animating something
+  // that doesn't render or is offscreen) we want to make sure they still
+  // start.
+  //
+  // Before we start, wait for the document to finish loading. This is because
+  // during loading we will have other paint events taking place which might,
+  // by luck, happen to trigger animations that otherwise would not have been
+  // triggered, leading to false positives.
+  //
+  // As a result, it's better to wait until we have a more stable state before
+  // continuing.
+  var promiseCallbackDone = false;
+  waitForDocLoad().then(function() {
+    div.style.animation = 'empty 1000s';
+    var player = div.getAnimationPlayers()[0];
+
+    player.ready.then(function() {
+      promiseCallbackDone = true;
+    }).catch(function() {
+      assert_unreached('ready promise was rejected');
+    });
+
+  }).then(waitForFrame).then(t.step_func(function() {
+    assert_true(promiseCallbackDone,
+                'ready promise callback was called before the next'
+                + ' requestAnimationFrame callback');
+    t.done();
+  }));
+}, 'AnimationPlayer.ready is resolved for an empty animation');
+
+// Test that compositor animations with delays get synced correctly
+//
+// NOTE: It is important that we DON'T use
+// SpecialWindows.DOMWindowUtils.advanceTimeAndRefresh here since that takes
+// us through a different code path.
+async_test(function(t) {
+  // This test only applies to compositor animations
+  const OMTAPrefKey = 'layers.offmainthreadcomposition.async-animations';
+  var omtaEnabled = SpecialPowers.DOMWindowUtils.layerManagerRemote &&
+                    SpecialPowers.getBoolPref(OMTAPrefKey);
+  if (!omtaEnabled) {
+    t.done();
+    return;
+  }
+
+  // Setup animation
+  var div = addDiv(t);
+  div.classList.add('target');
+  div.style.animation = 'animTransform 100s -50s forwards';
+  var player = div.getAnimationPlayers()[0];
+
+  player.ready.then(t.step_func(function() {
+    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] + ')');
+    t.done();
+  }));
+}, 'Starting an animation with a delay starts from the correct point');
+
+</script>

layout/base/RestyleManager.cpp

@@ -2615,7 +2615,7 @@ ElementRestyler::Restyle(nsRestyleHint aRestyleHint)
 
   nsRestyleHint hintToRestore = nsRestyleHint(0);
   if (mContent && mContent->IsElement() &&
-      // If we're we're resolving from the root of the frame tree (which
+      // If we're resolving from the root of the frame tree (which
       // we do in DoRebuildAllStyleData), we need to avoid getting the
       // root's restyle data until we get to its primary frame, since
       // it's the primary frame that has the styles for the root element
@@ -2636,12 +2636,6 @@ ElementRestyler::Restyle(nsRestyleHint aRestyleHint)
     }
   }
 
-  // Some changes to animations don't affect the computed style and yet still
-  // require the layer to be updated. For example, pausing an animation via
-  // the Web Animations API won't affect an element's style but still
-  // requires us to pull the animation off the layer.
-  AddLayerChangesForAnimation();
-
   // If we are restyling this frame with eRestyle_Self or weaker hints,
   // we restyle children with nsRestyleHint(0).  But we pass the
   // eRestyle_ChangeAnimationPhaseDescendants and eRestyle_ForceDescendants
@@ -2694,6 +2688,19 @@ ElementRestyler::Restyle(nsRestyleHint aRestyleHint)
     f = GetNextContinuationWithSameStyle(f, oldContext, &haveMoreContinuations);
   }
 
+  // Some changes to animations don't affect the computed style and yet still
+  // require the layer to be updated. For example, pausing an animation via
+  // the Web Animations API won't affect an element's style but still
+  // requires us to pull the animation off the layer.
+  //
+  // Although we only expect this code path to be called when computed style
+  // is not changing, we can sometimes reach this at the end of a transition
+  // when the animated style is being removed. Since
+  // AddLayerChangesForAnimation checks if mFrame has a transform style or not,
+  // we need to call it *after* calling RestyleSelf to ensure the animated
+  // transform has been removed first.
+  AddLayerChangesForAnimation();
+
   if (haveMoreContinuations && hintToRestore) {
     // If we have more continuations with different style (e.g., because
     // we're inside a ::first-letter or ::first-line), put the restyle

layout/style/nsTransitionManager.cpp

@@ -540,8 +540,12 @@ nsTransitionManager::ConsiderStartingTransition(
   segment.mTimingFunction.Init(tf);
 
   nsRefPtr<CSSTransitionPlayer> player = new CSSTransitionPlayer(timeline);
-  player->PlayFromStyle();
+  // The order of the following two calls is important since PlayFromStyle
+  // will add the player to the PendingPlayerTracker of its source content's
+  // document. When we come to make source writeable (bug 1049975) we should
+  // remove this dependency.
   player->SetSource(pt);
+  player->PlayFromStyle();
 
   if (!aElementTransitions) {
     aElementTransitions =

toolkit/devtools/server/actors/animation.js

@@ -178,6 +178,7 @@ let AnimationPlayerActor = ActorClass({
    */
   play: method(function() {
     this.player.play();
+    return this.player.ready;
   }, {
     request: {},
     response: {}

toolkit/devtools/server/tests/browser/browser_animation_actors_04.js

@@ -38,8 +38,11 @@ function* playStateIsUpdatedDynamically(walker, front) {
 
   let [player] = yield front.getAnimationPlayersForNode(node);
 
-  is(player.initialState.playState, "running",
-    "The playState is running while the transition is running");
+  // Bug 1113091 - We should wait for the player to become ready then
+  // test for the "running" state only
+  ok(player.initialState.playState == "running" ||
+     player.initialState.playState == "pending",
+    "The playState is running or pending while the transition is running");
 
   info("Wait until the animation stops (more than 1000ms)");
   yield wait(1500); // Waiting 1.5sec for good measure