Bug 1079627 (Part 1) - Make image decoders hold a strong reference to their image. r=tn a=lmandel
authorSeth Fowler <seth@mozilla.com>
Thu, 15 Jan 2015 15:11:35 -0800
changeset 249804 9051a8b7ee5a9876c84fb63fd5dd26755b6a1a8d
parent 249803 2e3aaa26bca4df3c91383708135d179467db4493
child 249805 2e0e58532c2de49805d53a7b606304ccb57059d9
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, lmandel
bugs1079627
milestone37.0a2
Bug 1079627 (Part 1) - Make image decoders hold a strong reference to their image. r=tn a=lmandel
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
 }
 
@@ -244,17 +244,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;
 
   void CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
                    int32_t width, int32_t height,
--- 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)
@@ -43,16 +43,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()
@@ -176,23 +191,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.
@@ -213,21 +228,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());
+    mImage->DecodingComplete(mCurrentFrame.get());
   }
 }
 
 void
 Decoder::FinishSharedDecoder()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -364,17 +379,17 @@ Decoder::InternalAddFrame(uint32_t aFram
 
   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();
   }
@@ -400,30 +415,30 @@ Decoder::InternalAddFrame(uint32_t aFram
     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();
 
@@ -182,16 +182,21 @@ public:
     PostSize(aSize.width, aSize.height, aOrientation);
   }
 
   // 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(); }
+
   // Tell the decoder infrastructure to allocate a frame. By default, frame 0
   // is created as an ARGB frame with no offset and with size width * height.
   // If decoders need something different, they must ask for it.
   // This is called by decoders when they need a new frame. These decoders
   // must then save the data they have been sent but not yet processed and
   // return from WriteInternal. When the new frame is created, WriteInternal
   // will be called again with nullptr and 0 as arguments.
   void NeedNewFrame(uint32_t frameNum, uint32_t x_offset, uint32_t y_offset,
@@ -307,17 +312,17 @@ protected:
                                      uint32_t aDecodeFlags,
                                      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
@@ -1448,42 +1448,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);
   if (!aDoSizeDecode) {
     // We already have the size; tell the decoder so it can preallocate a
     // frame.  By default, we create an ARGB frame with no offset. If decoders