Bug 1333539 - Part 1: Do not try to send animations without timeline. r=birtles
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Thu, 02 Feb 2017 15:11:15 +0900
changeset 469863 205c3c6c123fe277268770e3bdf082f89ec47ec5
parent 469862 d6fd02e91afb14f51ee238032533ddb7c3e468e9
child 469864 a8f1dbc491d8bf47d6d38ea396e0d63da8b71410
push id43881
push userbmo:gps@mozilla.com
push dateThu, 02 Feb 2017 23:49:03 +0000
reviewersbirtles
bugs1333539
milestone54.0a1
Bug 1333539 - Part 1: Do not try to send animations without timeline. r=birtles 1333539-2.html is the test case that crashes without |!aAnimation->GetTimeline()| check in AddAnimationForProperty(). MozReview-Commit-ID: 8UxYL8o63E1
dom/animation/Animation.h
dom/animation/test/crashtests/1333539-1.html
dom/animation/test/crashtests/1333539-2.html
dom/animation/test/crashtests/crashtests.list
layout/painting/nsDisplayList.cpp
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -274,16 +274,17 @@ public:
   bool IsInEffect() const
   {
     return GetEffect() && GetEffect()->IsInEffect();
   }
 
   bool IsPlaying() const
   {
     return mPlaybackRate != 0.0 &&
+           mTimeline &&
            (PlayState() == AnimationPlayState::Running ||
             mPendingState == PendingState::PlayPending);
   }
 
   bool ShouldBeSynchronizedWithMainThread(
     nsCSSPropertyID aProperty,
     const nsIFrame* aFrame,
     AnimationPerformanceWarning::Type& aPerformanceWarning) const;
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1333539-1.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<meta charset="UTF-8">
+<script>
+window.onload = function(){
+  let body = document.getElementsByTagName("body")[0];
+  let target = document.createElement("div");
+  let anim1 = new Animation();
+  let anim2 = new Animation();
+  let effect = new KeyframeEffect(target, { opacity: [ 0, 1 ] }, 1000);
+  body.appendChild(target);
+  target.appendChild(document.createElement("meter"));
+  anim1.startTime = 88;
+  anim1.timeline = null;
+  anim1.pause();
+  anim1.effect = effect;
+  anim2.effect = effect;
+  anim1.effect = effect;
+
+  Promise.all([anim1.ready, anim2.ready]).then(function() {
+    document.documentElement.classList.remove("reftest-wait");
+  });
+};
+</script>
+</head>
+<body></body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/animation/test/crashtests/1333539-2.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="UTF-8">
+<style>
+div {
+  width: 100px;
+  height: 100px;
+  background-color: blue;
+}
+</style>
+<script>
+window.onload = function(){
+  let body = document.getElementsByTagName("body")[0];
+  let target = document.createElement("div");
+  let anim1 = new Animation();
+  let anim2 = new Animation();
+  let effect = new KeyframeEffect(target, { opacity: [ 0, 1 ] }, 1000);
+  body.appendChild(target);
+  anim1.startTime = 88;
+  anim1.timeline = null;
+  anim1.pause();
+  anim1.effect = effect;
+  anim2.effect = effect;
+  anim1.effect = effect;
+  // Put another opacity animation on the top of the effect stack so that we
+  // try to send a lower priority animation that has no timeline to the
+  // compositor.
+  let anim3 = target.animate({ opacity : [ 1, 0 ] }, 1000);
+
+  Promise.all([anim1.ready, anim2.ready, anim2.ready]).then(function() {
+    document.documentElement.classList.remove("reftest-wait");
+  });
+};
+</script>
+</head>
+<body></body>
+</html>
--- a/dom/animation/test/crashtests/crashtests.list
+++ b/dom/animation/test/crashtests/crashtests.list
@@ -16,9 +16,11 @@ pref(dom.animations-api.core.enabled,tru
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-1.html # bug 1311257
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-2.html # bug 1311257 and bug 1311257
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1323114-1.html # bug 1324690 and bug 1311257
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1323114-2.html # bug 1324690
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1325193-1.html # bug 1311257
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330190-1.html # bug 1311257
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330190-2.html # bug 1311257
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1330513-1.html # bug 1311257
+skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1333539-1.html
+skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1333539-2.html
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1333418-1.html # bug 1311257
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -512,17 +512,17 @@ AddAnimationForProperty(nsIFrame* aFrame
     // an ElementPropertyTransition.
     aAnimation->GetEffect()->AsTransition()->
       UpdateStartValueFromReplacedTransition();
   }
 
   const ComputedTiming computedTiming =
     aAnimation->GetEffect()->GetComputedTiming();
   Nullable<TimeDuration> startTime = aAnimation->GetCurrentOrPendingStartTime();
-  animation->startTime() = startTime.IsNull()
+  animation->startTime() = startTime.IsNull() || !aAnimation->GetTimeline()
                            ? TimeStamp()
                            : aAnimation->GetTimeline()->
                               ToTimeStamp(startTime.Value());
   animation->holdTime() = aAnimation->GetCurrentTime().Value();
 
   animation->delay() = timing.mDelay;
   animation->endDelay() = timing.mEndDelay;
   animation->duration() = computedTiming.mDuration;