Bug 1287691 (Part 1) - Expose yielding to decoding tasks. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 18 Jul 2016 23:46:35 -0700
changeset 305622 5a7159dd41f1911eb634a97743dfb1c547bdf90b
parent 305621 59d21113b4619c117c178b86ec8c7fd5d7fb5807
child 305623 f0ec31db260e67c13526a33414cc8b860600b214
push id79664
push usermfowler@mozilla.com
push dateWed, 20 Jul 2016 09:30:47 +0000
treeherdermozilla-inbound@f518461663cd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1287691
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 1287691 (Part 1) - Expose yielding to decoding tasks. r=edwin
image/Decoder.cpp
image/Decoder.h
image/IDecodingTask.cpp
image/decoders/nsICODecoder.cpp
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -1,9 +1,8 @@
-
 /* -*- 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 "Decoder.h"
 
 #include "mozilla/gfx/2D.h"
@@ -101,56 +100,57 @@ Decoder::Init()
   // Implementation-specific initialization.
   nsresult rv = InitInternal();
 
   mInitialized = true;
 
   return rv;
 }
 
-nsresult
+LexerResult
 Decoder::Decode(IResumable* aOnResume /* = nullptr */)
 {
   MOZ_ASSERT(mInitialized, "Should be initialized here");
   MOZ_ASSERT(mIterator, "Should have a SourceBufferIterator");
 
   // If we're already done, don't attempt to keep decoding.
   if (GetDecodeDone()) {
-    return HasError() ? NS_ERROR_FAILURE : NS_OK;
+    return LexerResult(HasError() ? TerminalState::FAILURE
+                                  : TerminalState::SUCCESS);
   }
 
   LexerResult lexerResult(TerminalState::FAILURE);
   {
     PROFILER_LABEL("ImageDecoder", "Decode", js::ProfileEntry::Category::GRAPHICS);
     AutoRecordDecoderTelemetry telemetry(this);
 
     lexerResult =  DoDecode(*mIterator, aOnResume);
   };
 
   if (lexerResult.is<Yield>()) {
-    // We need more data to continue. If @aOnResume was non-null, the
-    // SourceBufferIterator will automatically reschedule us. Otherwise, it's up
-    // to the caller.
-    MOZ_ASSERT(lexerResult.as<Yield>() == Yield::NEED_MORE_DATA);
-    return NS_OK;
+    // We either need more data to continue (in which case either @aOnResume or
+    // the caller will reschedule us to run again later), or the decoder is
+    // yielding to allow the caller access to some intermediate output.
+    return lexerResult;
   }
 
   // We reached a terminal state; we're now done decoding.
   MOZ_ASSERT(lexerResult.is<TerminalState>());
   mReachedTerminalState = true;
 
   // If decoding failed, record that fact.
   if (lexerResult.as<TerminalState>() == TerminalState::FAILURE) {
     PostError();
   }
 
   // Perform final cleanup.
   CompleteDecode();
 
-  return HasError() ? NS_ERROR_FAILURE : NS_OK;
+  return LexerResult(HasError() ? TerminalState::FAILURE
+                                : TerminalState::SUCCESS);
 }
 
 bool
 Decoder::ShouldSyncDecode(size_t aByteLimit)
 {
   MOZ_ASSERT(aByteLimit > 0);
   MOZ_ASSERT(mIterator, "Should have a SourceBufferIterator");
 
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -43,19 +43,24 @@ public:
   nsresult Init();
 
   /**
    * Decodes, reading all data currently available in the SourceBuffer.
    *
    * If more data is needed and @aOnResume is non-null, Decode() will schedule
    * @aOnResume to be called when more data is available.
    *
-   * Any errors are reported by setting the appropriate state on the decoder.
+   * @return a LexerResult which may indicate:
+   *   - the image has been successfully decoded (TerminalState::SUCCESS), or
+   *   - 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).
    */
-  nsresult Decode(IResumable* aOnResume = nullptr);
+  LexerResult Decode(IResumable* aOnResume = nullptr);
 
   /**
    * 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);
 
   /**
--- a/image/IDecodingTask.cpp
+++ b/image/IDecodingTask.cpp
@@ -80,30 +80,41 @@ DecodingTask::DecodingTask(NotNull<Decod
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
 }
 
 void
 DecodingTask::Run()
 {
-  nsresult rv = mDecoder->Decode(WrapNotNull(this));
+  while (true) {
+    LexerResult result = mDecoder->Decode(WrapNotNull(this));
 
-  if (NS_SUCCEEDED(rv) && !mDecoder->GetDecodeDone()) {
+    if (result.is<TerminalState>()) {
+      NotifyDecodeComplete(mDecoder);
+      return;  // We're done.
+    }
+
+    MOZ_ASSERT(result.is<Yield>());
+
     // Notify for the progress we've made so far.
     if (mDecoder->HasProgress()) {
       NotifyProgress(mDecoder);
     }
 
-    // We don't need to do anything else for this case. The decoder itself will
-    // ensure that we get reenqueued when more data is available.
-    return;
+    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;
+    }
+
+    // Right now we don't do anything special for other kinds of yields, so just
+    // keep working.
   }
-
-  NotifyDecodeComplete(mDecoder);
 }
 
 bool
 DecodingTask::ShouldPreferSyncRun() const
 {
   return mDecoder->ShouldSyncDecode(gfxPrefs::ImageMemDecodeBytesAtATime());
 }
 
@@ -117,38 +128,59 @@ MetadataDecodingTask::MetadataDecodingTa
 {
   MOZ_ASSERT(mDecoder->IsMetadataDecode(),
              "Use DecodingTask for non-metadata decodes");
 }
 
 void
 MetadataDecodingTask::Run()
 {
-  nsresult rv = mDecoder->Decode(WrapNotNull(this));
+  LexerResult result = mDecoder->Decode(WrapNotNull(this));
 
-  if (NS_SUCCEEDED(rv) && !mDecoder->GetDecodeDone()) {
-    // It's important that metadata decode results are delivered atomically, so
-    // we'll wait until NotifyDecodeComplete() to report any progress.  We don't
-    // need to do anything else for this case. The decoder itself will ensure
-    // that we get reenqueued when more data is available.
+  if (result.is<TerminalState>()) {
+    NotifyDecodeComplete(mDecoder);
+    return;  // We're done.
+  }
+
+  if (result == LexerResult(Yield::NEED_MORE_DATA)) {
+    // We can't make any more progress right now. We also don't want to report
+    // any progress, because it's important that metadata decode results are
+    // delivered atomically. The decoder itself will ensure that we get
+    // reenqueued when more data is available; just return for now.
     return;
   }
 
-  NotifyDecodeComplete(mDecoder);
+  MOZ_ASSERT_UNREACHABLE("Metadata decode yielded for an unexpected reason");
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // AnonymousDecodingTask implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
 AnonymousDecodingTask::AnonymousDecodingTask(NotNull<Decoder*> aDecoder)
   : mDecoder(aDecoder)
 { }
 
 void
 AnonymousDecodingTask::Run()
 {
-  mDecoder->Decode(WrapNotNull(this));
+  while (true) {
+    LexerResult result = mDecoder->Decode(WrapNotNull(this));
+
+    if (result.is<TerminalState>()) {
+      return;  // We're done.
+    }
+
+    if (result == LexerResult(Yield::NEED_MORE_DATA)) {
+      // We can't make any more progress right now. Let the caller decide how to
+      // handle it.
+      return;
+    }
+
+    // Right now we don't do anything special for other kinds of yields, so just
+    // keep working.
+    MOZ_ASSERT(result.is<Yield>());
+  }
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -640,20 +640,24 @@ nsICODecoder::WriteToContainedDecoder(co
   mContainedSourceBuffer->Append(aBuffer, aCount);
 
   bool succeeded = true;
 
   // Write to the contained decoder. If we run out of data, the ICO decoder will
   // get resumed when there's more data available, as usual, so we don't need
   // the contained decoder to get resumed too. To avoid that, we provide an
   // IResumable which just does nothing.
-  if (NS_FAILED(mContainedDecoder->Decode())) {
+  LexerResult result = mContainedDecoder->Decode();
+  if (result == LexerResult(TerminalState::FAILURE)) {
     succeeded = false;
   }
 
+  MOZ_ASSERT(result != LexerResult(Yield::OUTPUT_AVAILABLE),
+             "Unexpected yield");
+
   // Make our state the same as the state of the contained decoder, and
   // propagate errors.
   mProgress |= mContainedDecoder->TakeProgress();
   mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
   if (mContainedDecoder->HasError()) {
     succeeded = false;
   }