Bug 1333539 - Part 1: Do not try to send animations without timeline. r=birtles a=gchang
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Thu, 02 Feb 2017 15:11:15 +0900
changeset 375985 82c75fd3a2de796351296592c459ab4aa4cd0baf
parent 375984 644301c96f60cc9f7813afd53be1f349bab78242
child 375986 72c673aa7da0276068c7013c5f3806be9017ccec
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbirtles, gchang
bugs1333539
milestone53.0a2
Bug 1333539 - Part 1: Do not try to send animations without timeline. r=birtles a=gchang 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
@@ -17,8 +17,10 @@ skip-if(stylo) pref(dom.animations-api.c
 skip-if(stylo) pref(dom.animations-api.core.enabled,true) load 1322291-2.html # bug 1311257
 asserts-if(stylo,0-5) pref(dom.animations-api.core.enabled,true) load 1323114-1.html # bug 1324690
 asserts-if(stylo,0-5) 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 1333418-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
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -480,17 +480,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;