Bug 816362 - Refactor imgTools::DecodeImageData to remove imgIContainer argument and use ImageFactory. r=joe
authorSeth Fowler <seth@mozilla.com>
Mon, 17 Dec 2012 17:35:07 -0800
changeset 116369 92b58637064edb3bd8ac5b869d1146db7ff7f1ee
parent 116368 b627cef1d7d2a7bf1cd0a9ee74defbcd0e114830
child 116370 ab006da8f88df746b52eeb1d580b1aa21a6ccf8c
push id24046
push useremorley@mozilla.com
push dateTue, 18 Dec 2012 09:32:10 +0000
treeherdermozilla-central@dd277d439d31 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe
bugs816362
milestone20.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 816362 - Refactor imgTools::DecodeImageData to remove imgIContainer argument and use ImageFactory. r=joe
image/public/imgITools.idl
image/src/ImageFactory.cpp
image/src/ImageFactory.h
image/src/imgTools.cpp
image/src/imgTools.h
image/test/unit/test_imgtools.js
--- a/image/public/imgITools.idl
+++ b/image/public/imgITools.idl
@@ -9,39 +9,54 @@
 interface nsIInputStream;
 interface imgIContainer;
 interface imgILoader;
 interface imgICache;
 interface nsIDOMDocument;
 interface imgIScriptedNotificationObserver;
 interface imgINotificationObserver;
 
