Bug 1187386 (Part 2) - Rework decoder code to avoid calling Decode::GetImage(). r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 31 Jul 2015 07:29:03 -0700
changeset 255697 bf702d0ae9c8d9a830a86b212ef8a40664b3226e
parent 255696 27a64c1be104819e9bd908eef736f4da6334b4d3
child 255698 6d193bf1559a8d331c5a007cfe4b8b8c5531c33f
push id63115
push usermfowler@mozilla.com
push dateFri, 31 Jul 2015 20:11:05 +0000
treeherdermozilla-inbound@9c768fc28bb0 [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 2) - Rework decoder code to avoid calling Decode::GetImage(). r=tn
image/Decoder.h
image/DecoderFactory.cpp
image/DecoderFactory.h
image/ImageFactory.cpp
image/RasterImage.cpp
image/RasterImage.h
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
image/decoders/nsJPEGDecoder.cpp
image/decoders/nsJPEGDecoder.h
image/decoders/nsPNGDecoder.cpp
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -123,16 +123,32 @@ public:
    * This must be called before Init() is called.
    */
   virtual nsresult SetTargetSize(const nsIntSize& aSize)
   {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   /**
+   * Set the requested sample size for this decoder. Used to implement the
+   * -moz-sample-size media fragment.
+   *
+   *  XXX(seth): Support for -moz-sample-size will be removed in bug 1120056.
+   */
+  virtual void SetSampleSize(int aSampleSize) { }
+
+  /**
+   * Set the requested resolution for this decoder. Used to implement the
+   * -moz-resolution media fragment.
+   *
+   *  XXX(seth): Support for -moz-resolution will be removed in bug 1118926.
+   */
+  virtual void SetResolution(const gfx::IntSize& aResolution) { }
+
+  /**
    * Set whether should send partial invalidations.
    *
    * If @aSend is true, we'll send partial invalidations when decoding the first
    * frame of the image, so image notifications observers will be able to
    * gradually draw in the image as it downloads.
    *
    * If @aSend is false (the default), we'll only send an invalidation when we
    * complete the first frame.
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -105,31 +105,35 @@ DecoderFactory::GetDecoder(DecoderType a
 }
 
 /* static */ already_AddRefed<Decoder>
 DecoderFactory::CreateDecoder(DecoderType aType,
                               RasterImage* aImage,
                               SourceBuffer* aSourceBuffer,
                               const Maybe<IntSize>& aTargetSize,
                               uint32_t aFlags,
+                              int aSampleSize,
+                              const IntSize& aResolution,
                               bool aIsRedecode,
                               bool aImageIsTransient,
                               bool aImageIsLocked)
 {
   if (aType == DecoderType::UNKNOWN) {
     return nullptr;
   }
 
   nsRefPtr<Decoder> decoder = GetDecoder(aType, aImage, aIsRedecode);
   MOZ_ASSERT(decoder, "Should have a decoder now");
 
   // Initialize the decoder.
   decoder->SetMetadataDecode(false);
   decoder->SetIterator(aSourceBuffer->Iterator());
   decoder->SetFlags(aFlags);
+  decoder->SetSampleSize(aSampleSize);
+  decoder->SetResolution(aResolution);
   decoder->SetSendPartialInvalidations(!aIsRedecode);
   decoder->SetImageIsTransient(aImageIsTransient);
 
   if (aImageIsLocked) {
     decoder->SetImageIsLocked();
   }
 
   // Set a target size for downscale-during-decode if applicable.
@@ -146,29 +150,33 @@ DecoderFactory::CreateDecoder(DecoderTyp
   }
 
   return decoder.forget();
 }
 
 /* static */ already_AddRefed<Decoder>
 DecoderFactory::CreateMetadataDecoder(DecoderType aType,
                                       RasterImage* aImage,
-                                      SourceBuffer* aSourceBuffer)
+                                      SourceBuffer* aSourceBuffer,
+                                      int aSampleSize,
+                                      const IntSize& aResolution)
 {
   if (aType == DecoderType::UNKNOWN) {
     return nullptr;
   }
 
   nsRefPtr<Decoder> decoder =
     GetDecoder(aType, aImage, /* aIsRedecode = */ false);
   MOZ_ASSERT(decoder, "Should have a decoder now");
 
   // Initialize the decoder.
   decoder->SetMetadataDecode(true);
   decoder->SetIterator(aSourceBuffer->Iterator());
+  decoder->SetSampleSize(aSampleSize);
+  decoder->SetResolution(aResolution);
 
   decoder->Init();
   if (NS_FAILED(decoder->GetDecoderError())) {
     return nullptr;
   }
 
   return decoder.forget();
 }
