Bug 1184996 (Part 4) - Forbid instantiation of decoders except via DecoderFactory. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 20 Jul 2015 18:54:21 -0700
changeset 509951 782bb88fb13de061cd0c47b31e7659932fb39be4
parent 509950 75eae14edb825baf5f1c546a016eeee496242453
child 509952 327073350d1217ad3f145b5724c4bf6234ded234
push id79148
push usermfowler@mozilla.com
push dateTue, 21 Jul 2015 01:55:43 +0000
treeherdertry@327073350d12 [default view] [failures only]
reviewerstn
bugs1184996
milestone42.0a1
Bug 1184996 (Part 4) - Forbid instantiation of decoders except via DecoderFactory. r=tn
image/Decoder.cpp
image/Decoder.h
image/DecoderFactory.cpp
image/DecoderFactory.h
image/decoders/nsBMPDecoder.h
image/decoders/nsGIFDecoder2.h
image/decoders/nsICODecoder.h
image/decoders/nsIconDecoder.h
image/decoders/nsJPEGDecoder.h
image/decoders/nsPNGDecoder.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -419,17 +419,16 @@ Decoder::SetSizeOnImage()
   }
 }
 
 /*
  * Hook stubs. Override these as necessary in decoder implementations.
  */
 
 void Decoder::InitInternal() { }
-void Decoder::WriteInternal(const char* aBuffer, uint32_t aCount) { }
 void Decoder::FinishInternal() { }
 void Decoder::FinishWithErrorInternal() { }
 
 /*
  * Progress Notifications
  */
 
 void
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -280,17 +280,17 @@ protected:
 
   virtual ~Decoder();
 
   /*
    * Internal hooks. Decoder implementations may override these and
    * only these methods.
    */
   virtual void InitInternal();
-  virtual void WriteInternal(const char* aBuffer, uint32_t aCount);
+  virtual void WriteInternal(const char* aBuffer, uint32_t aCount) = 0;
   virtual void FinishInternal();
   virtual void FinishWithErrorInternal();
 
   /*
    * Progress notifications.
    */
 
   // Called by decoders when they determine the size of the image. Informs
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -62,20 +62,20 @@ DecoderFactory::GetDecoderType(const cha
   // Icon
   } else if (!strcmp(aMimeType, IMAGE_ICON_MS)) {
     type = DecoderType::ICON;
   }
 
   return type;
 }
 
