Bug 1282275 - Return IDecodingTask objects instead of Decoder objects from most DecoderFactory functions. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sun, 26 Jun 2016 00:09:24 -0700
changeset 345314 626bc70dd8f972f8498d732f0f4c8c4343dc41d3
parent 345313 335dc3e1cb1fe6a09fd759062a6556a55aae43a3
child 345315 6ba925dbd2334e4071d374a93e4aa8ca532d404f
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1282275
milestone50.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 1282275 - Return IDecodingTask objects instead of Decoder objects from most DecoderFactory functions. r=dholbert
image/DecoderFactory.cpp
image/DecoderFactory.h
image/IDecodingTask.h
image/RasterImage.cpp
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -5,16 +5,17 @@
 
 #include "DecoderFactory.h"
 
 #include "nsMimeTypes.h"
 #include "mozilla/RefPtr.h"
 #include "nsString.h"
 
 #include "Decoder.h"
+#include "IDecodingTask.h"
 #include "nsPNGDecoder.h"
 #include "nsGIFDecoder2.h"
 #include "nsJPEGDecoder.h"
 #include "nsBMPDecoder.h"
 #include "nsICODecoder.h"
 #include "nsIconDecoder.h"
 
 namespace mozilla {
@@ -99,17 +100,17 @@ DecoderFactory::GetDecoder(DecoderType a
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unknown decoder type");
   }
 
   return decoder.forget();
 }
 
-/* static */ already_AddRefed<Decoder>
+/* static */ already_AddRefed<IDecodingTask>
 DecoderFactory::CreateDecoder(DecoderType aType,
                               RasterImage* aImage,
                               SourceBuffer* aSourceBuffer,
                               const Maybe<IntSize>& aTargetSize,
                               DecoderFlags aDecoderFlags,
                               SurfaceFlags aSurfaceFlags,
                               int aSampleSize)
 {
@@ -134,20 +135,21 @@ DecoderFactory::CreateDecoder(DecoderTyp
     MOZ_ASSERT(NS_SUCCEEDED(rv), "Bad downscale-during-decode target size?");
   }
 
   decoder->Init();
   if (NS_FAILED(decoder->GetDecoderError())) {
     return nullptr;
   }
 
-  return decoder.forget();
+  RefPtr<IDecodingTask> task = new DecodingTask(WrapNotNull(decoder));
+  return task.forget();
 }
 