--- a/image/DecoderFactory.h
+++ b/image/DecoderFactory.h
@@ -51,48 +51,60 @@ public:
    * @param aSourceBuffer The SourceBuffer which the decoder will read its data
    *                      from.
    * @param aTargetSize If not Nothing(), the target size which the image should
    *                    be scaled to during decoding. It's an error to specify
    *                    a target size for a decoder type which doesn't support
    *                    downscale-during-decode.
    * @param aFlags Flags specifying what type of output the decoder should
    *               produce; see GetDecodeFlags() in RasterImage.h.
+   * @param aSampleSize The sample size requested using #-moz-samplesize (or 0
+   *                    if none).
+   * @param aResolution The resolution requested using #-moz-resolution (or an
+   *                    empty rect if none).
    * @param aIsRedecode Specify 'true' if this image has been decoded before.
    * @param aImageIsTransient Specify 'true' if this image is transient.
    * @param aImageIsLocked Specify 'true' if this image is locked for the
    *                       lifetime of this decoder, and should be unlocked
    *                       when the decoder finishes.
    */
   static already_AddRefed<Decoder>
   CreateDecoder(DecoderType aType,
                 RasterImage* aImage,
                 SourceBuffer* aSourceBuffer,
                 const Maybe<gfx::IntSize>& aTargetSize,
                 uint32_t aFlags,
+                int aSampleSize,
+                const gfx::IntSize& aResolution,
                 bool aIsRedecode,
                 bool aImageIsTransient,
                 bool aImageIsLocked);
 
   /**
    * Creates and initializes a metadata decoder of type @aType. This decoder
    * will only decode the image's header, extracting metadata like the size of
    * the image. No actual image data will be decoded and no surfaces will be
    * allocated. The decoder will send notifications to @aImage.
    *
    * @param aType Which type of decoder to create - JPEG, PNG, etc.
    * @param aImage The image will own the decoder and which should receive
    *               notifications as decoding progresses.
    * @param aSourceBuffer The SourceBuffer which the decoder will read its data
    *                      from.
+   * @param aSampleSize The sample size requested using #-moz-samplesize (or 0
+   *                    if none).
+   * @param aResolution The resolution requested using #-moz-resolution (or an
+   *                    empty rect if none).
    */
   static already_AddRefed<Decoder>
   CreateMetadataDecoder(DecoderType aType,
                         RasterImage* aImage,
-                        SourceBuffer* aSourceBuffer);
+                        SourceBuffer* aSourceBuffer,
+                        int aSampleSize,
+                        const gfx::IntSize& aResolution);
 
 private:
   virtual ~DecoderFactory() = 0;
 
   /**
    * An internal method which allocates a new decoder of the requested @aType.
    */
   static already_AddRefed<Decoder> GetDecoder(DecoderType aType,
--- a/image/ImageFactory.cpp
+++ b/image/ImageFactory.cpp
@@ -216,16 +216,39 @@ ImageFactory::CreateRasterImage(nsIReque
   MOZ_ASSERT(aProgressTracker);
 
   nsresult rv;
 
   nsRefPtr<RasterImage> newImage = new RasterImage(aURI);
   aProgressTracker->SetImage(newImage);
   newImage->SetProgressTracker(aProgressTracker);
 
+  nsAutoCString ref;
+  aURI->GetRef(ref);
+  net::nsMediaFragmentURIParser parser(ref);
+  if (parser.HasResolution()) {
+    newImage->SetRequestedResolution(parser.GetResolution());
+  }
+
+  if (parser.HasSampleSize()) {
+      /* Get our principal */
+      nsCOMPtr<nsIChannel> chan(do_QueryInterface(aRequest));
+      nsCOMPtr<nsIPrincipal> principal;
+      if (chan) {
+        nsContentUtils::GetSecurityManager()
+          ->GetChannelResultPrincipal(chan, getter_AddRefs(principal));
+      }
+
+      if ((principal &&
+           principal->GetAppStatus() == nsIPrincipal::APP_STATUS_CERTIFIED) ||
+          gfxPrefs::ImageMozSampleSizeEnabled()) {
+        newImage->SetRequestedSampleSize(parser.GetSampleSize());
+      }
+  }
+
   rv = newImage->Init(aMimeType.get(), aImageFlags);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
 
   newImage->SetInnerWindowID(aInnerWindowId);
 
   uint32_t len = GetContentSize(aRequest);
 
   // Pass anything usable on so that the RasterImage can preallocate
@@ -240,39 +263,16 @@ ImageFactory::CreateRasterImage(nsIReque
       nsresult rv2 = newImage->SetSourceSizeHint(sizeHint);
       // If we've still failed at this point, things are going downhill.
       if (NS_FAILED(rv) || NS_FAILED(rv2)) {
         NS_WARNING("About to hit OOM in imagelib!");
       }
     }
   }
 
-  nsAutoCString ref;
-  aURI->GetRef(ref);
-  net::nsMediaFragmentURIParser parser(ref);
-  if (parser.HasResolution()) {
-    newImage->SetRequestedResolution(parser.GetResolution());
-  }
-
-  if (parser.HasSampleSize()) {
-      /* Get our principal */
-      nsCOMPtr<nsIChannel> chan(do_QueryInterface(aRequest));
-      nsCOMPtr<nsIPrincipal> principal;
-      if (chan) {
-        nsContentUtils::GetSecurityManager()
-          ->GetChannelResultPrincipal(chan, getter_AddRefs(principal));
-      }
-
-      if ((principal &&
-           principal->GetAppStatus() == nsIPrincipal::APP_STATUS_CERTIFIED) ||
-          gfxPrefs::ImageMozSampleSizeEnabled()) {
-        newImage->SetRequestedSampleSize(parser.GetSampleSize());
-      }
-  }
-
   return newImage.forget();
 }
 
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateVectorImage(nsIRequest* aRequest,
                                 ProgressTracker* aProgressTracker,
                                 const nsCString& aMimeType,
                                 ImageURL* aURI,
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1440,18 +1440,18 @@ RasterImage::Decode(const IntSize& aSize
     // The corresponding unlock happens in FinalizeDecoder.
     LockImage();
     imageIsLocked = true;
   }
 
   // Create a decoder.
   nsRefPtr<Decoder> decoder =
     DecoderFactory::CreateDecoder(mDecoderType, this, mSourceBuffer, targetSize,
-                                  aFlags, mHasBeenDecoded, mTransient,
-                                  imageIsLocked);
+                                  aFlags, mRequestedSampleSize, mRequestedResolution,
+                                  mHasBeenDecoded, mTransient, imageIsLocked);
 
   // Make sure DecoderFactory was able to create a decoder successfully.
   if (!decoder) {
     return NS_ERROR_FAILURE;
   }
 
   // Add a placeholder for the first frame to the SurfaceCache so we won't
   // trigger any more decoders with the same parameters.
