Bug 1258741 - Part 2. Ensure we consistently render partially decoded images. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Mon, 26 Sep 2016 14:18:37 -0400
changeset 361493 d56e9b123ed63993592c477540b64ec24a18002b
parent 361492 5238fbaf49fa56ffa84a83dfa9cc92e9818671a4
child 361494 e6d49c56eac16d019be8036900919b22e66fbccb
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1258741
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1258741 - Part 2. Ensure we consistently render partially decoded images. r=tnikkel
image/Decoder.cpp
image/RasterImage.cpp
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -185,51 +185,49 @@ Decoder::CompleteDecode()
   }
 
   rv = HasError() ? FinishWithErrorInternal()
                   : FinishInternal();
   if (NS_FAILED(rv)) {
     PostError();
   }
 
-  // If this was a metadata decode and we never got a size, the decode failed.
-  if (IsMetadataDecode() && !HasSize()) {
-    PostError();
+  if (IsMetadataDecode()) {
+    // If this was a metadata decode and we never got a size, the decode failed.
+    if (!HasSize()) {
+      PostError();
+    }
+    return;
   }
 
-  // If the implementation left us mid-frame, finish that up.
-  if (mInFrame && !HasError()) {
+  // If the implementation left us mid-frame, finish that up. Note that it may
+  // have left us transparent.
+  if (mInFrame) {
+    PostHasTransparency();
     PostFrameStop();
   }
 
-  // If PostDecodeDone() has not been called, we need to send teardown
-  // notifications (and report an error to the console later).
-  if (!mDecodeDone && !IsMetadataDecode()) {
+  // If PostDecodeDone() has not been called, we may need to send teardown
+  // notifications if it is unrecoverable.
+  if (!mDecodeDone) {
+    // We should always report an error to the console in this case.
     mShouldReportError = true;
 
-    // Even if we encountered an error, we're still usable if we have at least
-    // one complete frame.
     if (GetCompleteFrameCount() > 0) {
-      // We're usable, so do exactly what we should have when the decoder
-      // completed.
-
-      // Not writing to the entire frame may have left us transparent.
+      // We're usable if we have at least one complete frame, so do exactly
+      // what we should have when the decoder completed.
       PostHasTransparency();
-
-      if (mInFrame) {
-        PostFrameStop();
-      }
       PostDecodeDone();
     } else {
       // We're not usable. Record some final progress indicating the error.
       mProgress |= FLAG_DECODE_COMPLETE | FLAG_HAS_ERROR;
     }
   }
 
-  if (mDecodeDone && !IsMetadataDecode()) {
+  if (mDecodeDone) {
     MOZ_ASSERT(HasError() || mCurrentFrame, "Should have an error or a frame");
 
     // If this image wasn't animated and isn't a transient image, mark its frame
     // as optimizable. We don't support optimizing animated images and
     // optimizing transient images isn't worth it.
     if (!HasAnimation() &&
         !(mDecoderFlags & DecoderFlags::IMAGE_IS_TRANSIENT) &&
         mCurrentFrame) {
@@ -505,15 +503,19 @@ Decoder::PostDecodeDone(int32_t aLoopCou
   mProgress |= FLAG_DECODE_COMPLETE;
 }
 
 void
 Decoder::PostError()
 {
   mError = true;
 
-  if (mInFrame && mCurrentFrame) {
+  if (mInFrame) {
+    MOZ_ASSERT(mCurrentFrame);
+    MOZ_ASSERT(mFrameCount > 0);
     mCurrentFrame->Abort();
+    mInFrame = false;
+    --mFrameCount;
   }
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1612,50 +1612,54 @@ RasterImage::NotifyDecodeComplete(const 
 
   if (!(aDecoderFlags & DecoderFlags::FIRST_FRAME_ONLY) &&
       mHasBeenDecoded && mAnimationState) {
     // We've finished a full decode of all animation frames and our AnimationState
     // has been notified about them all, so let it know not to expect anymore.
     mAnimationState->SetDoneDecoding(true);
   }
 
-  if (!aStatus.mWasMetadataDecode && aTelemetry.mChunkCount) {
-    Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, aTelemetry.mChunkCount);
-  }
+  // Do some telemetry if this isn't a metadata decode.
+  if (!aStatus.mWasMetadataDecode) {
+    if (aTelemetry.mChunkCount) {
+      Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, aTelemetry.mChunkCount);
+    }
 
-  if (aStatus.mFinished) {
-    // Do some telemetry if this isn't a metadata decode.
-    if (!aStatus.mWasMetadataDecode) {
+    if (aStatus.mFinished) {
       Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME,
                             int32_t(aTelemetry.mDecodeTime.ToMicroseconds()));
 
       if (aTelemetry.mSpeedHistogram) {
         Telemetry::Accumulate(*aTelemetry.mSpeedHistogram, aTelemetry.Speed());
       }
     }
+  }
 
-    // Detect errors.
-    if (aStatus.mHadError) {
-      DoError();
-    } else if (aStatus.mWasMetadataDecode && !mHasSize) {
-      DoError();
-    }
+  // Only act on errors if we have no usable frames from the decoder.
+  if (aStatus.mHadError &&
+      (!mAnimationState || mAnimationState->KnownFrameCount() == 0)) {
+    DoError();
+  } else if (aStatus.mWasMetadataDecode && !mHasSize) {
+    DoError();
+  }
 
+  // XXX(aosmond): Can we get this far without mFinished == true?
+  if (aStatus.mFinished && aStatus.mWasMetadataDecode) {
     // If we were waiting to fire the load event, go ahead and fire it now.
-    if (mLoadProgress && aStatus.mWasMetadataDecode) {
+    if (mLoadProgress) {
       NotifyForLoadEvent(*mLoadProgress);
       mLoadProgress = Nothing();
       NotifyProgress(FLAG_ONLOAD_UNBLOCKED);
     }
-  }
 
-  // If we were a metadata decode and a full decode was requested, do it.
-  if (aStatus.mFinished && aStatus.mWasMetadataDecode && mWantFullDecode) {
-    mWantFullDecode = false;
-    RequestDecodeForSize(mSize, DECODE_FLAGS_DEFAULT);
+    // If we were a metadata decode and a full decode was requested, do it.
+    if (mWantFullDecode) {
+      mWantFullDecode = false;
+      RequestDecodeForSize(mSize, DECODE_FLAGS_DEFAULT);
+    }
   }
 }
 
 void
 RasterImage::ReportDecoderError()
 {
   nsCOMPtr<nsIConsoleService> consoleService =
     do_GetService(NS_CONSOLESERVICE_CONTRACTID);