Bug 1291045 (Part 7) - Replace DecodingTask with DecodedSurfaceProvider. r=dholbert,edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 05 Aug 2016 15:18:52 -0700
changeset 351970 b5418ecab2a98fa13e349d0be2405126634d2824
parent 351969 a6e140998c35958e3d05031afacd9e123356b2d3
child 351971 d416df8849a03fb005f6bd0ed9d844ed9dd4b6db
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, edwin
bugs1291045
milestone51.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 1291045 (Part 7) - Replace DecodingTask with DecodedSurfaceProvider. r=dholbert,edwin
image/Decoder.h
image/DecoderFactory.cpp
image/IDecodingTask.cpp
image/IDecodingTask.h
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -244,24 +244,16 @@ public:
    * This may happen due to a low-memory condition, or because another decoder
    * was racing with this one to decode the same frames with the same flags and
    * this decoder lost the race. Either way, this is not a permanent situation
    * and does not constitute an error, so we don't report any errors when this
    * happens.
    */
   bool WasAborted() const { return mDecodeAborted; }
 
-  /**
-   * Mark this decoder as aborted.
-   *
-   * XXX(seth): This is a temporary method which just exists to allow this patch
-   * series to be split into smaller patches. It'll be removed in a later patch.
-   */
-  void Abort() { mDecodeAborted = true; }
-
   enum DecodeStyle {
       PROGRESSIVE, // produce intermediate frames representing the partial
                    // state of the image
       SEQUENTIAL   // decode to final image immediately
   };
 
   /**
    * Get or set the DecoderFlags that influence the behavior of this decoder.
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -114,44 +114,52 @@ DecoderFactory::CreateDecoder(DecoderTyp
                               SurfaceFlags aSurfaceFlags,
                               int aSampleSize)
 {
   if (aType == DecoderType::UNKNOWN) {
     return nullptr;
   }
 
   // Create an anonymous decoder. Interaction with the SurfaceCache and the
-  // owning RasterImage will be mediated by DecodingTask.
+  // owning RasterImage will be mediated by DecodedSurfaceProvider.
   RefPtr<Decoder> decoder =
     GetDecoder(aType, nullptr, bool(aDecoderFlags & DecoderFlags::IS_REDECODE));
   MOZ_ASSERT(decoder, "Should have a decoder now");
 
   // Initialize the decoder.
   decoder->SetMetadataDecode(false);
   decoder->SetIterator(aSourceBuffer->Iterator());
   decoder->SetOutputSize(aOutputSize);
   decoder->SetDecoderFlags(aDecoderFlags | DecoderFlags::FIRST_FRAME_ONLY);
   decoder->SetSurfaceFlags(aSurfaceFlags);
   decoder->SetSampleSize(aSampleSize);
 
   if (NS_FAILED(decoder->Init())) {
     return nullptr;
   }
 
-  // Add a placeholder to the SurfaceCache so we won't trigger any more decoders
-  // with the same parameters.
+  // Create a DecodedSurfaceProvider which will manage the decoding process and
+  // make this decoder's output available in the surface cache.
   SurfaceKey surfaceKey =
     RasterSurfaceKey(aOutputSize, aSurfaceFlags, /* aFrameNum = */ 0);
+  NotNull<RefPtr<DecodedSurfaceProvider>> provider =
+    WrapNotNull(new DecodedSurfaceProvider(aImage,
+                                           WrapNotNull(decoder),
+                                           surfaceKey));
+
+  // Attempt to insert the surface provider into the surface cache right away so
+  // we won't trigger any more decoders with the same parameters.
   InsertOutcome outcome =
-    SurfaceCache::InsertPlaceholder(ImageKey(aImage.get()), surfaceKey);
+    SurfaceCache::Insert(provider, ImageKey(aImage.get()), surfaceKey);
   if (outcome != InsertOutcome::SUCCESS) {
     return nullptr;
   }
 
