Bug 1286165 (Part 2) - Advance decoders' SourceBufferIterator directly in StreamingLexer. r=edwin,njn

image/Decoder.cpp

@@ -107,7 +107,7 @@ Decoder::Init()
 }
 
 nsresult
-Decoder::Decode(NotNull<IResumable*> aOnResume)
+Decoder::Decode(IResumable* aOnResume /* = nullptr */)
 {
   MOZ_ASSERT(mInitialized, "Should be initialized here");
   MOZ_ASSERT(mIterator, "Should have a SourceBufferIterator");
@@ -118,54 +118,21 @@ Decoder::Decode(NotNull<IResumable*> aOnResume)
   }
 
   Maybe<TerminalState> terminalState;
-
-  // We keep decoding chunks until the decode completes (i.e., we reach a
-  // terminal state) or there are no more chunks available.
   {
-    PROFILER_LABEL("ImageDecoder", "Decode",
-                   js::ProfileEntry::Category::GRAPHICS);
+    PROFILER_LABEL("ImageDecoder", "Decode", js::ProfileEntry::Category::GRAPHICS);
     AutoRecordDecoderTelemetry telemetry(this);
 
-    do {
-      if (GetDecodeDone()) {
-        MOZ_ASSERT_UNREACHABLE("Finished decode without reaching terminal state?");
-        terminalState = Some(TerminalState::SUCCESS);
-        break;
-      }
-
-      switch (mIterator->AdvanceOrScheduleResume(aOnResume.get())) {
-        case SourceBufferIterator::WAITING:
-          // We can't continue because the rest of the data hasn't arrived from
-          // the network yet. We don't have to do anything special; the
-          // SourceBufferIterator will ensure that Decode() gets called again on a
-          // DecodePool thread when more data is available.
-          return NS_OK;
-
-        case SourceBufferIterator::COMPLETE:
-          // Normally even if the data is truncated, we want decoding to
-          // succeed so we can display whatever we got. However, if the
-          // SourceBuffer was completed with a failing status, we want to fail.
-          // This happens only in exceptional situations like SourceBuffer
-          // itself encountering a failure due to OOM.
-          terminalState = NS_SUCCEEDED(mIterator->CompletionStatus())
-                        ? Some(TerminalState::SUCCESS)
-                        : Some(TerminalState::FAILURE);
-
-          break;
-
-        case SourceBufferIterator::READY:
-          // Pass the data along to the implementation.
-          terminalState = DoDecode(*mIterator);
-          break;
-
-        default:
-          MOZ_ASSERT_UNREACHABLE("Unknown SourceBufferIterator state");
-          terminalState = Some(TerminalState::FAILURE);
-      }
-    } while (!terminalState);
+    terminalState = DoDecode(*mIterator, aOnResume);
   }
 
-  MOZ_ASSERT(terminalState);
+  if (!terminalState) {
+    // We need more data to continue. If @aOnResume was non-null, the
+    // SourceBufferIterator will automatically reschedule us. Otherwise, it's up
+    // to the caller.
+    return NS_OK;
+  }
+
+  // We reached a terminal state; we're now done decoding.
   mReachedTerminalState = true;
 
   // If decoding failed, record that fact.
@@ -173,7 +140,7 @@ Decoder::Decode(NotNull<IResumable*> aOnResume)
     PostError();
   }
 
-  // We're done decoding; perform final cleanup.
+  // Perform final cleanup.
   CompleteDecode();
 
   return HasError() ? NS_ERROR_FAILURE : NS_OK;

image/Decoder.h

@@ -45,12 +45,12 @@ public:
   /**
    * Decodes, reading all data currently available in the SourceBuffer.
    *
-   * If more data is needed, Decode() will schedule @aOnResume to be called when
-   * more data is available.
+   * 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.
    */