-[scriptable, builtinclass, uuid(98bd5bf9-87eb-4d92-81b1-4cd10c64f7b2)]
+[scriptable, builtinclass, uuid(4c2383a4-931c-484d-8c4a-973590f66e3f)]
 interface imgITools : nsISupports
 {
     /**
+     * decodeImage
+     * Caller provides an input stream and mimetype. We read from the stream
+     * and decompress it (according to the specified mime type) and return
+     * the resulting imgIContainer.
+     *
+     * @param aStream
+     *        An input stream for an encoded image file.
+     * @param aMimeType
+     *        Type of image in the stream.
+     */
+    imgIContainer decodeImage(in nsIInputStream aStream,
+                              in ACString aMimeType);
+
+    /**
      * decodeImageData
      * Caller provides an input stream and mimetype. We read from the stream
      * and decompress it (according to the specified mime type) and return
-     * the resulting imgIContainer. (If the caller already has a container,
-     * it can be provided as input to be reused).
+     * the resulting imgIContainer.
+     *
+     * This method is deprecated and will be removed at some time in the future;
+     * new code should use |decodeImage|.
      *
      * @param aStream
      *        An input stream for an encoded image file.
      * @param aMimeType
      *        Type of image in the stream.
      * @param aContainer
-     *        An imgIContainer holding the decoded image. Specify |null| when
-     *        calling to have one created, otherwise specify a container to
-     *        be used. It is an error to pass an already-initialized container
-     *        as aContainer.
+     *        An imgIContainer holding the decoded image will be returned via
+     *        this parameter. It is an error to provide any initial value but
+     *        |null|.
      */
-    void decodeImageData(in nsIInputStream aStream,
-                         in ACString aMimeType,
-                         inout imgIContainer aContainer);
+    [deprecated] void decodeImageData(in nsIInputStream aStream,
+                                      in ACString aMimeType,
+                                      inout imgIContainer aContainer);
 
     /**
      * encodeImage
      * Caller provides an image container, and the mime type it should be
      * encoded to. We return an input stream for the encoded image data.
      *
      * @param aContainer
      *        An image container.
--- a/image/src/ImageFactory.cpp
+++ b/image/src/ImageFactory.cpp
@@ -112,16 +112,30 @@ template <typename T>
 static already_AddRefed<Image>
 BadImage(nsRefPtr<T>& image)
 {
   image->SetHasError();
   return image.forget();
 }
 
 /* static */ already_AddRefed<Image>
+ImageFactory::CreateAnonymousImage(const nsCString& aMimeType)
+{
+  nsresult rv;
+
+  nsRefPtr<RasterImage> newImage = new RasterImage();
+
+  rv = newImage->Init(nullptr, aMimeType.get(), "<unknown>", Image::INIT_FLAG_NONE);
+  NS_ENSURE_SUCCESS(rv, BadImage(newImage));
+
+  return newImage.forget();
+}
+
+/* static */ already_AddRefed<Image>
+
 ImageFactory::CreateRasterImage(nsIRequest* aRequest,
                                 imgStatusTracker* aStatusTracker,
                                 const nsCString& aMimeType,
                                 const nsCString& aURIString,
                                 uint32_t aImageFlags,
                                 uint32_t aInnerWindowId)
 {
   nsresult rv;
--- a/image/src/ImageFactory.h
+++ b/image/src/ImageFactory.h
@@ -30,16 +30,24 @@ struct ImageFactory
    * @param aInnerWindowId The window this image belongs to.
    */
   static already_AddRefed<Image> CreateImage(nsIRequest* aRequest,
                                              imgStatusTracker* aStatusTracker,
                                              const nsCString& aMimeType,
                                              nsIURI* aURI,
                                              bool aIsMultiPart,
                                              uint32_t aInnerWindowId);
+  /**
+   * Creates a new image which isn't associated with a URI or loaded through
+   * the usual image loading mechanism.
+   *
+   * @param aMimeType      The mimetype of the image.
+   */
+  static already_AddRefed<Image> CreateAnonymousImage(const nsCString& aMimeType);
+
 
 private:
   // Factory functions that create specific types of image containers.
   static already_AddRefed<Image> CreateRasterImage(nsIRequest* aRequest,
                                                    imgStatusTracker* aStatusTracker,
                                                    const nsCString& aMimeType,
                                                    const nsCString& aURIString,
                                                    uint32_t aImageFlags,
--- a/image/src/imgTools.cpp
+++ b/image/src/imgTools.cpp
@@ -17,17 +17,17 @@
 #include "gfxContext.h"
 #include "nsStringStream.h"
 #include "nsComponentManagerUtils.h"
 #include "nsWeakReference.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsStreamUtils.h"
 #include "nsNetUtil.h"
 #include "nsContentUtils.h"
-#include "RasterImage.h"
+#include "ImageFactory.h"
 #include "ScriptedNotificationObserver.h"
 #include "imgIScriptedNotificationObserver.h"
 
 using namespace mozilla::image;
 
 class nsIDOMDocument;
 class nsIDocument;
 
@@ -42,74 +42,66 @@ imgTools::imgTools()
   /* member initializers and constructor code */
 }
 
 imgTools::~imgTools()
 {
   /* destructor code */
 }
 
-
 NS_IMETHODIMP imgTools::DecodeImageData(nsIInputStream* aInStr,
                                         const nsACString& aMimeType,
                                         imgIContainer **aContainer)
 {
+  NS_ABORT_IF_FALSE(*aContainer == nullptr,
+                    "Cannot provide an existing image container to DecodeImageData");
+
+  return DecodeImage(aInStr, aMimeType, aContainer);
+}
+
+NS_IMETHODIMP imgTools::DecodeImage(nsIInputStream* aInStr,
+                                    const nsACString& aMimeType,
+                                    imgIContainer **aContainer)
+{
   nsresult rv;
-  RasterImage* image;  // convenience alias for *aContainer
+  nsRefPtr<Image> image;
 
   NS_ENSURE_ARG_POINTER(aInStr);
 
-  // XXX(seth) This needs to be switched over to use ImageFactory, but we need
-  // to be sure that no callers actually provide an existing imgIContainer first.
+  // Create a new image container to hold the decoded data.
+  nsAutoCString mimeType(aMimeType);
+  image = ImageFactory::CreateAnonymousImage(mimeType);
 
-  // If the caller didn't provide an imgIContainer, create one.
-  if (*aContainer) {
-    NS_ABORT_IF_FALSE((*aContainer)->GetType() == imgIContainer::TYPE_RASTER,
-                      "wrong type of imgIContainer for decoding into");
-    image = static_cast<RasterImage*>(*aContainer);
-  } else {
-    *aContainer = image = new RasterImage();
-    NS_ADDREF(image);
-  }
+  if (image->HasError())
+    return NS_ERROR_FAILURE;
 
-  // Initialize the Image. If we're using the one from the caller, we
-  // require that it not be initialized.
-  nsCString mimeType(aMimeType);
-  rv = image->Init(nullptr, mimeType.get(), "<unknown>", Image::INIT_FLAG_NONE);
-  NS_ENSURE_SUCCESS(rv, rv);
-
+  // Prepare the input stream.
   nsCOMPtr<nsIInputStream> inStream = aInStr;
   if (!NS_InputStreamIsBuffered(aInStr)) {
     nsCOMPtr<nsIInputStream> bufStream;
     rv = NS_NewBufferedInputStream(getter_AddRefs(bufStream), aInStr, 1024);
     if (NS_SUCCEEDED(rv))
       inStream = bufStream;
   }
 
-  // Figure out how much data we've been passed
+  // Figure out how much data we've been passed.
   uint64_t length;
   rv = inStream->Available(&length);
   NS_ENSURE_SUCCESS(rv, rv);
   NS_ENSURE_TRUE(length <= UINT32_MAX, NS_ERROR_FILE_TOO_BIG);
 
-  // Send the source data to the Image. WriteToRasterImage always
-  // consumes everything it gets if it doesn't run out of memory.
-  uint32_t bytesRead;
-  rv = inStream->ReadSegments(RasterImage::WriteToRasterImage,
-                              static_cast<void*>(image),
-                              (uint32_t)length, &bytesRead);
+  // Send the source data to the Image.
+  rv = image->OnImageDataAvailable(nullptr, nullptr, inStream, 0, uint32_t(length));
   NS_ENSURE_SUCCESS(rv, rv);
-  NS_ABORT_IF_FALSE(bytesRead == length || image->HasError(),
-  "WriteToRasterImage should consume everything or the image must be in error!");
-
-  // Let the Image know we've sent all the data
+  // Let the Image know we've sent all the data.
   rv = image->OnImageDataComplete(nullptr, nullptr, NS_OK);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // All done
+  // All done.
+  NS_ADDREF(*aContainer = image.get());
   return NS_OK;
 }
 
 
 NS_IMETHODIMP imgTools::EncodeImage(imgIContainer *aContainer,
                                     const nsACString& aMimeType,
                                     const nsAString& aOutputOptions,
                                     nsIInputStream **aStream)
--- a/image/src/imgTools.h
+++ b/image/src/imgTools.h
@@ -3,21 +3,21 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "imgITools.h"
 #include "gfxContext.h"
 
 #define NS_IMGTOOLS_CID \
-{ /* fd9a9e8a-a77b-496a-b7bb-263df9715149 */         \
-     0xfd9a9e8a,                                     \
-     0xa77b,                                         \
-     0x496a,                                         \
-    {0xb7, 0xbb, 0x26, 0x3d, 0xf9, 0x71, 0x51, 0x49} \
+{ /* 4c2383a4-931c-484d-8c4a-973590f66e3f */         \
+     0x4c2383a4,                                     \
+     0x931c,                                         \
+     0x484d,                                         \
+    {0x8c, 0x4a, 0x97, 0x35, 0x90, 0xf6, 0x6e, 0x3f} \
 }
 
 class imgTools : public imgITools
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_IMGITOOLS
 
--- a/image/test/unit/test_imgtools.js
+++ b/image/test/unit/test_imgtools.js
@@ -144,16 +144,18 @@ testdesc = "test decoding a PNG";
 // 64x64 png, 8415 bytes.
 var imgName = "image1.png";
 var inMimeType = "image/png";
 var imgFile = do_get_file(imgName);
 
 var istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 8415);
 
+// Use decodeImageData for this test even though it's deprecated to ensure that
+// it correctly forwards to decodeImage and continues to work.
 var outParam = { value: null };
 imgTools.decodeImageData(istream, inMimeType, outParam);
 var container = outParam.value;
 
 // It's not easy to look at the pixel values from JS, so just
 // check the container's size.
 do_check_eq(container.width,  64);
 do_check_eq(container.height, 64);
@@ -204,19 +206,17 @@ testdesc = "test decoding a JPEG";
 // 32x32 jpeg, 3494 bytes.
 imgName = "image2.jpg";
 inMimeType = "image/jpeg";
 imgFile = do_get_file(imgName);
 
 istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 3494);
 
-outParam = {};
-imgTools.decodeImageData(istream, inMimeType, outParam);
-container = outParam.value;
+container = imgTools.decodeImage(istream, inMimeType);
 
 // It's not easy to look at the pixel values from JS, so just
 // check the container's size.
 do_check_eq(container.width,  32);
 do_check_eq(container.height, 32);
 
 
 /* ========== 5 ========== */
@@ -268,19 +268,17 @@ testdesc = "test decoding a ICO";
 // 16x16 ico, 1406 bytes.
 imgName = "image3.ico";
 inMimeType = "image/x-icon";
 imgFile = do_get_file(imgName);
 
 istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 1406);
 
