Bug 1079627 (Part 1) - Make image decoders hold a strong reference to their image. r=tn
authorSeth Fowler <seth@mozilla.com>
Sun, 11 Jan 2015 05:34:20 -0800
changeset 248976 3dd5401f359cd1d442c2ebb800c9a4938396aba6
parent 248975 7bd61e25e18fcfef89c1424b413a639cf89e518e
child 248977 4d93dcf74f01409f8166fb93f8e535c1ae6ac898
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1079627
milestone37.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 1079627 (Part 1) - Make image decoders hold a strong reference to their image. r=tn
image/decoders/nsBMPDecoder.cpp
image/decoders/nsBMPDecoder.h
image/decoders/nsGIFDecoder2.cpp
image/decoders/nsGIFDecoder2.h
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
image/decoders/nsIconDecoder.cpp
image/decoders/nsIconDecoder.h
image/decoders/nsJPEGDecoder.cpp
image/decoders/nsJPEGDecoder.h
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
image/src/Decoder.cpp
image/src/Decoder.h
image/src/RasterImage.cpp
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -33,17 +33,17 @@ GetBMPLog()
   return sBMPLog;
 }
 #endif
 
 // Convert from row (1..height) to absolute line (0..height-1)
 #define LINE(row) ((mBIH.height < 0) ? (-mBIH.height - (row)) : ((row) - 1))
 #define PIXEL_OFFSET(row, col) (LINE(row) * mBIH.width + col)
 
-nsBMPDecoder::nsBMPDecoder(RasterImage& aImage)
+nsBMPDecoder::nsBMPDecoder(RasterImage* aImage)
   : Decoder(aImage)
   , mPos(0)
   , mLOH(WIN_V3_HEADER_LENGTH)
   , mNumColors(0)
   , mColors(nullptr)
   , mRow(nullptr)
   , mRowBytes(0)
   , mCurLine(1)  // Otherwise decoder will never start.
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -18,17 +18,17 @@ namespace image {
 class RasterImage;
 
 /// Decoder for BMP-Files, as used by Windows and OS/2
 
 class nsBMPDecoder : public Decoder
 {
 public:
 
-    explicit nsBMPDecoder(RasterImage& aImage);
+    explicit nsBMPDecoder(RasterImage* aImage);
     ~nsBMPDecoder();
 
     // Specifies whether or not the BMP file will contain alpha data
     // If set to true and the BMP is 32BPP, the alpha data will be
     // retrieved from the 4th byte of image data per pixel
     void SetUseAlphaData(bool useAlphaData);
 
     // Obtains the bits per pixel from the internal BIH header
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -63,17 +63,17 @@ namespace image {
     mGIFStruct.state = (s);            \
   PR_END_MACRO
 
 // Get a 16-bit value stored in little-endian format
 #define GETINT16(p)   ((p)[1]<<8|(p)[0])
 //////////////////////////////////////////////////////////////////////
 // GIF Decoder Implementation
 
-nsGIFDecoder2::nsGIFDecoder2(RasterImage& aImage)
+nsGIFDecoder2::nsGIFDecoder2(RasterImage* aImage)
   : Decoder(aImage)
   , mCurrentRow(-1)
   , mLastFlushedRow(-1)
   , mOldColor(0)
   , mCurrentFrameIndex(-1)
   , mCurrentPass(0)
   , mLastFlushedPass(0)
   , mGIFOpen(false)
--- a/image/decoders/nsGIFDecoder2.h
+++ b/image/decoders/nsGIFDecoder2.h
@@ -18,17 +18,17 @@ class RasterImage;
 
 //////////////////////////////////////////////////////////////////////
 // nsGIFDecoder2 Definition
 
 class nsGIFDecoder2 : public Decoder
 {
 public:
 
-  explicit nsGIFDecoder2(RasterImage& aImage);
+  explicit nsGIFDecoder2(RasterImage* aImage);
   ~nsGIFDecoder2();
 
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) MOZ_OVERRIDE;
   virtual void FinishInternal() MOZ_OVERRIDE;
   virtual Telemetry::ID SpeedHistogram() MOZ_OVERRIDE;
 
 private:
   // These functions will be called when the decoder has a decoded row,
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -53,17 +53,17 @@ nsICODecoder::GetNumColors()
     default:
       numColors = (uint16_t)-1;
     }
   }
   return numColors;
 }
 
 