-  nsresult Decode(NotNull<IResumable*> aOnResume);
+  nsresult Decode(IResumable* aOnResume = nullptr);
 
   /**
    * Given a maximum number of bytes we're willing to decode, @aByteLimit,
@@ -297,7 +297,8 @@ protected:
    * return a failing nsresult.
    */
   virtual nsresult InitInternal();
-  virtual Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) = 0;
+  virtual Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                        IResumable* aOnResume) = 0;
   virtual nsresult BeforeFinishInternal();
   virtual nsresult FinishInternal();
   virtual nsresult FinishWithErrorInternal();

image/StreamingLexer.h

@@ -262,9 +262,61 @@ public:
     , mToReadUnbuffered(0)
   { }
 
+  template <typename Func>
+  Maybe<TerminalState> Lex(SourceBufferIterator& aIterator,
+                           IResumable* aOnResume,
+                           Func aFunc)
+  {
+    if (mTransition.NextStateIsTerminal()) {
+      // We've already reached a terminal state. We never deliver any more data
+      // in this case; just return the terminal state again immediately.
+      return Some(mTransition.NextStateAsTerminal());
+    }
+
+    do {
+      switch (aIterator.AdvanceOrScheduleResume(aOnResume)) {
+        case SourceBufferIterator::WAITING:
+          // We can't continue because the rest of the data hasn't arrived from
+          // the network yet. We don't have to do anything special; the
+          // SourceBufferIterator will ensure that |aOnResume| gets called when
+          // more data is available.
+          return Nothing();
+
+        case SourceBufferIterator::COMPLETE:
+          // Normally even if the data is truncated, we want decoding to
+          // succeed so we can display whatever we got. However, if the
+          // SourceBuffer was completed with a failing status, we want to fail.
+          // This happens only in exceptional situations like SourceBuffer
+          // itself encountering a failure due to OOM.
+          mTransition = NS_SUCCEEDED(aIterator.CompletionStatus())
+                      ? Transition::TerminateSuccess()
+                      : Transition::TerminateFailure();
+          break;
+
+        case SourceBufferIterator::READY:
+          // Process the new data that became available. This may result in us
+          // transitioning to a terminal state; we'll check if that happened at
+          // the bottom of the loop.
+          MOZ_ASSERT(aIterator.Data());
+          MOZ_ASSERT(aIterator.Length() > 0);
+          Lex(aIterator.Data(), aIterator.Length(), aFunc);
+          break;
+
+        default:
+          MOZ_ASSERT_UNREACHABLE("Unknown SourceBufferIterator state");
+          mTransition = Transition::TerminateFailure();
+      }
+    } while (!mTransition.NextStateIsTerminal());
+
+    // We're done. Return the terminal state.
+    return Some(mTransition.NextStateAsTerminal());
+  }
+
   template <typename Func>
   Maybe<TerminalState> Lex(const char* aInput, size_t aLength, Func aFunc)
   {
+    MOZ_ASSERT(aInput);
+
     if (mTransition.NextStateIsTerminal()) {
       // We've already reached a terminal state. We never deliver any more data
       // in this case; just return the terminal state again immediately.

image/decoders/nsBMPDecoder.cpp

@@ -447,13 +447,11 @@ nsBMPDecoder::FinishRow()
 }
 
 Maybe<TerminalState>
-nsBMPDecoder::DoDecode(SourceBufferIterator& aIterator)
+nsBMPDecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
-  MOZ_ASSERT(aIterator.Data());
-  MOZ_ASSERT(aIterator.Length() > 0);
 
-  return mLexer.Lex(aIterator.Data(), aIterator.Length(),
+  return mLexer.Lex(aIterator, aOnResume,
                     [=](State aState, const char* aData, size_t aLength) {
     switch (aState) {
       case State::FILE_HEADER:      return ReadFileHeader(aData, aLength);

image/decoders/nsBMPDecoder.h

@@ -142,7 +142,8 @@ public:
   /// bitmap has been fully decoded.)
   bool HasTransparency() const { return mDoesHaveTransparency; }
 
-  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) override;
+  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                IResumable* aOnResume) override;
   nsresult BeforeFinishInternal() override;
   nsresult FinishInternal() override;
 

image/decoders/nsGIFDecoder2.cpp

@@ -457,13 +457,11 @@ ConvertColormap(uint32_t* aColormap, uint32_t aColors)
 }
 
 Maybe<TerminalState>
-nsGIFDecoder2::DoDecode(SourceBufferIterator& aIterator)
+nsGIFDecoder2::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
-  MOZ_ASSERT(aIterator.Data());
-  MOZ_ASSERT(aIterator.Length() > 0);
 
-  return mLexer.Lex(aIterator.Data(), aIterator.Length(),
+  return mLexer.Lex(aIterator, aOnResume,
                     [=](State aState, const char* aData, size_t aLength) {
     switch(aState) {
       case State::GIF_HEADER:

image/decoders/nsGIFDecoder2.h

@@ -24,7 +24,8 @@ class nsGIFDecoder2 : public Decoder
 public:
   ~nsGIFDecoder2();
 
-  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) override;
+  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                IResumable* aOnResume) override;
   nsresult FinishInternal() override;
   virtual Telemetry::ID SpeedHistogram() override;
 

image/decoders/nsICODecoder.cpp

@@ -54,7 +54,6 @@ nsICODecoder::GetNumColors()
 nsICODecoder::nsICODecoder(RasterImage* aImage)
   : Decoder(aImage)
   , mLexer(Transition::To(ICOState::HEADER, ICOHEADERSIZE))
-  , mDoNotResume(WrapNotNull(new DoNotResume))
   , mBiggestResourceColorDepth(0)
   , mBestResourceDelta(INT_MIN)
   , mBestResourceColorDepth(0)
@@ -95,7 +94,7 @@ nsICODecoder::GetFinalStateFromContainedDecoder()
   // Let the contained decoder finish up if necessary.
   if (!mContainedSourceBuffer->IsComplete()) {
     mContainedSourceBuffer->Complete(NS_OK);
-    mContainedDecoder->Decode(mDoNotResume);
+    mContainedDecoder->Decode();
   }
 
   // Make our state the same as the state of the contained decoder.
@@ -590,13 +589,11 @@ nsICODecoder::FinishResource()
 }
 
 Maybe<TerminalState>
-nsICODecoder::DoDecode(SourceBufferIterator& aIterator)
+nsICODecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
-  MOZ_ASSERT(aIterator.Data());
-  MOZ_ASSERT(aIterator.Length() > 0);
 
-  return mLexer.Lex(aIterator.Data(), aIterator.Length(),
+  return mLexer.Lex(aIterator, aOnResume,
                     [=](ICOState aState, const char* aData, size_t aLength) {
     switch (aState) {
       case ICOState::HEADER:
@@ -647,7 +644,7 @@ 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(mDoNotResume))) {
+  if (NS_FAILED(mContainedDecoder->Decode())) {
     succeeded = false;
   }
 

image/decoders/nsICODecoder.h

@@ -11,7 +11,6 @@
 #include "Decoder.h"
 #include "imgFrame.h"
 #include "mozilla/gfx/2D.h"
-#include "mozilla/NotNull.h"
 #include "nsBMPDecoder.h"
 #include "nsPNGDecoder.h"
 #include "ICOFileHeaders.h"
@@ -70,7 +69,8 @@ public:
   /// @return The offset from the beginning of the ICO to the first resource.
   size_t FirstResourceOffset() const;
 
-  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) override;
+  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                IResumable* aOnResume) override;
   nsresult FinishInternal() override;
   nsresult FinishWithErrorInternal() override;
 
@@ -111,22 +111,9 @@ private:
   LexerTransition<ICOState> FinishMask();
   LexerTransition<ICOState> FinishResource();
 
-  // A helper implementation of IResumable which just does nothing; see
-  // WriteToContainedDecoder() for more details.
-  class DoNotResume final : public IResumable
-  {
-  public:
-    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DoNotResume, override)
-    void Resume() override { }
-
-  private:
-    virtual ~DoNotResume() { }
-  };
-
   StreamingLexer<ICOState, 32> mLexer; // The lexer.
   RefPtr<Decoder> mContainedDecoder; // Either a BMP or PNG decoder.
   RefPtr<SourceBuffer> mContainedSourceBuffer;  // SourceBuffer for mContainedDecoder.
-  NotNull<RefPtr<IResumable>> mDoNotResume;  // IResumable helper for SourceBuffer.
   UniquePtr<uint8_t[]> mMaskBuffer;    // A temporary buffer for the alpha mask.
   char mBIHraw[bmp::InfoHeaderLength::WIN_ICO]; // The bitmap information header.
   IconDirEntry mDirEntry;              // The dir entry for the selected resource.

image/decoders/nsIconDecoder.cpp

@@ -27,13 +27,11 @@ nsIconDecoder::~nsIconDecoder()
 { }
 
 Maybe<TerminalState>
-nsIconDecoder::DoDecode(SourceBufferIterator& aIterator)
+nsIconDecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
-  MOZ_ASSERT(aIterator.Data());
-  MOZ_ASSERT(aIterator.Length() > 0);
 
-  return mLexer.Lex(aIterator.Data(), aIterator.Length(),
+  return mLexer.Lex(aIterator, aOnResume,
                     [=](State aState, const char* aData, size_t aLength) {
     switch (aState) {
       case State::HEADER:

image/decoders/nsIconDecoder.h

@@ -37,7 +37,8 @@ class nsIconDecoder : public Decoder
 public:
   virtual ~nsIconDecoder();
 
-  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) override;
+  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                IResumable* aOnResume) override;
 
 private:
   friend class DecoderFactory;

image/decoders/nsJPEGDecoder.cpp

@@ -183,13 +183,11 @@ nsJPEGDecoder::FinishInternal()
 }
 
 Maybe<TerminalState>
-nsJPEGDecoder::DoDecode(SourceBufferIterator& aIterator)
+nsJPEGDecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
-  MOZ_ASSERT(aIterator.Data());
-  MOZ_ASSERT(aIterator.Length() > 0);
 
-  return mLexer.Lex(aIterator.Data(), aIterator.Length(),
+  return mLexer.Lex(aIterator, aOnResume,
                     [=](State aState, const char* aData, size_t aLength) {
     switch (aState) {
       case State::JPEG_DATA:

image/decoders/nsJPEGDecoder.h

@@ -58,7 +58,8 @@ public:
   }
 
   nsresult InitInternal() override;
-  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) override;
+  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                IResumable* aOnResume) override;
   nsresult FinishInternal() override;
 
   virtual Telemetry::ID SpeedHistogram() override;

image/decoders/nsPNGDecoder.cpp

@@ -348,13 +348,11 @@ nsPNGDecoder::InitInternal()
 }
 
 Maybe<TerminalState>
-nsPNGDecoder::DoDecode(SourceBufferIterator& aIterator)
+nsPNGDecoder::DoDecode(SourceBufferIterator& aIterator, IResumable* aOnResume)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
-  MOZ_ASSERT(aIterator.Data());
-  MOZ_ASSERT(aIterator.Length() > 0);
 
-  return mLexer.Lex(aIterator.Data(), aIterator.Length(),
+  return mLexer.Lex(aIterator, aOnResume,
                     [=](State aState, const char* aData, size_t aLength) {
     switch (aState) {
       case State::PNG_DATA:

image/decoders/nsPNGDecoder.h

@@ -23,7 +23,8 @@ public:
   virtual ~nsPNGDecoder();
 
   nsresult InitInternal() override;
-  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator) override;
+  Maybe<TerminalState> DoDecode(SourceBufferIterator& aIterator,
+                                IResumable* aOnResume) override;
   virtual Telemetry::ID SpeedHistogram() override;
 
   /// @return true if this PNG is a valid ICO resource.