Bug 1079627 (Part 1) - Make image decoders hold a strong reference to their image. r=tn
☠☠ backed out by cb26891d69e9 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Mon, 12 Jan 2015 01:20:22 -0800
changeset 249143 c86c43915254b769d8b63cdf997a159d32905055
parent 249142 b096ec537e81ffa5a936761eb6f0e1d317fef0e3
child 249144 44b622a479b60c6ecd1883a84b782e54a44221a7
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
 }
 
@@ -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
@@ -1453,42 +1453,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