Bug 1291045 (Part 3) - Handle interactions with the SurfaceCache in DecodingTask. r=dholbert,edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Thu, 04 Aug 2016 18:59:10 -0700
changeset 397555 4999ae42eaec6fce2ca8cc439ba660a2fdb9ec58
parent 397554 50ac73a27eb3206a849e8eb397c95ab75cddd4f9
child 397556 c31142f750f879fa3bc8f523285f3f19cd310bf6
push id25332
push usermaglione.k@gmail.com
push dateSat, 06 Aug 2016 21:21:51 +0000
reviewersdholbert, edwin
bugs1291045
milestone51.0a1
Bug 1291045 (Part 3) - Handle interactions with the SurfaceCache in DecodingTask. r=dholbert,edwin
image/Decoder.cpp
image/Decoder.h
image/DecoderFactory.cpp
image/IDecodingTask.cpp
image/IDecodingTask.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -149,16 +149,30 @@ Decoder::Decode(IResumable* aOnResume /*
 
   // Perform final cleanup.
   CompleteDecode();
 
   return LexerResult(HasError() ? TerminalState::FAILURE
                                 : TerminalState::SUCCESS);
 }
 
+LexerResult
+Decoder::TerminateFailure()
+{
+  PostError();
+
+  // Perform final cleanup if need be.
+  if (!mReachedTerminalState) {
+    mReachedTerminalState = true;
+    CompleteDecode();
+  }
+
+  return LexerResult(TerminalState::FAILURE);
+}
+
 bool
 Decoder::ShouldSyncDecode(size_t aByteLimit)
 {
   MOZ_ASSERT(aByteLimit > 0);
   MOZ_ASSERT(mIterator, "Should have a SourceBufferIterator");
 
   return mIterator->RemainingBytesIsNoMoreThan(aByteLimit);
 }
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -53,16 +53,27 @@ public:
    *   - the image has failed to decode (TerminalState::FAILURE), or
    *   - the decoder is yielding until it gets more data (Yield::NEED_MORE_DATA), or
    *   - the decoder is yielding to allow the caller to access intermediate
    *     output (Yield::OUTPUT_AVAILABLE).
    */
   LexerResult Decode(IResumable* aOnResume = nullptr);
 
   /**
+   * Terminate this decoder in a failure state, just as if the decoder
+   * implementation had returned TerminalState::FAILURE from DoDecode().
+   *
+   * XXX(seth): This method should be removed ASAP; it exists only because
+   * RasterImage::FinalizeDecoder() requires an actual Decoder object as an
+   * argument, so we can't simply tell RasterImage a decode failed except via an
+   * intervening decoder. We'll fix this in bug 1291071.
+   */
+  LexerResult TerminateFailure();
+
+  /**
    * Given a maximum number of bytes we're willing to decode, @aByteLimit,
    * returns true if we should attempt to run this decoder synchronously.
    */
   bool ShouldSyncDecode(size_t aByteLimit);
 
   /**
    * Gets the invalidation region accumulated by the decoder so far, and clears
    * the decoder's invalidation region. This means that each call to
@@ -233,16 +244,24 @@ 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
@@ -113,18 +113,20 @@ DecoderFactory::CreateDecoder(DecoderTyp
                               DecoderFlags aDecoderFlags,
                               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.
   RefPtr<Decoder> decoder =
-    GetDecoder(aType, aImage, bool(aDecoderFlags & DecoderFlags::IS_REDECODE));
+    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);
@@ -139,17 +141,17 @@ DecoderFactory::CreateDecoder(DecoderTyp
   SurfaceKey surfaceKey =
     RasterSurfaceKey(aOutputSize, aSurfaceFlags, /* aFrameNum = */ 0);
   InsertOutcome outcome =
     SurfaceCache::InsertPlaceholder(ImageKey(aImage.get()), surfaceKey);
   if (outcome != InsertOutcome::SUCCESS) {
     return nullptr;
   }
 
-  RefPtr<IDecodingTask> task = new DecodingTask(WrapNotNull(decoder));
+  RefPtr<IDecodingTask> task = new DecodingTask(aImage, WrapNotNull(decoder));
   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,21 +1,23 @@
 /* -*- 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 {
 
 using gfx::IntRect;
 
 namespace image {
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -83,53 +85,93 @@ IDecodingTask::Resume()
   DecodePool::Singleton()->AsyncRun(this);
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // DecodingTask implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
-DecodingTask::DecodingTask(NotNull<Decoder*> aDecoder)
-  : mDecoder(aDecoder)
+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()
 {
-  while (true) {
-    LexerResult result = mDecoder->Decode(WrapNotNull(this));
+  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 (result.is<TerminalState>()) {
-      NotifyDecodeComplete(mDecoder->GetImage(), mDecoder);
-      return;  // We're done.
-    }
-
-    MOZ_ASSERT(result.is<Yield>());
+    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));
 
-    // Notify for the progress we've made so far.
-    if (mDecoder->HasProgress()) {
-      NotifyProgress(mDecoder->GetImage(), mDecoder);
+      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 == 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;
-    }
+  if (result.is<TerminalState>()) {
+    NotifyDecodeComplete(mImage, mDecoder);
+    return;  // We're done.
+  }
+
+  MOZ_ASSERT(result.is<Yield>());
 
-    // Right now we don't do anything special for other kinds of yields, so just
-    // keep working.
+  // 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());
 }
 
--- a/image/IDecodingTask.h
+++ b/image/IDecodingTask.h
@@ -9,22 +9,24 @@
  */
 
 #ifndef mozilla_image_IDecodingTask_h
 #define mozilla_image_IDecodingTask_h
 
 #include "mozilla/NotNull.h"
 #include "mozilla/RefPtr.h"
 
+#include "imgFrame.h"
 #include "SourceBuffer.h"
 
 namespace mozilla {
 namespace image {
 
 class Decoder;
+class RasterImage;
 
 /// A priority hint that DecodePool can use when scheduling an IDecodingTask.
 enum class TaskPriority : uint8_t
 {
   eLow,
   eHigh
 };
 
@@ -58,31 +60,34 @@ protected:
 /**
  * An IDecodingTask implementation for full decodes of single frame images.
  */
 class DecodingTask final : public IDecodingTask
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodingTask, override)
 
-  explicit DecodingTask(NotNull<Decoder*> aDecoder);
+  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; }
 
   NotNull<Decoder*> GetDecoder() const override { return mDecoder; }
 
 private:
-  virtual ~DecodingTask() { }
+  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
 {