-static already_AddRefed<Decoder>
-GetDecoder(DecoderType aType,
-           RasterImage* aImage,
-           bool aIsRedecode)
+/* static */ already_AddRefed<Decoder>
+DecoderFactory::GetDecoder(DecoderType aType,
+                           RasterImage* aImage,
+                           bool aIsRedecode)
 {
   nsRefPtr<Decoder> decoder;
 
   switch (aType) {
     case DecoderType::PNG:
       decoder = new nsPNGDecoder(aImage);
       break;
     case DecoderType::GIF:
--- a/image/DecoderFactory.h
+++ b/image/DecoderFactory.h
@@ -81,14 +81,21 @@ public:
    */
   static already_AddRefed<Decoder>
   CreateMetadataDecoder(DecoderType aType,
                         RasterImage* aImage,
                         SourceBuffer* aSourceBuffer);
 
 private:
   virtual ~DecoderFactory() = 0;
+
+  /**
+   * An internal method which allocates a new decoder of the requested @aType.
+   */
+  static already_AddRefed<Decoder> GetDecoder(DecoderType aType,
+                                              RasterImage* aImage,
+                                              bool aIsRedecode);
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_DecoderFactory_h
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -17,18 +17,16 @@ namespace image {
 
 class RasterImage;
 
 /// Decoder for BMP-Files, as used by Windows and OS/2
 
 class nsBMPDecoder : public Decoder
 {
 public:
-
-    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
@@ -50,16 +48,22 @@ public:
     // for 32BPP bitmaps.  Only use after the bitmap has been processed.
     bool HasAlphaData() const;
 
     virtual void WriteInternal(const char* aBuffer,
                                uint32_t aCount) override;
     virtual void FinishInternal() override;
 
 private:
+    friend class DecoderFactory;
+    friend class nsICODecoder;
+
+    // Decoders should only be instantiated via DecoderFactory.
+    // XXX(seth): nsICODecoder is temporarily an exception to this rule.
+    explicit nsBMPDecoder(RasterImage* aImage);
 
     /// Calculates the red-, green- and blueshift in mBitFields using
     /// the bitmasks from mBitFields
     NS_METHOD CalcBitShift();
 
     uint32_t mPos; //< Number of bytes read from aBuffer in WriteInternal()
 
     BMPFILEHEADER mBFH;
--- a/image/decoders/nsGIFDecoder2.h
+++ b/image/decoders/nsGIFDecoder2.h
@@ -17,28 +17,30 @@ namespace image {
 class RasterImage;
 
 //////////////////////////////////////////////////////////////////////
 // nsGIFDecoder2 Definition
 
 class nsGIFDecoder2 : public Decoder
 {
 public:
-
-  explicit nsGIFDecoder2(RasterImage* aImage);
   ~nsGIFDecoder2();
 
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual void FinishInternal() override;
   virtual Telemetry::ID SpeedHistogram() override;
 
 private:
+  friend class DecoderFactory;
+
+  // Decoders should only be instantiated via DecoderFactory.
+  explicit nsGIFDecoder2(RasterImage* aImage);
+
   // These functions will be called when the decoder has a decoded row,
   // frame size information, etc.
-
   void      BeginGIF();
   nsresult  BeginImageFrame(uint16_t aDepth);
   void      EndImageFrame();
   void      FlushImageData();
   void      FlushImageData(uint32_t fromRow, uint32_t rows);
 
   nsresult  GifWrite(const uint8_t* buf, uint32_t numbytes);
   uint32_t  OutputRow();
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -17,18 +17,16 @@
 namespace mozilla {
 namespace image {
 
 class RasterImage;
 
 class nsICODecoder : public Decoder
 {
 public:
-
-  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;
   }
 
@@ -38,16 +36,21 @@ public:
     return mDirEntry.mHeight == 0 ? 256 : mDirEntry.mHeight;
   }
 
   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.
+  explicit nsICODecoder(RasterImage* aImage);
+
   // Writes to the contained decoder and sets the appropriate errors
   // Returns true if there are no errors.
   bool WriteToContainedDecoder(const char* aBuffer, uint32_t aCount);
 
   // Gets decoder state from the contained decoder so it's visible externally.
   void GetFinalStateFromContainedDecoder();
 
   // Processes a single dir entry of the icon resource
--- a/image/decoders/nsIconDecoder.h
+++ b/image/decoders/nsIconDecoder.h
@@ -32,22 +32,27 @@ class RasterImage;
 //       premultiplied alpha.
 //
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 class nsIconDecoder : public Decoder
 {
 public:
-
-  explicit nsIconDecoder(RasterImage* aImage);
   virtual ~nsIconDecoder();
 
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
 
+private:
+  friend class DecoderFactory;
+
+  // Decoders should only be instantiated via DecoderFactory.
+  explicit nsIconDecoder(RasterImage* aImage);
+
+public:
   uint8_t mWidth;
   uint8_t mHeight;
   uint32_t mPixBytesRead;
   uint32_t mState;
 };
 
 enum {
   iconStateStart      = 0,
--- a/image/decoders/nsJPEGDecoder.h
+++ b/image/decoders/nsJPEGDecoder.h
@@ -48,17 +48,16 @@ typedef enum {
 } jstate;
 
 class RasterImage;
 struct Orientation;
 
 class nsJPEGDecoder : public Decoder
 {
 public:
-  nsJPEGDecoder(RasterImage* aImage, Decoder::DecodeStyle aDecodeStyle);
   virtual ~nsJPEGDecoder();
 
   virtual nsresult SetTargetSize(const nsIntSize& aSize) override;
 
   virtual void InitInternal() override;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual void FinishInternal() override;
 
@@ -66,16 +65,22 @@ public:
   void NotifyDone();
 
 protected:
   Orientation ReadOrientationFromEXIF();
   void OutputScanlines(bool* suspend);
 
   Maybe<Downscaler> mDownscaler;
 
+private:
+  friend class DecoderFactory;
+
+  // Decoders should only be instantiated via DecoderFactory.
+  nsJPEGDecoder(RasterImage* aImage, Decoder::DecodeStyle aDecodeStyle);
+
 public:
   struct jpeg_decompress_struct mInfo;
   struct jpeg_source_mgr mSourceMgr;
   decoder_error_mgr mErr;
   jstate mState;
 
   uint32_t mBytesToSkip;
 
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -19,17 +19,16 @@
 
 namespace mozilla {
 namespace image {
 class RasterImage;
 
 class nsPNGDecoder : public Decoder
 {
 public:
-  explicit nsPNGDecoder(RasterImage* aImage);
   virtual ~nsPNGDecoder();
 
   virtual void InitInternal() override;
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual Telemetry::ID SpeedHistogram() override;
 
   nsresult CreateFrame(png_uint_32 aXOffset, png_uint_32 aYOffset,
                        int32_t aWidth, int32_t aHeight,
@@ -62,16 +61,24 @@ public:
       return ((png_color_type == PNG_COLOR_TYPE_RGB_ALPHA ||
                png_color_type == PNG_COLOR_TYPE_RGB) &&
               png_bit_depth == 8);
     } else {
       return false;
     }
   }
 
+private:
+  friend class DecoderFactory;
+  friend class nsICODecoder;
+
+  // Decoders should only be instantiated via DecoderFactory.
+  // XXX(seth): nsICODecoder is temporarily an exception to this rule.
+  explicit nsPNGDecoder(RasterImage* aImage);
+
 public:
   png_structp mPNG;
   png_infop mInfo;
   nsIntRect mFrameRect;
   uint8_t* mCMSLine;
   uint8_t* interlacebuf;
   qcms_profile* mInProfile;
   qcms_transform* mTransform;