-  RefPtr<IDecodingTask> task = new DecodingTask(aImage, WrapNotNull(decoder));
+  // Return the surface provider in its IDecodingTask guise.
+  RefPtr<IDecodingTask> task = provider.get();
   return task.forget();
 }
 
 /* static */ already_AddRefed<IDecodingTask>
 DecoderFactory::CreateAnimationDecoder(DecoderType aType,
                                        NotNull<RasterImage*> aImage,
                                        NotNull<SourceBuffer*> aSourceBuffer,
                                        const IntSize& aIntrinsicSize,
--- a/image/IDecodingTask.cpp
+++ b/image/IDecodingTask.cpp
@@ -1,17 +1,16 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* 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 "IDecodingTask.h"
 
 #include "gfxPrefs.h"
-#include "nsProxyRelease.h"
 #include "nsThreadUtils.h"
 
 #include "Decoder.h"
 #include "DecodePool.h"
 #include "RasterImage.h"
 #include "SurfaceCache.h"
 
 namespace mozilla {
@@ -84,106 +83,16 @@ IDecodingTask::NotifyDecodeComplete(NotN
 void
 IDecodingTask::Resume()
 {
   DecodePool::Singleton()->AsyncRun(this);
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
-// DecodingTask implementation.
-///////////////////////////////////////////////////////////////////////////////
-
-DecodingTask::DecodingTask(NotNull<RasterImage*> aImage,
-                           NotNull<Decoder*> aDecoder)
-  : mImage(aImage)
-  , mDecoder(aDecoder)
-{
-  MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
-             "Use MetadataDecodingTask for metadata decodes");
-  MOZ_ASSERT(mDecoder->IsFirstFrameDecode(),
-             "Use AnimationDecodingTask for animation decodes");
-}
-
-DecodingTask::~DecodingTask()
-{
-  // RasterImage objects need to be destroyed on the main thread.
-  RefPtr<RasterImage> image = mImage;
-  NS_ReleaseOnMainThread(image.forget());
-}
-
-void
-DecodingTask::Run()
-{
-  LexerResult result = mDecoder->Decode(WrapNotNull(this));
-
-  // If the decoder hadn't produced a surface up to this point, see if a surface
-  // is now available.
-  if (!mSurface) {
-    mSurface = mDecoder->GetCurrentFrameRef().get();
-
-    if (mSurface) {
-      // There's a new surface available; insert it into the SurfaceCache.
-      NotNull<RefPtr<ISurfaceProvider>> provider =
-        WrapNotNull(new SimpleSurfaceProvider(WrapNotNull(mSurface.get())));
-      InsertOutcome outcome =
-        SurfaceCache::Insert(provider, ImageKey(mImage.get()),
-                             RasterSurfaceKey(mDecoder->OutputSize(),
-                                              mDecoder->GetSurfaceFlags(),
-                                              /* aFrameNum = */ 0));
-
-      if (outcome == InsertOutcome::FAILURE) {
-        // We couldn't insert the surface, almost certainly due to low memory. We
-        // treat this as a permanent error to help the system recover; otherwise,
-        // we might just end up attempting to decode this image again immediately.
-        result = mDecoder->TerminateFailure();
-      } else if (outcome == InsertOutcome::FAILURE_ALREADY_PRESENT) {
-        // Another decoder beat us to decoding this frame. We abort this decoder
-        // rather than treat this as a real error.
-        mDecoder->Abort();
-        result = mDecoder->TerminateFailure();
-      }
-    }
-  }
-
-  MOZ_ASSERT(mSurface.get() == mDecoder->GetCurrentFrameRef().get(),
-             "DecodingTask and Decoder have different surfaces?");
-
-  if (result.is<TerminalState>()) {
-    NotifyDecodeComplete(mImage, mDecoder);
-    return;  // We're done.
-  }
-
-  MOZ_ASSERT(result.is<Yield>());
-
-  // Notify for the progress we've made so far.
-  if (mDecoder->HasProgress()) {
-    NotifyProgress(mImage, mDecoder);
-  }
-
-  if (result == LexerResult(Yield::NEED_MORE_DATA)) {
-    // We can't make any more progress right now. The decoder itself will ensure
-    // that we get reenqueued when more data is available; just return for now.
-    return;
-  }
-
-  // Other kinds of yields shouldn't happen during single-frame image decodes.
-  MOZ_ASSERT_UNREACHABLE("Unexpected yield during single-frame image decode");
-  mDecoder->TerminateFailure();
-  NotifyDecodeComplete(mImage, mDecoder);
-}
-
-bool
-DecodingTask::ShouldPreferSyncRun() const
-{
-  return mDecoder->ShouldSyncDecode(gfxPrefs::ImageMemDecodeBytesAtATime());
-}
-
-
-///////////////////////////////////////////////////////////////////////////////
 // AnimationDecodingTask implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
 AnimationDecodingTask::AnimationDecodingTask(NotNull<Decoder*> aDecoder)
   : mDecoder(aDecoder)
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
--- a/image/IDecodingTask.h
+++ b/image/IDecodingTask.h
@@ -58,43 +58,16 @@ protected:
   static void NotifyDecodeComplete(NotNull<RasterImage*> aImage,
                                    NotNull<Decoder*> aDecoder);
 
   virtual ~IDecodingTask() { }
 };
 
 
 /**
- * An IDecodingTask implementation for full decodes of single frame images.
- */
-class DecodingTask final : public IDecodingTask
-{
-public:
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodingTask, override)
-
-  DecodingTask(NotNull<RasterImage*> aImage,
-               NotNull<Decoder*> aDecoder);
-
-  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; }
-
-private:
-  virtual ~DecodingTask();
-
-  NotNull<RefPtr<RasterImage>> mImage;
-  NotNull<RefPtr<Decoder>> mDecoder;
-  RefPtr<imgFrame> mSurface;
-};
-
-
-/**
  * An IDecodingTask implementation for full decodes of animated images.
  */
 class AnimationDecodingTask final : public IDecodingTask
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AnimationDecodingTask, override)
 
   explicit AnimationDecodingTask(NotNull<Decoder*> aDecoder);