Bug 1474213 - Add an assertion in DocumentTimeline::NotifyAnimationUpdated checking the animation's timeline document is the same document of the DocumentTimeline. r?birtles draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Mon, 09 Jul 2018 07:16:37 +0900
changeset 815507 8a52143a30aa3e40d6f56606c277f3f3608ba30a
parent 815506 c66b824525e0d38d7f213cc09a3b445eaec2a119
child 815508 273539e248701b9b1ada5a32b93152e512cb365f
push id115520
push userbmo:hikezoe@mozilla.com
push dateSun, 08 Jul 2018 22:24:35 +0000
reviewersbirtles
bugs1474213
milestone63.0a1
Bug 1474213 - Add an assertion in DocumentTimeline::NotifyAnimationUpdated checking the animation's timeline document is the same document of the DocumentTimeline. r?birtles MozReview-Commit-ID: DqAtIwJ07dM
dom/animation/Animation.h
dom/animation/DocumentTimeline.cpp
--- a/dom/animation/Animation.h
+++ b/dom/animation/Animation.h
@@ -390,16 +390,21 @@ public:
    */
   virtual void MaybeQueueCancelEvent(const StickyTimeDuration& aActiveTime) {};
 
   /**
    * Returns the document associated with the animation target element.
    */
   nsIDocument* GetRenderedDocument() const;
 
+  /**
+   * Returns the document associated with the timeline.
+   */
+  nsIDocument* GetTimelineDocument() const;
+
 protected:
   void SilentlySetCurrentTime(const TimeDuration& aNewCurrentTime);
   void CancelNoUpdate();
   void PlayNoUpdate(ErrorResult& aRv, LimitBehavior aLimitBehavior);
   void ResumeAt(const TimeDuration& aReadyTime);
   void PauseAt(const TimeDuration& aReadyTime);
   void FinishPendingAt(const TimeDuration& aReadyTime)
   {
@@ -522,18 +527,16 @@ protected:
 
     static constexpr StickyTimeDuration zeroDuration = StickyTimeDuration();
     return std::max(
       std::min((EffectEnd() - mEffect->SpecifiedTiming().Delay()),
                aActiveDuration),
       zeroDuration);
   }
 
-  nsIDocument* GetTimelineDocument() const;
-
   RefPtr<AnimationTimeline> mTimeline;
   RefPtr<AnimationEffect> mEffect;
   // The beginning of the delay period.
   Nullable<TimeDuration> mStartTime; // Timeline timescale
   Nullable<TimeDuration> mHoldTime;  // Animation timescale
   Nullable<TimeDuration> mPendingReadyTime; // Timeline timescale
   Nullable<TimeDuration> mPreviousCurrentTime; // Animation timescale
   double mPlaybackRate;
--- a/dom/animation/DocumentTimeline.cpp
+++ b/dom/animation/DocumentTimeline.cpp
@@ -128,16 +128,21 @@ DocumentTimeline::ToTimelineTime(const T
                   - timing->GetNavigationStartTimeStamp()
                   - mOriginTime);
   return result;
 }
 
 void
 DocumentTimeline::NotifyAnimationUpdated(Animation& aAnimation)
 {
+  MOZ_ASSERT(aAnimation.GetTimelineDocument() &&
+               aAnimation.GetTimelineDocument() == mDocument,
+             "Should be used for animations that the animation's timeline "
+             "document is the same document of this DocumentTimeline");
+
   AnimationTimeline::NotifyAnimationUpdated(aAnimation);
 
   if (!mIsObservingRefreshDriver) {
     nsRefreshDriver* refreshDriver = GetRefreshDriver();
     if (refreshDriver) {
       MOZ_ASSERT(isInList(),
                 "We should not register with the refresh driver if we are not"
                 " in the document's list of timelines");
@@ -268,16 +273,23 @@ DocumentTimeline::NotifyRefreshDriverDes
   }
 
   DisconnectRefreshDriver(aDriver);
 }
 
 void
 DocumentTimeline::RemoveAnimation(Animation* aAnimation)
 {
+  // I wondered we can assert here as well as we do in NotifyAnimationUpdated,
+  // but this function is also called from MostRecentRefreshTimeUpdated() if
+  // the animation timeline has been changed.
+  // MOZ_ASSERT(aAnimation->GetTimelineDocument() &&
+  //              aAnimation->GetTimelineDocument() == mDocument,
+  //           "Should be used for animations that the animation's timeline "
+  //           "document is the same document of this DocumentTimeline");
   AnimationTimeline::RemoveAnimation(aAnimation);
 
   if (mIsObservingRefreshDriver && mAnimations.IsEmpty()) {
     UnregisterFromRefreshDriver();
   }
 }
 
 TimeStamp