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 351966 4999ae42eaec6fce2ca8cc439ba660a2fdb9ec58
parent 351965 50ac73a27eb3206a849e8eb397c95ab75cddd4f9
child 351967 c31142f750f879fa3bc8f523285f3f19cd310bf6
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 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
 {