Bug 1287691 (Part 1) - Expose yielding to decoding tasks. r=edwin

image/Decoder.cpp

@@ -106,7 +106,7 @@ Decoder::Init()
   return rv;
 }
 
-nsresult
+LexerResult
 Decoder::Decode(IResumable* aOnResume /* = nullptr */)
 {
   MOZ_ASSERT(mInitialized, "Should be initialized here");
@@ -114,7 +114,8 @@ Decoder::Decode(IResumable* aOnResume /* = nullptr */)
 
   // 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);
@@ -126,11 +127,10 @@ Decoder::Decode(IResumable* aOnResume /* = nullptr */)
   };
 
   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.
@@ -145,7 +145,8 @@ Decoder::Decode(IResumable* aOnResume /* = nullptr */)
   // Perform final cleanup.
   CompleteDecode();
 
-  return HasError() ? NS_ERROR_FAILURE : NS_OK;
+  return LexerResult(HasError() ? TerminalState::FAILURE
+                                : TerminalState::SUCCESS);
 }
 
 bool

image/Decoder.h

@@ -48,9 +48,14 @@ public:
    * 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,

image/IDecodingTask.cpp

@@ -85,20 +85,31 @@ DecodingTask::DecodingTask(NotNull<Decoder*> aDecoder)
 void
 DecodingTask::Run()
 {
-  nsresult rv = mDecoder->Decode(WrapNotNull(this));
+  while (true) {
+    LexerResult result = mDecoder->Decode(WrapNotNull(this));
+
+    if (result.is<TerminalState>()) {
+      NotifyDecodeComplete(mDecoder);
+      return;  // We're done.
+    }
+
+    MOZ_ASSERT(result.is<Yield>());
 
-  if (NS_SUCCEEDED(rv) && !mDecoder->GetDecodeDone()) {
     // 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;
+    }
 
-  NotifyDecodeComplete(mDecoder);
+    // Right now we don't do anything special for other kinds of yields, so just
+    // keep working.
+  }
 }
 
 bool
@@ -122,17 +133,22 @@ MetadataDecodingTask::MetadataDecodingTask(NotNull<Decoder*> aDecoder)
 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");
 }
 
 
@@ -147,7 +163,23 @@ AnonymousDecodingTask::AnonymousDecodingTask(NotNull<Decoder*> 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

image/decoders/nsICODecoder.cpp

@@ -645,10 +645,14 @@ nsICODecoder::WriteToContainedDecoder(const char* aBuffer, uint32_t aCount)
   // 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();