Bug 1187386 (Part 6) - Merge Decoder::Finish() and RasterImage::OnDecodingComplete() into RasterImage::FinalizeDecoder(). r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 31 Jul 2015 07:29:15 -0700
changeset 287402 819db042055a9105a1ea3f563d6c5365006892bb
parent 287401 8372b72b359cb1265b7d6a93bfe4f180bee4a212
child 287403 9c768fc28bb09825dd20dfecd2df3021a1462274
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1187386
milestone42.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 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.
@@ -213,29 +207,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);
 }
@@ -2007,19 +1973,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();
 
@@ -2066,16 +2063,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,