Bug 1049975 - Part 11: Fix mutation observer when setting effects. draft
authorBoris Chiou <boris.chiou@gmail.com>
Tue, 16 Aug 2016 20:00:35 +0800
changeset 405239 a7d08821c6bdcd19c5855a46dbe35339b664f0ad
parent 405238 1411cca0001f4fc6acf2856255ff884b5d69b426
child 405240 588622cae10d2679f93e6d5b37f36584b8947c23
push id27442
push userbmo:boris.chiou@gmail.com
push dateThu, 25 Aug 2016 04:26:27 +0000
bugs1049975
milestone51.0a1
Bug 1049975 - Part 11: Fix mutation observer when setting effects. MozReview-Commit-ID: 3td2343LFxX
dom/animation/Animation.cpp
dom/animation/test/chrome/test_animation_observers.html
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -135,42 +135,62 @@ void
 Animation::SetEffectNoUpdate(AnimationEffectReadOnly* aEffect)
 {
   RefPtr<Animation> kungFuDeathGrip(this);
 
   if (mEffect == aEffect) {
     return;
   }
 
+  AutoMutationBatchForAnimation mb(*this);
+  bool wasRelevant = mIsRelevant;
+
   if (mEffect) {
     if (!aEffect) {
       // If the new effect is null, call ResetPendingTasks before clearing
       // mEffect since ResetPendingTasks needs it to get the appropriate
       // PendingAnimationTracker.
       ResetPendingTasks();
     }
 
+    // We need to notify observers now because once we set mEffect to null
+    // we won't be able to find the target element to notify.
+    if (mIsRelevant) {
+      nsNodeUtils::AnimationRemoved(this);
+    }
+
     // Break links with the old effect and then drop it.
     RefPtr<AnimationEffectReadOnly> oldEffect = mEffect;
     mEffect = nullptr;
     oldEffect->SetAnimation(nullptr);
+
+    // The following will not do any notification because mEffect is null.
+    UpdateRelevance();
   }
 
   if (aEffect) {
     // Break links from the new effect to its previous animation, if any.
     RefPtr<AnimationEffectReadOnly> newEffect = aEffect;
     Animation* prevAnim = aEffect->GetAnimation();
     if (prevAnim) {
       prevAnim->SetEffect(nullptr);
     }
 
     // Create links with the new effect.
     mEffect = newEffect;
     mEffect->SetAnimation(this);
 
+    // Update relevance and then notify possible add or change.
+    // If the target is different, the change notification will be ignored by
+    // AutoMutationBatchForAnimation.
+    UpdateRelevance();
+    if (wasRelevant && mIsRelevant) {
+      nsNodeUtils::AnimationChanged(this);
+    }
+
     // Reschedule pending pause or pending play tasks.
     // If we have a pending animation, it will either be registered
     // in the pending animation tracker and have a null pending ready time,
     // or, after it has been painted, it will be removed from the tracker
     // and assigned a pending ready time.
     // After updating the effect we'll typically need to repaint so if we've
     // already been assigned a pending ready time, we should clear it and put
     // the animation back in the tracker.
--- a/dom/animation/test/chrome/test_animation_observers.html
+++ b/dom/animation/test/chrome/test_animation_observers.html
@@ -1789,16 +1789,118 @@ addAsyncAnimTest("set_redundant_animatio
   anim.effect.target = null;
   yield await_frame();
   assert_records([], "records after setting redundant null");
 
   anim.cancel();
   yield await_frame();
 });
 
+addAsyncAnimTest("set_null_animation_effect",
+                 { observe: div, subtree: true }, function*() {
+  var anim = div.animate({ opacity: [ 0, 1 ] },
+                         { duration: 100 * MS_PER_SEC });
+  yield await_frame();
+  assert_records([{ added: [anim], changed: [], removed: [] }],
+                 "records after animation is added");
+
+  anim.effect = null;
+  yield await_frame();
+  assert_records([{ added: [], changed: [], removed: [anim] }],
+                 "records after animation is removed");
+
+  anim.cancel();
+  yield await_frame();
+});
+
+addAsyncAnimTest("set_effect_on_null_effect_animation",
+                 { observe: div, subtree: true }, function*() {
+  var anim = new Animation();
+  anim.play();
+  anim.effect = new KeyframeEffect(div, { opacity: [ 0, 1 ] },
+                                   100 * MS_PER_SEC);
+  yield await_frame();
+  assert_records([{ added: [anim], changed: [], removed: [] }],
+                 "records after animation is added");
+
+  anim.cancel();
+  yield await_frame();
+});
+
+addAsyncAnimTest("replace_effect_targeting_on_the_same_element",
+                 { observe: div, subtree: true }, function*() {
+  var anim = div.animate({ marginLeft: [ "0px", "100px" ] },
+                         100 * MS_PER_SEC);
+  yield await_frame();
+  assert_records([{ added: [anim], changed: [], removed: [] }],
+                 "records after animation is added");
+
+  anim.effect = new KeyframeEffect(div, { opacity: [ 0, 1 ] },
+                                   100 * MS_PER_SEC);
+  yield await_frame();
+  assert_records([{ added: [], changed: [anim], removed: [] }],
+                 "records after replace effects");
+
+  anim.cancel();
+  yield await_frame();
+});
+
+addAsyncAnimTest("replace_effect_targeting_on_the_same_element_not_in_effect",
+                 { observe: div, subtree: true }, function*() {
+  var anim = div.animate({ marginLeft: [ "0px", "100px" ] },
+                         100 * MS_PER_SEC);
+  yield await_frame();
+  assert_records([{ added: [anim], changed: [], removed: [] }],
+                 "records after animation is added");
+
+  anim.currentTime = 60 * MS_PER_SEC;
+  yield await_frame();
+  assert_records([{ added: [], changed: [anim], removed: [] }],
+                 "records after animation is changed");
+
+  anim.effect = new KeyframeEffect(div, { opacity: [ 0, 1 ] },
+                                   50 * MS_PER_SEC);
+  yield await_frame();
+  assert_records([{ added: [], changed: [], removed: [anim] }],
+                 "records after replacing effects");
+
+  anim.cancel();
+  yield await_frame();
+});
+
+addAsyncAnimTest("set_effect_with_previous_animation",
+                 { observe: div, subtree: true }, function*() {
+  var child = document.createElement("div");
+  div.appendChild(child);
+
+  var anim1 = div.animate({ marginLeft: [ "0px", "50px" ] },
+                          100 * MS_PER_SEC);
+  var anim2 = child.animate({ marginLeft: [ "0px", "100px" ] },
+                              50 * MS_PER_SEC);
+  yield await_frame();
+  assert_records([{ added: [anim1], changed: [], removed: [] },
+                  { added: [anim2], changed: [], removed: [] }],
+                 "records after animation is added");
+
+  // After setting a new effect, we remove the current animation, anim1, because
+  // it is no longer attached to |div|, and then remove the previous animation,
+  // anim2. Finally, add back the anim1 which is in effect on |child| now.
+  // In addition, we sort them by tree order and they are batched.
+  anim1.effect = anim2.effect;
+  yield await_frame();
+  assert_records([{ added: [], changed: [], removed: [anim1] },       // div
+                  { added: [anim1], changed: [], removed: [anim2] }], // child
+                 "records after animation effects are changed");
+
+  anim1.cancel();
+  anim2.cancel();
+  child.remove();
+  yield await_frame();
+});
+
 // Run the tests.
 SimpleTest.requestLongerTimeout(2);
 SimpleTest.waitForExplicitFinish();
 
 runAllAsyncTests().then(function() {
   SimpleTest.finish();
 }, function(aError) {
   ok(false, "Something failed: " + aError);