Bug 1187386 (Part 6) - Merge Decoder::Finish() and RasterImage::OnDecodingComplete() into RasterImage::FinalizeDecoder(). r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 24 Jul 2015 11:03:37 -0700
changeset 513427 79ece54768efbc9945fe86cec55cb2e9fb4f4fe2
parent 513426 5617780174956cbbd38861caec6be4b555f202eb
child 513428 768e69f4f883eca283ce4a62d118f7160a446190
push id79927
push usermfowler@mozilla.com
push dateFri, 24 Jul 2015 18:04:24 +0000
treeherdertry@52d47fd24448 [default view] [failures only]
reviewerstn
bugs1187386
milestone42.0a1
Bug 1187386 (Part 6) - Merge Decoder::Finish() and RasterImage::OnDecodingComplete() into RasterImage::FinalizeDecoder(). r=tn
image/DecodePool.cpp
image/Decoder.cpp
image/Decoder.h
image/RasterImage.cpp
image/RasterImage.h
--- a/image/DecodePool.cpp
+++ b/image/DecodePool.cpp
@@ -91,17 +91,16 @@ public:
 
     nsCOMPtr<nsIRunnable> worker = new NotifyDecodeCompleteWorker(aDecoder);
     NS_DispatchToMainThread(worker);
   }
 
   NS_IMETHOD Run() override
   {
     MOZ_ASSERT(NS_IsMainThread());
-    mDecoder->Finish();
     mDecoder->GetImage()->FinalizeDecoder(mDecoder);
     return NS_OK;
   }
 
 private:
   explicit NotifyDecodeCompleteWorker(Decoder* aDecoder)
     : mDecoder(aDecoder)
   { }
@@ -487,14 +486,13 @@ DecodePool::NotifyDecodeComplete(Decoder
   MOZ_ASSERT(aDecoder);
 
   if (!NS_IsMainThread() ||
       (aDecoder->GetFlags() & imgIContainer::FLAG_ASYNC_NOTIFY)) {
     NotifyDecodeCompleteWorker::Dispatch(aDecoder);
     return;
   }
 
-  aDecoder->Finish();
   aDecoder->GetImage()->FinalizeDecoder(aDecoder);
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -5,18 +5,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "Decoder.h"
 
 #include "mozilla/gfx/2D.h"
 #include "DecodePool.h"
 #include "GeckoProfiler.h"
 #include "imgIContainer.h"
-#include "nsIConsoleService.h"
-#include "nsIScriptError.h"
 #include "nsProxyRelease.h"
 #include "nsServiceManagerUtils.h"
 #include "nsComponentManagerUtils.h"
 #include "mozilla/Telemetry.h"
 
 using mozilla::gfx::IntSize;
 using mozilla::gfx::SurfaceFormat;
 
@@ -234,61 +232,16 @@ Decoder::CompleteDecode()
     // as optimizable. We don't support optimizing animated images and
     // optimizing transient images isn't worth it.
     if (!mIsAnimated && !mImageIsTransient && mCurrentFrame) {
       mCurrentFrame->SetOptimizable();
     }
   }
 }
 