-/* static */ already_AddRefed<Decoder>
+/* static */ already_AddRefed<IDecodingTask>
 DecoderFactory::CreateAnimationDecoder(DecoderType aType,
                                        RasterImage* aImage,
                                        SourceBuffer* aSourceBuffer,
                                        DecoderFlags aDecoderFlags,
                                        SurfaceFlags aSurfaceFlags)
 {
   if (aType == DecoderType::UNKNOWN) {
     return nullptr;
@@ -166,20 +168,21 @@ DecoderFactory::CreateAnimationDecoder(D
   decoder->SetDecoderFlags(aDecoderFlags | DecoderFlags::IS_REDECODE);
   decoder->SetSurfaceFlags(aSurfaceFlags);
 
   decoder->Init();
   if (NS_FAILED(decoder->GetDecoderError())) {
     return nullptr;
   }
 
-  return decoder.forget();
+  RefPtr<IDecodingTask> task = new DecodingTask(WrapNotNull(decoder));
+  return task.forget();
 }
 
-/* static */ already_AddRefed<Decoder>
+/* static */ already_AddRefed<IDecodingTask>
 DecoderFactory::CreateMetadataDecoder(DecoderType aType,
                                       RasterImage* aImage,
                                       SourceBuffer* aSourceBuffer,
                                       int aSampleSize)
 {
   if (aType == DecoderType::UNKNOWN) {
     return nullptr;
   }
@@ -193,17 +196,18 @@ DecoderFactory::CreateMetadataDecoder(De
   decoder->SetIterator(aSourceBuffer->Iterator());
   decoder->SetSampleSize(aSampleSize);
 
   decoder->Init();
   if (NS_FAILED(decoder->GetDecoderError())) {
     return nullptr;
   }
 
-  return decoder.forget();
+  RefPtr<IDecodingTask> task = new MetadataDecodingTask(WrapNotNull(decoder));
+  return task.forget();
 }
 
 /* static */ already_AddRefed<Decoder>
 DecoderFactory::CreateAnonymousDecoder(DecoderType aType,
                                        SourceBuffer* aSourceBuffer,
                                        const Maybe<IntSize>& aTargetSize,
                                        SurfaceFlags aSurfaceFlags)
 {
--- a/image/DecoderFactory.h
+++ b/image/DecoderFactory.h
@@ -15,16 +15,17 @@
 #include "SurfaceFlags.h"
 
 class nsACString;
 
 namespace mozilla {
 namespace image {
 
 class Decoder;
+class IDecodingTask;
 class RasterImage;
 class SourceBuffer;
 
 /**
  * The type of decoder; this is usually determined from a MIME type using
  * DecoderFactory::GetDecoderType().
  */
 enum class DecoderType
@@ -59,17 +60,17 @@ public:
    *                    a target size for a decoder type which doesn't support
    *                    downscale-during-decode.
    * @param aDecoderFlags Flags specifying the behavior of this decoder.
    * @param aSurfaceFlags Flags specifying the type of output this decoder
    *                      should produce.
    * @param aSampleSize The sample size requested using #-moz-samplesize (or 0
    *                    if none).
    */
-  static already_AddRefed<Decoder>
+  static already_AddRefed<IDecodingTask>
   CreateDecoder(DecoderType aType,
                 RasterImage* aImage,
                 SourceBuffer* aSourceBuffer,
                 const Maybe<gfx::IntSize>& aTargetSize,
                 DecoderFlags aDecoderFlags,
                 SurfaceFlags aSurfaceFlags,
                 int aSampleSize);
 
@@ -81,17 +82,17 @@ public:
    * @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 aDecoderFlags Flags specifying the behavior of this decoder.
    * @param aSurfaceFlags Flags specifying the type of output this decoder
    *                      should produce.
    */
-  static already_AddRefed<Decoder>
+  static already_AddRefed<IDecodingTask>
   CreateAnimationDecoder(DecoderType aType,
                          RasterImage* aImage,
                          SourceBuffer* aSourceBuffer,
                          DecoderFlags aDecoderFlags,
                          SurfaceFlags aSurfaceFlags);
 
   /**
    * Creates and initializes a metadata decoder of type @aType. This decoder
@@ -102,17 +103,17 @@ public:
    * @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).
    */
-  static already_AddRefed<Decoder>
+  static already_AddRefed<IDecodingTask>
   CreateMetadataDecoder(DecoderType aType,
                         RasterImage* aImage,
                         SourceBuffer* aSourceBuffer,
                         int aSampleSize);
 
   /**
    * Creates and initializes an anonymous decoder (one which isn't associated
    * with an Image object). Only the first frame of the image will be decoded.
--- a/image/IDecodingTask.h
+++ b/image/IDecodingTask.h
@@ -42,16 +42,19 @@ public:
 
   /// @return a priority hint that DecodePool can use when scheduling this task.
   virtual TaskPriority Priority() const = 0;
 
   /// A default implementation of IResumable which resubmits the task to the
   /// DecodePool. Subclasses can override this if they need different behavior.
   void Resume() override;
 
+  /// @return a non-null weak pointer to the Decoder associated with this task.
+  virtual NotNull<Decoder*> GetDecoder() const = 0;
+
   // Notify the Image associated with a Decoder of its progress, sending a
   // runnable to the main thread if necessary.
   // XXX(seth): This is a hack that will be removed soon.
   static void NotifyProgress(NotNull<Decoder*> aDecoder);
 
 protected:
   virtual ~IDecodingTask() { }
 };
@@ -69,16 +72,18 @@ public:
 
   void Run() override;
   bool ShouldPreferSyncRun() const override;
 
   // Full decodes are low priority compared to metadata decodes because they
   // don't block layout or page load.
   TaskPriority Priority() const override { return TaskPriority::eLow; }
 
+  NotNull<Decoder*> GetDecoder() const override { return mDecoder; }
+
 private:
   virtual ~DecodingTask() { }
 
   NotNull<RefPtr<Decoder>> mDecoder;
 };
 
 
 /**
@@ -97,16 +102,18 @@ public:
   // header) so there's no reason to refuse to run them synchronously if the
   // caller will allow us to.
   bool ShouldPreferSyncRun() const override { return true; }
 
   // Metadata decodes run at the highest priority because they block layout and
   // page load.
   TaskPriority Priority() const override { return TaskPriority::eHigh; }
 
+  NotNull<Decoder*> GetDecoder() const override { return mDecoder; }
+
 private:
   virtual ~MetadataDecodingTask() { }
 
   NotNull<RefPtr<Decoder>> mDecoder;
 };
 
 
 /**
@@ -125,16 +132,18 @@ public:
   bool ShouldPreferSyncRun() const override { return true; }
   TaskPriority Priority() const override { return TaskPriority::eLow; }
 
   // Anonymous decoders normally get all their data at once. We have tests where
   // they don't; in these situations, the test re-runs them manually. So no
   // matter what, we don't want to resume by posting a task to the DecodePool.
   void Resume() override { }
 
+  NotNull<Decoder*> GetDecoder() const override { return mDecoder; }
+
 private:
   virtual ~AnonymousDecodingTask() { }
 
   NotNull<RefPtr<Decoder>> mDecoder;
 };
 
 } // namespace image
 } // namespace mozilla
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1301,73 +1301,72 @@ RasterImage::Decode(const IntSize& aSize
   SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags);
   if (IsOpaque()) {
     // If there's no transparency, it doesn't matter whether we premultiply
     // alpha or not.
     surfaceFlags &= ~SurfaceFlags::NO_PREMULTIPLY_ALPHA;
   }
 
   // Create a decoder.
-  RefPtr<Decoder> decoder;
+  RefPtr<IDecodingTask> task;
   if (mAnim) {
-    decoder = DecoderFactory::CreateAnimationDecoder(mDecoderType, this,
-                                                     mSourceBuffer, decoderFlags,
-                                                     surfaceFlags);
+    task = DecoderFactory::CreateAnimationDecoder(mDecoderType, this,
+                                                  mSourceBuffer, decoderFlags,
+                                                  surfaceFlags);
   } else {
-    decoder = DecoderFactory::CreateDecoder(mDecoderType, this, mSourceBuffer,
-                                            targetSize, decoderFlags,
-                                            surfaceFlags,
-                                            mRequestedSampleSize);
+    task = DecoderFactory::CreateDecoder(mDecoderType, this, mSourceBuffer,
+                                         targetSize, decoderFlags,
+                                         surfaceFlags,
+                                         mRequestedSampleSize);
   }
 
   // Make sure DecoderFactory was able to create a decoder successfully.
-  if (!decoder) {
+  if (!task) {
     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.
+  SurfaceKey surfaceKey =
+    RasterSurfaceKey(aSize,
+                     task->GetDecoder()->GetSurfaceFlags(),
+                     /* aFrameNum = */ 0);
   InsertOutcome outcome =
-    SurfaceCache::InsertPlaceholder(ImageKey(this),
-                                    RasterSurfaceKey(aSize,
-                                                     decoder->GetSurfaceFlags(),
-                                                     /* aFrameNum = */ 0));
+    SurfaceCache::InsertPlaceholder(ImageKey(this), surfaceKey);
   if (outcome != InsertOutcome::SUCCESS) {
     return NS_ERROR_FAILURE;
   }
 
   mDecodeCount++;
 
   // We're ready to decode; start the decoder.
-  RefPtr<IDecodingTask> task = new DecodingTask(WrapNotNull(decoder));
   LaunchDecodingTask(task, this, aFlags, mHasSourceData);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 RasterImage::DecodeMetadata(uint32_t aFlags)
 {
   if (mError) {
     return NS_ERROR_FAILURE;
   }
 
   MOZ_ASSERT(!mHasSize, "Should not do unnecessary metadata decodes");
 
   // Create a decoder.
-  RefPtr<Decoder> decoder =
+  RefPtr<IDecodingTask> task =
     DecoderFactory::CreateMetadataDecoder(mDecoderType, this, mSourceBuffer,
                                           mRequestedSampleSize);
 
   // Make sure DecoderFactory was able to create a decoder successfully.
-  if (!decoder) {
+  if (!task) {
     return NS_ERROR_FAILURE;
   }
 
   // We're ready to decode; start the decoder.
-  RefPtr<IDecodingTask> task = new MetadataDecodingTask(WrapNotNull(decoder));
   LaunchDecodingTask(task, this, aFlags, mHasSourceData);
   return NS_OK;
 }
 
 void
 RasterImage::RecoverFromInvalidFrames(const IntSize& aSize, uint32_t aFlags)
 {
   if (!mHasSize) {