-outParam = { value: null };
-imgTools.decodeImageData(istream, inMimeType, outParam);
-container = outParam.value;
+container = imgTools.decodeImage(istream, inMimeType);
 
 // It's not easy to look at the pixel values from JS, so just
 // check the container's size.
 do_check_eq(container.width,  16);
 do_check_eq(container.height, 16);
 
 
 /* ========== 8 ========== */
@@ -328,19 +326,17 @@ testdesc = "test decoding a GIF";
 // 32x32 gif, 1809 bytes.
 imgName = "image4.gif";
 inMimeType = "image/gif";
 imgFile = do_get_file(imgName);
 
 istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 1809);
 
-outParam = { value: null };
-imgTools.decodeImageData(istream, inMimeType, outParam);
-container = outParam.value;
+container = imgTools.decodeImage(istream, inMimeType);
 
 // It's not easy to look at the pixel values from JS, so just
 // check the container's size.
 do_check_eq(container.width, 32);
 do_check_eq(container.height, 32);
 
 /* ========== 11 ========== */
 testnum++;
@@ -438,19 +434,17 @@ testdesc = "test cropping a JPG";
 // 32x32 jpeg, 3494 bytes.
 imgName = "image2.jpg";
 inMimeType = "image/jpeg";
 imgFile = do_get_file(imgName);
 
 istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 3494);
 
