Bug 403364 - "APNG animations sometimes loop incorrectly" [p=alfredkayser@gmail.com (Alfred Kayser) r=stuart a=blocking1.9+]
authorreed@reedloden.com
Sat, 19 Jan 2008 00:10:26 -0800
changeset 10438 8c42b0b4f72281742b73f2e1af87ad35b95d474c
parent 10437 decaf4a24b05692220789e7ccecd29cd8ddd20b4
child 10439 7e8a8af8a63fd3d8420895e1d3d598b576358071
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherdermozilla-central@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstuart, blocking1.9
bugs403364
milestone1.9b3pre
Bug 403364 - "APNG animations sometimes loop incorrectly" [p=alfredkayser@gmail.com (Alfred Kayser) r=stuart a=blocking1.9+]
modules/libpr0n/src/imgContainer.cpp
--- a/modules/libpr0n/src/imgContainer.cpp
+++ b/modules/libpr0n/src/imgContainer.cpp
@@ -119,16 +119,19 @@ NS_IMETHODIMP imgContainer::Init(PRInt32
 {
   if (aWidth <= 0 || aHeight <= 0) {
     NS_WARNING("error - negative image size\n");
     return NS_ERROR_FAILURE;
   }
 
   mSize.SizeTo(aWidth, aHeight);
   
+  // As we are reloading it means we are no longer in 'discarded' state
+  mDiscarded = PR_FALSE;
+
   mObserver = do_GetWeakReference(aObserver);
   
   return NS_OK;
 }
 
 //******************************************************************************
 /* readonly attribute gfx_format preferredAlphaChannelFormat; */
 NS_IMETHODIMP imgContainer::GetPreferredAlphaChannelFormat(gfx_format *aFormat)
@@ -561,24 +564,19 @@ NS_IMETHODIMP imgContainer::SetDiscardab
 }
 
 //******************************************************************************
 /* void addRestoreData(in nsIInputStream aInputStream, in unsigned long aCount); */
 NS_IMETHODIMP imgContainer::AddRestoreData(const char *aBuffer, PRUint32 aCount)
 {
   NS_ASSERTION(aBuffer, "imgContainer::AddRestoreData() called with null aBuffer");
 
-  if (!DiscardingEnabled ())
+  if (!mDiscardable)
     return NS_OK;
 
-  if (!mDiscardable) {
-    NS_WARNING ("imgContainer::AddRestoreData() can only be called if SetDiscardable is called first");
-    return NS_ERROR_FAILURE;
-  }
-
   if (mRestoreDataDone) {
     /* We are being called from the decoder while the data is being restored
      * (i.e. we were fully loaded once, then we discarded the image data, then
      * we are being restored).  We don't want to save the compressed data again,
      * since we already have it.
      */
     return NS_OK;
   }
@@ -617,18 +615,18 @@ get_header_str (char *buf, char *data, P
 
   buf[i * 2] = 0;
 }
 
 //******************************************************************************
 /* void restoreDataDone(); */
 NS_IMETHODIMP imgContainer::RestoreDataDone (void)
 {
-
-  if (!DiscardingEnabled ())
+  // If image is not discardable, don't start discard timer
+  if (!mDiscardable)
     return NS_OK;
 
   if (mRestoreDataDone)
     return NS_OK;
 
   mRestoreData.Compact();
 
   mRestoreDataDone = PR_TRUE;
@@ -649,27 +647,22 @@ NS_IMETHODIMP imgContainer::RestoreDataD
 
   return ResetDiscardTimer();
 }
 
 //******************************************************************************
 /* void notify(in nsITimer timer); */
 NS_IMETHODIMP imgContainer::Notify(nsITimer *timer)
 {
-  nsresult result;
-
-  result = RestoreDiscardedData();
-  if (NS_FAILED (result))
-    return result;
+  nsresult rv = RestoreDiscardedData();
+  NS_ENSURE_SUCCESS(rv, rv);
 
   // This should never happen since the timer is only set up in StartAnimation()
   // after mAnim is checked to exist.
-  NS_ASSERTION(mAnim, "imgContainer::Notify() called but mAnim is null");
-  if (!mAnim)
-    return NS_ERROR_UNEXPECTED;
+  NS_ENSURE_TRUE(mAnim, NS_ERROR_UNEXPECTED);
   NS_ASSERTION(mAnim->timer == timer,
                "imgContainer::Notify() called with incorrect timer");
 
   if (!(mAnim->animating) || !(mAnim->timer))
     return NS_OK;
 
   nsCOMPtr<imgIContainerObserver> observer(do_QueryReferent(mObserver));
   if (!observer) {
@@ -1264,31 +1257,30 @@ NS_IMETHODIMP imgContainer::GetKeys(PRUi
   }
   return mProperties->GetKeys(count, keys);
 }
 
 static int
 get_discard_timer_ms (void)
 {
   /* FIXME: don't hardcode this */
-  return 45000; /* 45 seconds */
+  return 15000; /* 15 seconds */
 }
 
 void
 imgContainer::sDiscardTimerCallback(nsITimer *aTimer, void *aClosure)
 {
   imgContainer *self = (imgContainer *) aClosure;
-  int old_frame_count;
 
   NS_ASSERTION(aTimer == self->mDiscardTimer,
                "imgContainer::DiscardTimerCallback() got a callback for an unknown timer");
 
   self->mDiscardTimer = nsnull;
 
-  old_frame_count = self->mFrames.Count();
+  int old_frame_count = self->mFrames.Count();
 
   if (self->mAnim) {
     delete self->mAnim;
     self->mAnim = nsnull;
   }
 
   self->mFrames.Clear();
 
@@ -1308,57 +1300,50 @@ imgContainer::sDiscardTimerCallback(nsIT
 nsresult
 imgContainer::ResetDiscardTimer (void)
 {
   if (!DiscardingEnabled())
     return NS_OK;
 
   if (!mDiscardTimer) {
     mDiscardTimer = do_CreateInstance("@mozilla.org/timer;1");
-
-    if (!mDiscardTimer)
-      return NS_ERROR_OUT_OF_MEMORY;
+    NS_ENSURE_TRUE(mDiscardTimer, NS_ERROR_OUT_OF_MEMORY);
   } else {
-    if (NS_FAILED(mDiscardTimer->Cancel()))
-      return NS_ERROR_FAILURE;
+    nsresult rv = mDiscardTimer->Cancel();
+    NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
   }
 
   return mDiscardTimer->InitWithFuncCallback(sDiscardTimerCallback,
                                              (void *) this,
                                              get_discard_timer_ms (),
                                              nsITimer::TYPE_ONE_SHOT);
 }
 
 nsresult
 imgContainer::RestoreDiscardedData(void)
 {
-  nsresult result;
-  int num_expected_frames;
-
-  if (!mDiscardable)
+  // mRestoreDataDone = PR_TRUE means that we want to timeout and then discard the image frames
+  // So, we only need to restore, if mRestoreDataDone is true, and then only when the frames are discarded...
+  if (!mRestoreDataDone) 
     return NS_OK;
 
-  result = ResetDiscardTimer();
-  if (NS_FAILED (result))
-    return result;
+  // Reset timer, as the frames are accessed
+  nsresult rv = ResetDiscardTimer();
+  NS_ENSURE_SUCCESS(rv, rv);
 
   if (!mDiscarded)
     return NS_OK;
 
-  num_expected_frames = mNumFrames;
+  int num_expected_frames = mNumFrames;
 
-  result = ReloadImages ();
-  if (NS_FAILED (result)) {
-    PR_LOG (gCompressedImageAccountingLog, PR_LOG_DEBUG,
-            ("CompressedImageAccounting: imgContainer::RestoreDiscardedData() for container %p failed to ReloadImages()",
-             this));
-    return result;
-  }
+  // To prevent that ReloadImages is called multiple times, reset the flag before reloading
+  mDiscarded = PR_FALSE;
 
-  mDiscarded = PR_FALSE;
+  rv = ReloadImages();
+  NS_ENSURE_SUCCESS(rv, rv);
 
   NS_ASSERTION (mNumFrames == mFrames.Count(),
                 "number of restored image frames doesn't match");
   NS_ASSERTION (num_expected_frames == mNumFrames,
                 "number of restored image frames doesn't match the original number of frames!");
   
   PR_LOG (gCompressedImageAccountingLog, PR_LOG_DEBUG,
           ("CompressedImageAccounting: imgContainer::RestoreDiscardedData() restored discarded data "
@@ -1490,19 +1475,16 @@ NS_IMETHODIMP
 ContainerLoader::FrameChanged(imgIContainer *aContainer, gfxIImageFrame *aFrame, nsIntRect * aDirtyRect)
 {
   return NS_OK;
 }
 
 nsresult
 imgContainer::ReloadImages(void)
 {
-  nsresult result = NS_ERROR_FAILURE;
-  nsCOMPtr<nsIInputStream> stream;
-
   NS_ASSERTION(!mRestoreData.IsEmpty(),
                "imgContainer::ReloadImages(): mRestoreData should not be empty");
   NS_ASSERTION(mRestoreDataDone,
                "imgContainer::ReloadImages(): mRestoreDataDone shoudl be true!");
 
   mNumFrames = 0;
   NS_ASSERTION(mFrames.Count() == 0,
                "imgContainer::ReloadImages(): mFrames should be empty");
@@ -1523,26 +1505,27 @@ imgContainer::ReloadImages(void)
            ("CompressedImageAccounting: imgContainer::ReloadImages() could not allocate ContainerLoader "
             "when reloading the images for container %p",
             this));
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   loader->SetImage(this);
 
-  result = decoder->Init(loader);
+  nsresult result = decoder->Init(loader);
   if (NS_FAILED(result)) {
     PR_LOG(gCompressedImageAccountingLog, PR_LOG_WARNING,
            ("CompressedImageAccounting: imgContainer::ReloadImages() image container %p "
             "failed to initialize the decoder (%s)",
             this,
             mDiscardableMimeType.get()));
     return result;
   }
 
+  nsCOMPtr<nsIInputStream> stream;
   result = NS_NewByteInputStream(getter_AddRefs(stream), mRestoreData.Elements(), mRestoreData.Length(), NS_ASSIGNMENT_DEPEND);
   NS_ENSURE_SUCCESS(result, result);
 
   if (PR_LOG_TEST(gCompressedImageAccountingLog, PR_LOG_DEBUG)) {
     char buf[9];
     get_header_str(buf, mRestoreData.Elements(), mRestoreData.Length());
     PR_LOG(gCompressedImageAccountingLog, PR_LOG_WARNING,
            ("CompressedImageAccounting: imgContainer::ReloadImages() starting to restore images for container %p (%s) - "
@@ -1553,18 +1536,18 @@ imgContainer::ReloadImages(void)
             buf,
             mRestoreData.Length()));
   }
 
   PRUint32 written;
   result = decoder->WriteFrom(stream, mRestoreData.Length(), &written);
   NS_ENSURE_SUCCESS(result, result);
 
-  if (NS_FAILED(decoder->Flush()))
-    return result;
+  result = decoder->Flush();
+  NS_ENSURE_SUCCESS(result, result);
 
   result = decoder->Close();
   NS_ENSURE_SUCCESS(result, result);
 
   NS_ASSERTION(mFrames.Count() == mNumFrames,
                "imgContainer::ReloadImages(): the restored mFrames.Count() doesn't match mNumFrames!");
 
   return result;