-nsICODecoder::nsICODecoder(RasterImage& aImage)
+nsICODecoder::nsICODecoder(RasterImage* aImage)
  : Decoder(aImage)
 {
   mPos = mImageOffset = mCurrIcon = mNumIcons = mBPP = mRowBytes = 0;
   mIsPNG = false;
   mRow = nullptr;
   mOldLine = mCurLine = 1; // Otherwise decoder will never start
 }
 
@@ -247,17 +247,17 @@ nsICODecoder::WriteInternal(const char* 
     aCount -= 2;
   }
 
   if (mNumIcons == 0) {
     return; // Nothing to do.
   }
 
   uint16_t colorDepth = 0;
-  nsIntSize prefSize = mImage.GetRequestedResolution();
+  nsIntSize prefSize = mImage->GetRequestedResolution();
   if (prefSize.width == 0 && prefSize.height == 0) {
     prefSize.SizeTo(PREFICONSIZE, PREFICONSIZE);
   }
 
   // A measure of the difference in size between the entry we've found
   // and the requested size. We will choose the smallest image that is
   // >= requested size (i.e. we assume it's better to downscale a larger
   // icon than to upscale a smaller one).
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -18,17 +18,17 @@ namespace mozilla {
 namespace image {
 
 class RasterImage;
 
 class nsICODecoder : public Decoder
 {
 public:
 
-  explicit nsICODecoder(RasterImage& aImage);
+  explicit nsICODecoder(RasterImage* aImage);
   virtual ~nsICODecoder();
 
   // Obtains the width of the icon directory entry
   uint32_t GetRealWidth() const
   {
     return mDirEntry.mWidth == 0 ? 256 : mDirEntry.mWidth;
   }
 
--- a/image/decoders/nsIconDecoder.cpp
+++ b/image/decoders/nsIconDecoder.cpp
@@ -10,17 +10,17 @@
 #include "nsRect.h"
 #include "nsError.h"
 #include "RasterImage.h"
 #include <algorithm>
 
 namespace mozilla {
 namespace image {
 
-nsIconDecoder::nsIconDecoder(RasterImage& aImage)
+nsIconDecoder::nsIconDecoder(RasterImage* aImage)
  : Decoder(aImage),
    mWidth(-1),
    mHeight(-1),
    mPixBytesRead(0),
    mState(iconStateStart)
 {
   // Nothing to do
 }
--- a/image/decoders/nsIconDecoder.h
+++ b/image/decoders/nsIconDecoder.h
@@ -33,17 +33,17 @@ class RasterImage;
 //
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 class nsIconDecoder : public Decoder
 {
 public:
 
-  explicit nsIconDecoder(RasterImage& aImage);
+  explicit nsIconDecoder(RasterImage* aImage);
   virtual ~nsIconDecoder();
 
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) MOZ_OVERRIDE;
 
   uint8_t mWidth;
   uint8_t mHeight;
   uint32_t mPixBytesRead;
   uint32_t mState;
--- a/image/decoders/nsJPEGDecoder.cpp
+++ b/image/decoders/nsJPEGDecoder.cpp
@@ -79,17 +79,17 @@ METHODDEF(boolean) fill_input_buffer (j_
 METHODDEF(void) skip_input_data (j_decompress_ptr jd, long num_bytes);
 METHODDEF(void) term_source (j_decompress_ptr jd);
 METHODDEF(void) my_error_exit (j_common_ptr cinfo);
 
 // Normal JFIF markers can't have more bytes than this.
 #define MAX_JPEG_MARKER_LENGTH  (((uint32_t)1 << 16) - 1)
 
 
-nsJPEGDecoder::nsJPEGDecoder(RasterImage& aImage,
+nsJPEGDecoder::nsJPEGDecoder(RasterImage* aImage,
                              Decoder::DecodeStyle aDecodeStyle)
  : Decoder(aImage)
  , mDecodeStyle(aDecodeStyle)
 {
   mState = JPEG_HEADER;
   mReading = true;
   mImageData = nullptr;
 
@@ -232,17 +232,17 @@ nsJPEGDecoder::WriteInternal(const char*
 
       // Step 3: read file parameters with jpeg_read_header()
       if (jpeg_read_header(&mInfo, TRUE) == JPEG_SUSPENDED) {
         PR_LOG(GetJPEGDecoderAccountingLog(), PR_LOG_DEBUG,
                ("} (JPEG_SUSPENDED)"));
         return; // I/O suspension
       }
 
-      int sampleSize = mImage.GetRequestedSampleSize();
+      int sampleSize = mImage->GetRequestedSampleSize();
       if (sampleSize > 0) {
         mInfo.scale_num = 1;
         mInfo.scale_denom = sampleSize;
       }
 
       // Used to set up image size so arrays can be allocated
       jpeg_calc_output_dimensions(&mInfo);
 
--- a/image/decoders/nsJPEGDecoder.h
+++ b/image/decoders/nsJPEGDecoder.h
@@ -47,17 +47,17 @@ typedef enum {
 } jstate;
 
 class RasterImage;
 struct Orientation;
 
 class nsJPEGDecoder : public Decoder
 {
 public:
-  nsJPEGDecoder(RasterImage& aImage, Decoder::DecodeStyle aDecodeStyle);
+  nsJPEGDecoder(RasterImage* aImage, Decoder::DecodeStyle aDecodeStyle);
   virtual ~nsJPEGDecoder();
 
   virtual void InitInternal() MOZ_OVERRIDE;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) MOZ_OVERRIDE;
   virtual void FinishInternal() MOZ_OVERRIDE;
 
   virtual Telemetry::ID SpeedHistogram() MOZ_OVERRIDE;
   void NotifyDone();
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -102,17 +102,17 @@ nsPNGDecoder::AnimFrameInfo::AnimFrameIn
   }
 }
 #endif
 
 // First 8 bytes of a PNG file
 const uint8_t
 nsPNGDecoder::pngSignatureBytes[] = { 137, 80, 78, 71, 13, 10, 26, 10 };
 
-nsPNGDecoder::nsPNGDecoder(RasterImage& aImage)
+nsPNGDecoder::nsPNGDecoder(RasterImage* aImage)
  : Decoder(aImage),
    mPNG(nullptr), mInfo(nullptr),
    mCMSLine(nullptr), interlacebuf(nullptr),
    mInProfile(nullptr), mTransform(nullptr),
    mHeaderBytesRead(0), mCMSMode(0),
    mChannels(0), mFrameIsHidden(false),
    mDisablePremultipliedAlpha(false),
    mNumFrames(0)
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -19,17 +19,17 @@
 
 namespace mozilla {
 namespace image {
 class RasterImage;
 
 class nsPNGDecoder : public Decoder
 {
 public:
-  explicit nsPNGDecoder(RasterImage& aImage);
+  explicit nsPNGDecoder(RasterImage* aImage);
   virtual ~nsPNGDecoder();
 
   virtual void InitInternal() MOZ_OVERRIDE;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) MOZ_OVERRIDE;
   virtual Telemetry::ID SpeedHistogram() MOZ_OVERRIDE;
 
   nsresult CreateFrame(png_uint_32 aXOffset, png_uint_32 aYOffset,
                        int32_t aWidth, int32_t aHeight,
--- a/image/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -15,17 +15,17 @@
 #include "nsComponentManagerUtils.h"
 
 using mozilla::gfx::IntSize;
 using mozilla::gfx::SurfaceFormat;
 
 namespace mozilla {
 namespace image {
 
-Decoder::Decoder(RasterImage &aImage)
+Decoder::Decoder(RasterImage* aImage)
   : mImage(aImage)
   , mProgress(NoProgress)
   , mImageData(nullptr)
   , mColormap(nullptr)
   , mChunkCount(0)
   , mDecodeFlags(0)
   , mBytesDecoded(0)
   , mSendPartialInvalidations(false)
@@ -41,16 +41,31 @@ Decoder::Decoder(RasterImage &aImage)
 
 Decoder::~Decoder()
 {
   MOZ_ASSERT(mProgress == NoProgress,
              "Destroying Decoder without taking all its progress changes");
   MOZ_ASSERT(mInvalidRect.IsEmpty(),
              "Destroying Decoder without taking all its invalidations");
   mInitialized = false;
+
+  if (!NS_IsMainThread()) {
+    // Dispatch mImage to main thread to prevent it from being destructed by the
+    // decode thread.
+    nsCOMPtr<nsIThread> mainThread = do_GetMainThread();
+    NS_WARN_IF_FALSE(mainThread, "Couldn't get the main thread!");
+    if (mainThread) {
+      // Handle ambiguous nsISupports inheritance.
+      RasterImage* rawImg = nullptr;
+      mImage.swap(rawImg);
+      DebugOnly<nsresult> rv =
+        NS_ProxyRelease(mainThread, NS_ISUPPORTS_CAST(ImageResource*, rawImg));
+      MOZ_ASSERT(NS_SUCCEEDED(rv), "Failed to proxy release to main thread");
+    }
+  }
 }
 
 /*
  * Common implementation of the decoder interface.
  */
 
 void
 Decoder::Init()
@@ -125,23 +140,23 @@ Decoder::Finish(ShutdownReason aReason)
     // Log data errors to the error console
     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: ") +
-                       NS_ConvertUTF8toUTF16(mImage.GetURIString()));
+                       NS_ConvertUTF8toUTF16(mImage->GetURIString()));
 
       if (NS_SUCCEEDED(errorObject->InitWithWindowID(
                          msg,
-                         NS_ConvertUTF8toUTF16(mImage.GetURIString()),
+                         NS_ConvertUTF8toUTF16(mImage->GetURIString()),
                          EmptyString(), 0, 0, nsIScriptError::errorFlag,
-                         "Image", mImage.InnerWindowID()
+                         "Image", mImage->InnerWindowID()
                        ))) {
         consoleService->LogMessage(errorObject);
       }
     }
 
     bool usable = !HasDecoderError();
     if (aReason != ShutdownReason::NOT_NEEDED && !HasDecoderError()) {
       // If we only have a data error, we're usable if we have at least one complete frame.
@@ -162,21 +177,21 @@ Decoder::Finish(ShutdownReason aReason)
         mProgress |= FLAG_DECODE_COMPLETE | FLAG_ONLOAD_UNBLOCKED;
       }
       mProgress |= FLAG_HAS_ERROR;
     }
   }
 
   // Set image metadata before calling DecodingComplete, because
   // DecodingComplete calls Optimize().
-  mImageMetadata.SetOnImage(&mImage);
+  mImageMetadata.SetOnImage(mImage);
 
   if (mDecodeDone) {
     MOZ_ASSERT(HasError() || mCurrentFrame, "Should have an error or a frame");
-    mImage.DecodingComplete(mCurrentFrame.get(), mIsAnimated);
+    mImage->DecodingComplete(mCurrentFrame.get(), mIsAnimated);
   }
 }
 
 nsresult
 Decoder::AllocateFrame(uint32_t aFrameNum,
                        const nsIntRect& aFrameRect,
                        gfx::SurfaceFormat aFormat,
                        uint8_t aPaletteDepth /* = 0 */)
@@ -241,17 +256,17 @@ Decoder::AllocateFrameInternal(uint32_t 
 
   RawAccessFrameRef ref = frame->RawAccessRef();
   if (!ref) {
     frame->Abort();
     return RawAccessFrameRef();
   }
 
   InsertOutcome outcome =
-    SurfaceCache::Insert(frame, ImageKey(&mImage),
+    SurfaceCache::Insert(frame, ImageKey(mImage.get()),
                          RasterSurfaceKey(imageSize.ToIntSize(),
                                           aDecodeFlags,
                                           aFrameNum),
                          Lifetime::Persistent);
   if (outcome != InsertOutcome::SUCCESS) {
     ref->Abort();
     return RawAccessFrameRef();
   }
@@ -277,30 +292,30 @@ Decoder::AllocateFrameInternal(uint32_t 
     ref->SetRawAccessOnly();
 
     // Some GIFs are huge but only have a small area that they animate. We only
     // need to refresh that small area when frame 0 comes around again.
     refreshArea.UnionRect(refreshArea, frame->GetRect());
   }
 
   mFrameCount++;
-  mImage.OnAddedFrame(mFrameCount, refreshArea);
+  mImage->OnAddedFrame(mFrameCount, refreshArea);
 
   return ref;
 }
 
 void
 Decoder::SetSizeOnImage()
 {
   MOZ_ASSERT(mImageMetadata.HasSize(), "Should have size");
   MOZ_ASSERT(mImageMetadata.HasOrientation(), "Should have orientation");
 
-  mImage.SetSize(mImageMetadata.GetWidth(),
-                 mImageMetadata.GetHeight(),
-                 mImageMetadata.GetOrientation());
+  mImage->SetSize(mImageMetadata.GetWidth(),
+                  mImageMetadata.GetHeight(),
+                  mImageMetadata.GetOrientation());
 }
 
 /*
  * Hook stubs. Override these as necessary in decoder implementations.
  */
 
 void Decoder::InitInternal() { }
 void Decoder::WriteInternal(const char* aBuffer, uint32_t aCount) { }
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -17,17 +17,17 @@
 namespace mozilla {
 
 namespace image {
 
 class Decoder
 {
 public:
 
-  explicit Decoder(RasterImage& aImage);
+  explicit Decoder(RasterImage* aImage);
 
   /**
    * Initialize an image decoder. Decoders may not be re-initialized.
    *
    * Notifications Sent: TODO
    */
   void Init();
 
@@ -155,16 +155,21 @@ public:
   bool HasSize() const { return mImageMetadata.HasSize(); }
   void SetSizeOnImage();
 
   // Use HistogramCount as an invalid Histogram ID
   virtual Telemetry::ID SpeedHistogram() { return Telemetry::HistogramCount; }
 
   ImageMetadata& GetImageMetadata() { return mImageMetadata; }
 
+  /**
+   * Returns a weak pointer to the image associated with this decoder.
+   */
+  RasterImage* GetImage() const { MOZ_ASSERT(mImage); return mImage.get(); }
+
   already_AddRefed<imgFrame> GetCurrentFrame()
   {
     nsRefPtr<imgFrame> frame = mCurrentFrame.get();
     return frame.forget();
   }
 
   RawAccessFrameRef GetCurrentFrameRef()
   {
@@ -258,17 +263,17 @@ protected:
                                           gfx::SurfaceFormat aFormat,
                                           uint8_t aPaletteDepth,
                                           imgFrame* aPreviousFrame);
 
   /*
    * Member variables.
    *
    */
-  RasterImage &mImage;
+  nsRefPtr<RasterImage> mImage;
   RawAccessFrameRef mCurrentFrame;
   ImageMetadata mImageMetadata;
   nsIntRect mInvalidRect; // Tracks an invalidation region in the current frame.
   Progress mProgress;
 
   uint8_t* mImageData;       // Pointer to image data in either Cairo or 8bit format
   uint32_t mImageDataLength;
   uint32_t* mColormap;       // Current colormap to be used in Cairo format
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -1469,42 +1469,42 @@ RasterImage::InitDecoder(bool aDoSizeDec
   if (!aDoSizeDecode) {
     NS_ABORT_IF_FALSE(mHasSize, "Must do a size decode before a full decode!");
   }
 
   // Figure out which decoder we want
   eDecoderType type = GetDecoderType(mSourceDataMimeType.get());
   CONTAINER_ENSURE_TRUE(type != eDecoderType_unknown, NS_IMAGELIB_ERROR_NO_DECODER);
 
-  // Instantiate the appropriate decoder
+  // Instantiate the appropriate decoder.
   switch (type) {
     case eDecoderType_png:
-      mDecoder = new nsPNGDecoder(*this);
+      mDecoder = new nsPNGDecoder(this);
       break;
     case eDecoderType_gif:
-      mDecoder = new nsGIFDecoder2(*this);
+      mDecoder = new nsGIFDecoder2(this);
       break;
     case eDecoderType_jpeg:
       // If we have all the data we don't want to waste cpu time doing
-      // a progressive decode
-      mDecoder = new nsJPEGDecoder(*this,
+      // a progressive decode.
+      mDecoder = new nsJPEGDecoder(this,
                                    mHasBeenDecoded ? Decoder::SEQUENTIAL :
                                                      Decoder::PROGRESSIVE);
       break;
     case eDecoderType_bmp:
-      mDecoder = new nsBMPDecoder(*this);
+      mDecoder = new nsBMPDecoder(this);
       break;
     case eDecoderType_ico:
-      mDecoder = new nsICODecoder(*this);
+      mDecoder = new nsICODecoder(this);
       break;
     case eDecoderType_icon:
-      mDecoder = new nsIconDecoder(*this);
+      mDecoder = new nsIconDecoder(this);
       break;
     default:
-      NS_ABORT_IF_FALSE(0, "Shouldn't get here!");
+      MOZ_ASSERT_UNREACHABLE("Unknown decoder type");
   }
 
   // Initialize the decoder
   mDecoder->SetSizeDecode(aDoSizeDecode);
   mDecoder->SetDecodeFlags(mFrameDecodeFlags);
   mDecoder->SetSendPartialInvalidations(!mHasBeenDecoded);
   mDecoder->Init();
   CONTAINER_ENSURE_SUCCESS(mDecoder->GetDecoderError());