@@ -1494,17 +1494,19 @@ RasterImage::DecodeMetadata(uint32_t aFl
   if (mError) {
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(!mHasSize, "Should not do unnecessary metadata decodes");
 
   // Create a decoder.
   nsRefPtr<Decoder> decoder =
-    DecoderFactory::CreateMetadataDecoder(mDecoderType, this, mSourceBuffer);
+    DecoderFactory::CreateMetadataDecoder(mDecoderType, this, mSourceBuffer,
+                                          mRequestedSampleSize,
+                                          mRequestedResolution);
 
   // Make sure DecoderFactory was able to create a decoder successfully.
   if (!decoder) {
     return NS_ERROR_FAILURE;
   }
 
   // We're ready to decode; start the decoder.
   LaunchDecoder(decoder, this, aFlags, mHasSourceData);
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -264,28 +264,21 @@ public:
    */
   nsresult SetSourceSizeHint(uint32_t aSizeHint);
 
   /* Provide a hint for the requested resolution of the resulting image. */
   void SetRequestedResolution(const nsIntSize requestedResolution) {
     mRequestedResolution = requestedResolution;
   }
 
-  nsIntSize GetRequestedResolution() {
-    return mRequestedResolution;
-  }
   /* Provide a hint for the requested dimension of the resulting image. */
   void SetRequestedSampleSize(int requestedSampleSize) {
     mRequestedSampleSize = requestedSampleSize;
   }
 
-  int GetRequestedSampleSize() {
-    return mRequestedSampleSize;
-  }
-
  nsCString GetURIString() {
     nsCString spec;
     if (GetURI()) {
       GetURI()->GetSpec(spec);
     }
     return spec;
   }
 
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -261,19 +261,20 @@ nsICODecoder::WriteInternal(const char* 
     aCount -= 2;
   }
 
   if (mNumIcons == 0) {
     return; // Nothing to do.
   }
 
   uint16_t colorDepth = 0;
-  nsIntSize prefSize = mImage->GetRequestedResolution();
-  if (prefSize.width == 0 && prefSize.height == 0) {
-    prefSize.SizeTo(PREFICONSIZE, PREFICONSIZE);
+
+  // If we didn't get a #-moz-resolution, default to PREFICONSIZE.
+  if (mResolution.width == 0 && mResolution.height == 0) {
+    mResolution.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).
   int32_t diff = INT_MIN;
 
@@ -301,18 +302,18 @@ nsICODecoder::WriteInternal(const char* 
                 (mCurrIcon * sizeof(mDirEntryArray))) {
       mCurrIcon++;
       ProcessDirEntry(e);
       // We can't use GetRealWidth and GetRealHeight here because those operate
       // on mDirEntry, here we are going through each item in the directory.
       // Calculate the delta between this image's size and the desired size,
       // so we can see if it is better than our current-best option.
       // In the case of several equally-good images, we use the last one.
-      int32_t delta = (e.mWidth == 0 ? 256 : e.mWidth) - prefSize.width +
-                      (e.mHeight == 0 ? 256 : e.mHeight) - prefSize.height;
+      int32_t delta = (e.mWidth == 0 ? 256 : e.mWidth) - mResolution.width +
+                      (e.mHeight == 0 ? 256 : e.mHeight) - mResolution.height;
       if (e.mBitCount >= colorDepth &&
           ((diff < 0 && delta >= diff) || (delta >= 0 && delta <= diff))) {
         diff = delta;
         mImageOffset = e.mImageOffset;
 
         // ensure mImageOffset is >= size of the direntry headers (bug #245631)
         uint32_t minImageOffset = DIRENTRYOFFSET +
                                   mNumIcons * sizeof(mDirEntryArray);
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -31,16 +31,21 @@ public:
   }
 
   // Obtains the height of the icon directory entry
   uint32_t GetRealHeight() const
   {
     return mDirEntry.mHeight == 0 ? 256 : mDirEntry.mHeight;
   }
 
+  virtual void SetResolution(const gfx::IntSize& aResolution) override
+  {
+    mResolution = aResolution;
+  }
+
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual void FinishInternal() override;
   virtual void FinishWithErrorInternal() override;
 
 private:
   friend class DecoderFactory;
 
   // Decoders should only be instantiated via DecoderFactory.
@@ -71,16 +76,17 @@ private:
   int32_t ExtractBIHSizeFromBitmap(int8_t* bih);
   // Extract bit count from BMP information header
   int32_t ExtractBPPFromBitmap(int8_t* bih);
   // Calculates the row size in bytes for the AND mask table
   uint32_t CalcAlphaRowSize();
   // Obtains the number of colors from the BPP, mBPP must be filled in
   uint16_t GetNumColors();
 
+  gfx::IntSize mResolution;  // The requested -moz-resolution for this icon.
   uint16_t mBPP; // Stores the images BPP
   uint32_t mPos; // Keeps track of the position we have decoded up until
   uint16_t mNumIcons; // Stores the number of icons in the ICO file
   uint16_t mCurrIcon; // Stores the current dir entry index we are processing
   uint32_t mImageOffset; // Stores the offset of the image data we want
   uint8_t* mRow;      // Holds one raw line of the image
   int32_t mCurLine;   // Line index of the image that's currently being decoded
   uint32_t mRowBytes; // How many bytes of the row were already received
--- a/image/decoders/nsJPEGDecoder.cpp
+++ b/image/decoders/nsJPEGDecoder.cpp
@@ -79,16 +79,17 @@ METHODDEF(void) my_error_exit (j_common_
 
 // Normal JFIF markers can't have more bytes than this.
 #define MAX_JPEG_MARKER_LENGTH  (((uint32_t)1 << 16) - 1)
 
 nsJPEGDecoder::nsJPEGDecoder(RasterImage* aImage,
                              Decoder::DecodeStyle aDecodeStyle)
  : Decoder(aImage)
  , mDecodeStyle(aDecodeStyle)
+ , mSampleSize(0)
 {
   mState = JPEG_HEADER;
   mReading = true;
   mImageData = nullptr;
 
   mBytesToSkip = 0;
   memset(&mInfo, 0, sizeof(jpeg_decompress_struct));
   memset(&mSourceMgr, 0, sizeof(mSourceMgr));
@@ -243,20 +244,20 @@ nsJPEGDecoder::WriteInternal(const char*
 
       // Step 3: read file parameters with jpeg_read_header()
       if (jpeg_read_header(&mInfo, TRUE) == JPEG_SUSPENDED) {
         MOZ_LOG(GetJPEGDecoderAccountingLog(), LogLevel::Debug,
                ("} (JPEG_SUSPENDED)"));
         return; // I/O suspension
       }
 
-      int sampleSize = GetImage()->GetRequestedSampleSize();
-      if (sampleSize > 0) {
+      // If we have a sample size specified for -moz-sample-size, use it.
+      if (mSampleSize > 0) {
         mInfo.scale_num = 1;
-        mInfo.scale_denom = sampleSize;
+        mInfo.scale_denom = mSampleSize;
       }
 
       // Used to set up image size so arrays can be allocated
       jpeg_calc_output_dimensions(&mInfo);
 
       // Post our size to the superclass
       PostSize(mInfo.output_width, mInfo.output_height,
                ReadOrientationFromEXIF());
--- a/image/decoders/nsJPEGDecoder.h
+++ b/image/decoders/nsJPEGDecoder.h
@@ -52,16 +52,21 @@ struct Orientation;
 
 class nsJPEGDecoder : public Decoder
 {
 public:
   virtual ~nsJPEGDecoder();
 
   virtual nsresult SetTargetSize(const nsIntSize& aSize) override;
 
+  virtual void SetSampleSize(int aSampleSize) override
+  {
+    mSampleSize = aSampleSize;
+  }
+
   virtual void InitInternal() override;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual void FinishInternal() override;
 
   virtual Telemetry::ID SpeedHistogram() override;
   void NotifyDone();
 
 protected:
@@ -98,14 +103,16 @@ public:
   qcms_profile* mInProfile;
   qcms_transform* mTransform;
 
   bool mReading;
 
   const Decoder::DecodeStyle mDecodeStyle;
 
   uint32_t mCMSMode;
+
+  int mSampleSize;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_decoders_nsJPEGDecoder_h
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -171,19 +171,18 @@ nsPNGDecoder::CreateFrame(png_uint_32 aX
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   mFrameRect = frameRect;
 
   MOZ_LOG(GetPNGDecoderAccountingLog(), LogLevel::Debug,
          ("PNGDecoderAccounting: nsPNGDecoder::CreateFrame -- created "
-          "image frame with %dx%d pixels in container %p",
-          aWidth, aHeight,
-          GetImage()));
+          "image frame with %dx%d pixels for decoder %p",
+          aWidth, aHeight, this));
 
 #ifdef PNG_APNG_SUPPORTED
   if (png_get_valid(mPNG, mInfo, PNG_INFO_acTL)) {
     mAnimInfo = AnimFrameInfo(mPNG, mInfo);
 
     if (mAnimInfo.mDispose == DisposalMethod::CLEAR) {
       // We may have to display the background under this image during
       // animation playback, so we regard it as transparent.