-void
-Decoder::Finish()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  MOZ_ASSERT(HasError() || !mInFrame, "Finishing while we're still in a frame");
-
-  // If we detected an error in CompleteDecode(), log it to the error console.
-  if (mShouldReportError && !WasAborted()) {
-    nsCOMPtr<nsIConsoleService> consoleService =
-      do_GetService(NS_CONSOLESERVICE_CONTRACTID);
-    nsCOMPtr<nsIScriptError> errorObject =
-      do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
-
-    if (consoleService && errorObject && !HasDecoderError()) {
-      nsAutoString msg(NS_LITERAL_STRING("Image corrupt or truncated."));
-      nsAutoString src;
-      if (mImage->GetURI()) {
-        nsCString uri;
-        if (mImage->GetURI()->GetSpecTruncatedTo1k(uri) == ImageURL::TruncatedTo1k) {
-          msg += NS_LITERAL_STRING(" URI in this note truncated due to length.");
-        }
-        src = NS_ConvertUTF8toUTF16(uri);
-      }
-      if (NS_SUCCEEDED(errorObject->InitWithWindowID(
-                         msg,
-                         src,
-                         EmptyString(), 0, 0, nsIScriptError::errorFlag,
-                         "Image", mImage->InnerWindowID()
-                       ))) {
-        consoleService->LogMessage(errorObject);
-      }
-    }
-  }
-
-  nsresult rv = mImageMetadata.SetOnImage(mImage);
-  if (NS_FAILED(rv)) {
-    PostResizeError();
-  }
-
-  if (mDecodeDone && !IsMetadataDecode()) {
-    mImage->OnDecodingComplete(mIsAnimated);
-  }
-}
-
 nsresult
 Decoder::AllocateFrame(uint32_t aFrameNum,
                        const nsIntSize& aTargetSize,
                        const nsIntRect& aFrameRect,
                        gfx::SurfaceFormat aFormat,
                        uint8_t aPaletteDepth)
 {
   mCurrentFrame = AllocateFrameInternal(aFrameNum, aTargetSize, aFrameRect,
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -38,22 +38,16 @@ public:
    * data is needed, Decode() automatically ensures that it will be called again
    * on a DecodePool thread when the data becomes available.
    *
    * Any errors are reported by setting the appropriate state on the decoder.
    */
   nsresult Decode();
 
   /**
-   * Cleans up the decoder's state and notifies our image about success or
-   * failure. May only be called on the main thread.
-   */
-  void Finish();
-
-  /**
    * Given a maximum number of bytes we're willing to decode, @aByteLimit,
    * returns true if we should attempt to run this decoder synchronously.
    */
   bool ShouldSyncDecode(size_t aByteLimit);
 
   /**
    * Gets the invalidation region accumulated by the decoder so far, and clears
    * the decoder's invalidation region. This means that each call to
@@ -102,17 +96,17 @@ public:
    * is enough to determine the image's intrinsic size. A metadata decode is
    * enabled by calling SetMetadataDecode() *before* calling Init().
    */
   void SetMetadataDecode(bool aMetadataDecode)
   {
     MOZ_ASSERT(!mInitialized, "Shouldn't be initialized yet");
     mMetadataDecode = aMetadataDecode;
   }
-  bool IsMetadataDecode() { return mMetadataDecode; }
+  bool IsMetadataDecode() const { return mMetadataDecode; }
 
   /**
    * If this decoder supports downscale-during-decode, sets the target size that
    * this image should be decoded to.
    *
    * If this decoder *doesn't* support downscale-during-decode, returns
    * NS_ERROR_NOT_AVAILABLE. If the provided size is unacceptable, returns
    * another error.
@@ -205,29 +199,41 @@ public:
   uint32_t GetFrameCount() { return mFrameCount; }
 
   // The number of complete frames we have (ie, not including anything
   // in-progress).
   uint32_t GetCompleteFrameCount() {
     return mInFrame ? mFrameCount - 1 : mFrameCount;
   }
 
+  // Did we discover that the image we're decoding is animated?
+  bool HasAnimation() const { return mIsAnimated; }
+
   // Error tracking
   bool HasError() const { return HasDataError() || HasDecoderError(); }
   bool HasDataError() const { return mDataError; }
   bool HasDecoderError() const { return NS_FAILED(mFailCode); }
+  bool ShouldReportError() const { return mShouldReportError; }
   nsresult GetDecoderError() const { return mFailCode; }
   void PostResizeError() { PostDataError(); }
 
+  /// Did we finish decoding enough that calling Decode() again would be useless?
   bool GetDecodeDone() const
   {
     return mDecodeDone || (mMetadataDecode && HasSize()) ||
            HasError() || mDataDone;
   }
 
+  /// Did we finish decoding enough to set |RasterImage::mHasBeenDecoded|?
+  // XXX(seth): This will be removed in bug 1187401.
+  bool GetDecodeTotallyDone() const { return mDecodeDone && !IsMetadataDecode(); }
+
+  /// Are we in the middle of a frame right now? Used for assertions only.
+  bool InFrame() const { return mInFrame; }
+
   /**
    * Returns true if this decoder was aborted.
    *
    * This may happen due to a low-memory condition, or because another decoder
    * was racing with this one to decode the same frames with the same flags and
    * this decoder lost the race. Either way, this is not a permanent situation
    * and does not constitute an error, so we don't report any errors when this
    * happens.
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -16,17 +16,19 @@
 #include "Decoder.h"
 #include "nsAutoPtr.h"
 #include "prenv.h"
 #include "prsystem.h"
 #include "ImageContainer.h"
 #include "ImageRegion.h"
 #include "Layers.h"
 #include "LookupResult.h"
+#include "nsIConsoleService.h"
 #include "nsIInputStream.h"
+#include "nsIScriptError.h"
 #include "nsPresContext.h"
 #include "SourceBuffer.h"
 #include "SurfaceCache.h"
 #include "FrameAnimator.h"
 
 #include "gfxContext.h"
 
 #include "mozilla/gfx/2D.h"
@@ -974,52 +976,16 @@ RasterImage::SetSize(int32_t aWidth, int
   // Set the size and flag that we have it
   mSize.SizeTo(aWidth, aHeight);
   mOrientation = aOrientation;
   mHasSize = true;
 
   return NS_OK;
 }
 
-void
-RasterImage::OnDecodingComplete(bool aIsAnimated)
-{
-  MOZ_ASSERT(NS_IsMainThread());
-
-  if (mError) {
-    return;
-  }
-
-  // Flag that we've been decoded before.
-  mHasBeenDecoded = true;
-
-  if (aIsAnimated) {
-    if (mAnim) {
-      mAnim->SetDoneDecoding(true);
-    } else {
-      // The OnAddedFrame event that will create mAnim is still in the event
-      // queue. Wait for it.
-      nsCOMPtr<nsIRunnable> runnable =
-        NS_NewRunnableMethod(this, &RasterImage::MarkAnimationDecoded);
-      NS_DispatchToMainThread(runnable);
-    }
-  }
-}
-
-void
-RasterImage::MarkAnimationDecoded()
-{
-  MOZ_ASSERT(mAnim, "Should have an animation now");
-  if (!mAnim) {
-    return;
-  }
-
-  mAnim->SetDoneDecoding(true);
-}
-
 NS_IMETHODIMP
 RasterImage::SetAnimationMode(uint16_t aAnimationMode)
 {
   if (mAnim) {
     mAnim->SetAnimationMode(aAnimationMode);
   }
   return SetAnimationModeInternal(aAnimationMode);
 }
@@ -2005,19 +1971,50 @@ RasterImage::NotifyProgress(Progress aPr
   image->mProgressTracker->SyncNotifyProgress(aProgress, aInvalidRect);
 }
 
 void
 RasterImage::FinalizeDecoder(Decoder* aDecoder)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aDecoder);
+  MOZ_ASSERT(aDecoder->HasError() || !aDecoder->InFrame(),
+             "Finalizing a decoder in the middle of a frame");
+
+  // If the decoder detected an error, log it to the error console.
+  if (aDecoder->ShouldReportError() && !aDecoder->WasAborted()) {
+    ReportDecoderError(aDecoder);
+  }
+
+  // Record all the metadata the decoder gathered about this image.
+  nsresult rv = aDecoder->GetImageMetadata().SetOnImage(this);
+  if (NS_FAILED(rv)) {
+    aDecoder->PostResizeError();
+  }
+
   MOZ_ASSERT(mError || mHasSize || !aDecoder->HasSize(),
              "Should have handed off size by now");
 
+  if (aDecoder->GetDecodeTotallyDone() && !mError) {
+    // Flag that we've been decoded before.
+    mHasBeenDecoded = true;
+
+    if (aDecoder->HasAnimation()) {
+      if (mAnim) {
+        mAnim->SetDoneDecoding(true);
+      } else {
+        // The OnAddedFrame event that will create mAnim is still in the event
+        // queue. Wait for it.
+        nsCOMPtr<nsIRunnable> runnable =
+          NS_NewRunnableMethod(this, &RasterImage::MarkAnimationDecoded);
+        NS_DispatchToMainThread(runnable);
+      }
+    }
+  }
+
   // Send out any final notifications.
   NotifyProgress(aDecoder->TakeProgress(),
                  aDecoder->TakeInvalidRect(),
                  aDecoder->GetDecodeFlags());
 
   bool wasMetadata = aDecoder->IsMetadataDecode();
   bool done = aDecoder->GetDecodeDone();
 
@@ -2064,16 +2061,56 @@ RasterImage::FinalizeDecoder(Decoder* aD
 
   // If we were a metadata decode and a full decode was requested, do it.
   if (done && wasMetadata && mWantFullDecode) {
     mWantFullDecode = false;
     RequestDecode();
   }
 }
 
+void
+RasterImage::MarkAnimationDecoded()
+{
+  MOZ_ASSERT(mAnim, "Should have an animation now");
+  if (!mAnim) {
+    return;
+  }
+
+  mAnim->SetDoneDecoding(true);
+}
+
+void
+RasterImage::ReportDecoderError(Decoder* aDecoder)
+{
+  nsCOMPtr<nsIConsoleService> consoleService =
+    do_GetService(NS_CONSOLESERVICE_CONTRACTID);
+  nsCOMPtr<nsIScriptError> errorObject =
+    do_CreateInstance(NS_SCRIPTERROR_CONTRACTID);
+
+  if (consoleService && errorObject && !aDecoder->HasDecoderError()) {
+    nsAutoString msg(NS_LITERAL_STRING("Image corrupt or truncated."));
+    nsAutoString src;
+    if (GetURI()) {
+      nsCString uri;
+      if (GetURI()->GetSpecTruncatedTo1k(uri) == ImageURL::TruncatedTo1k) {
+        msg += NS_LITERAL_STRING(" URI in this note truncated due to length.");
+      }
+      src = NS_ConvertUTF8toUTF16(uri);
+    }
+    if (NS_SUCCEEDED(errorObject->InitWithWindowID(
+                       msg,
+                       src,
+                       EmptyString(), 0, 0, nsIScriptError::errorFlag,
+                       "Image", InnerWindowID()
+                     ))) {
+      consoleService->LogMessage(errorObject);
+    }
+  }
+}
+
 already_AddRefed<imgIContainer>
 RasterImage::Unwrap()
 {
   nsCOMPtr<imgIContainer> self(this);
   return self.forget();
 }
 
 IntSize
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -201,22 +201,16 @@ public:
   nsresult SetSize(int32_t aWidth, int32_t aHeight, Orientation aOrientation);
 
   /**
    * Number of times to loop the image.
    * @note -1 means forever.
    */
   void     SetLoopCount(int32_t aLoopCount);
 
-  /// Notification that the entire image has been decoded.
-  void OnDecodingComplete(bool aIsAnimated);
-
-  /// Helper method for OnDecodingComplete.
-  void MarkAnimationDecoded();
-
   /**
    * Sends the provided progress notifications to ProgressTracker.
    *
    * Main-thread only.
    *
    * @param aProgress    The progress notifications to send.
    * @param aInvalidRect An invalidation rect to send.
    * @param aFlags       The decode flags used by the decoder that generated
@@ -229,16 +223,20 @@ public:
 
   /**
    * Records telemetry and does final teardown of the provided decoder.
    *
    * Main-thread only.
    */
   void FinalizeDecoder(Decoder* aDecoder);
 
+  // Helper methods for FinalizeDecoder.
+  void MarkAnimationDecoded();
+  void ReportDecoderError(Decoder* aDecoder);
+
 
   //////////////////////////////////////////////////////////////////////////////
   // Network callbacks.
   //////////////////////////////////////////////////////////////////////////////
 
   virtual nsresult OnImageDataAvailable(nsIRequest* aRequest,
                                         nsISupports* aContext,
                                         nsIInputStream* aInStr,