-outParam = {};
-imgTools.decodeImageData(istream, inMimeType, outParam);
-container = outParam.value;
+container = imgTools.decodeImage(istream, inMimeType);
 
 // It's not easy to look at the pixel values from JS, so just
 // check the container's size.
 do_check_eq(container.width,  32);
 do_check_eq(container.height, 32);
 
 // encode a cropped image
 istream = imgTools.encodeCroppedImage(container, "image/jpeg", 0, 0, 16, 16);
@@ -658,19 +652,17 @@ for(var i=0; i<testData.length; ++i) {
 
     var imgFile = do_get_file(dict["refImage"]);
     var istream = getFileInputStream(imgFile);
     var refBytes = streamToArray(istream);
 
     imgFile = do_get_file(dict["preImage"]);
     istream = getFileInputStream(imgFile);
 
-    var outParam = { value: null };
-    imgTools.decodeImageData(istream, dict["preImageMimeType"], outParam);
-    var container = outParam.value;
+    var container = imgTools.decodeImage(istream, dict["preImageMimeType"]);
 
     istream = imgTools.encodeImage(container, dict["refImageMimeType"]);
 
     var sstream = Cc["@mozilla.org/storagestream;1"].
 	          createInstance(Ci.nsIStorageStream);
     sstream.init(4096, 4294967295, null);
     var ostream = sstream.getOutputStream(0);
     var bostream = Cc["@mozilla.org/network/buffered-output-stream;1"].
@@ -696,21 +688,19 @@ imgName = "bug413512.ico";
 inMimeType = "image/x-icon";
 imgFile = do_get_file(imgName);
 
 istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 17759);
 var errsrc = "none";
 
 try {
-  outParam = { value: null };
-  imgTools.decodeImageData(istream, inMimeType, outParam);
-  container = outParam.value;
+  container = imgTools.decodeImage(istream, inMimeType);
 
-  // We should never hit this - decodeImageData throws an assertion because the
+  // We should never hit this - decodeImage throws an assertion because the
   // image decoded doesn't have enough frames.
   try {
       istream = imgTools.encodeImage(container, "image/png");
   } catch (e) {
       err = e;
       errsrc = "encode";
   }
 } catch (e) {
@@ -728,19 +718,17 @@ testdesc = "test correct ico hotspots (b
 
 imgName = "bug815359.ico";
 inMimeType = "image/x-icon";
 imgFile = do_get_file(imgName);
 
 istream = getFileInputStream(imgFile);
 do_check_eq(istream.available(), 4286);
 
-outParam = { value: null };
-imgTools.decodeImageData(istream, inMimeType, outParam);
-container = outParam.value;
+container = imgTools.decodeImage(istream, inMimeType);
 
 var props = container.QueryInterface(Ci.nsIProperties);
 
 do_check_eq(props.get("hotspotX", Ci.nsISupportsPRUint32).data, 10);
 do_check_eq(props.get("hotspotY", Ci.nsISupportsPRUint32).data, 9);
 
 
 /* ========== end ========== */