Bug 916602 (Part 1) - Avoid null timestamp assertion in FrameAnimator::GetCurrentImgFrameEndTime. r=seth

image/src/Image.cpp

@@ -136,7 +136,6 @@ ImageResource::EvaluateAnimation()
     mAnimating = NS_SUCCEEDED(rv);
   } else if (mAnimating && !ShouldAnimate()) {
     StopAnimation();
-    mAnimating = false;
   }
 }
 

image/src/RasterImage.cpp

@@ -1436,12 +1436,13 @@ RasterImage::StartAnimation()
   EnsureAnimExists();
 
   imgFrame* currentFrame = GetCurrentImgFrame();
-  if (currentFrame) {
-    if (currentFrame->GetTimeout() < 0) { // -1 means display this frame forever
-      mAnimationFinished = true;
-      return NS_ERROR_ABORT;
-    }
+  // A timeout of -1 means we should display this frame forever.
+  if (currentFrame && currentFrame->GetTimeout() < 0) {
+    mAnimationFinished = true;
+    return NS_ERROR_ABORT;
+  }
 
+  if (mAnim) {
     // We need to set the time that this initial frame was first displayed, as
     // this is used in AdvanceFrame().
     mAnim->InitAnimationFrameTimeIfNecessary();
@@ -1457,12 +1458,15 @@ RasterImage::StopAnimation()
 {
   NS_ABORT_IF_FALSE(mAnimating, "Should be animating!");
 
-  if (mError)
-    return NS_ERROR_FAILURE;
+  nsresult rv = NS_OK;
+  if (mError) {
+    rv = NS_ERROR_FAILURE;
+  } else {
+    mAnim->SetAnimationFrameTime(TimeStamp());
+  }
 
-  mAnim->SetAnimationFrameTime(TimeStamp());
-
-  return NS_OK;
+  mAnimating = false;
+  return rv;
 }
 
 //******************************************************************************
@@ -1483,9 +1487,7 @@ RasterImage::ResetAnimation()
     StopAnimation();
 
   mFrameBlender.ResetAnimation();
-  if (mAnim) {
-    mAnim->ResetAnimation();
-  }
+  mAnim->ResetAnimation();
 
   UpdateImageContainer();
 
@@ -1498,13 +1500,9 @@ RasterImage::ResetAnimation()
     mStatusTracker->FrameChanged(&rect);
   }
 
-  if (ShouldAnimate()) {
-    StartAnimation();
-    // The animation may not have been running before, if mAnimationFinished
-    // was false (before we changed it to true in this function). So, mark the
-    // animation as running.
-    mAnimating = true;
-  }
+  // Start the animation again. It may not have been running before, if
+  // mAnimationFinished was true before entering this function.
+  EvaluateAnimation();
 
   return NS_OK;
 }
@@ -1573,10 +1571,8 @@ RasterImage::AddSourceData(const char *aBuffer, uint32_t aCount)
   // so that there's no gap for anything to miss us.
   if (mMultipart && mBytesDecoded == 0) {
     // Our previous state may have been animated, so let's clean up
-    if (mAnimating) {
+    if (mAnimating)
       StopAnimation();
-      mAnimating = false;
-    }
     mAnimationFinished = false;
     if (mAnim) {
       delete mAnim;

image/src/VectorImage.cpp

@@ -432,14 +432,18 @@ VectorImage::StartAnimation()
 nsresult
 VectorImage::StopAnimation()
 {
-  if (mError)
-    return NS_ERROR_FAILURE;
+  nsresult rv = NS_OK;
+  if (mError) {
+    rv = NS_ERROR_FAILURE;
+  } else {
+    MOZ_ASSERT(mIsFullyLoaded && mHaveAnimations,
+               "Should not have been animating!");
 
-  MOZ_ASSERT(mIsFullyLoaded && mHaveAnimations,
-             "Should not have been animating!");
+    mSVGDocumentWrapper->StopAnimation();
+  }
 
-  mSVGDocumentWrapper->StopAnimation();
-  return NS_OK;
+  mAnimating = false;
+  return rv;
 }
 
 bool