Bug 1210291 - Streamline StreamingLexer's handling of terminal states. r=seth.

This patch introduces TerminalState and changes LexerTransition::mNextState to
be a Variant<State, TerminalState>. This means that SUCCESS and FAILURE no
longer need to be part of State.

Some things to note:

- This simplifies the handling of Lex()'s return value, which is nice.

- The patch splits Terminate() into TerminateSuccess() and TerminateFailure().

- |const State& aNextState| wouldn't work for the first arg to
  LexerTransition's ctor due to errors in Variant construction that I didn't
  understand. I had to change it to |State aNextState|.

image/StreamingLexer.h

@@ -1,4 +1,5 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/. */
@@ -14,7 +15,9 @@
 
 #include <algorithm>
 #include "mozilla/Assertions.h"
+#include "mozilla/Attributes.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/Variant.h"
 #include "mozilla/Vector.h"
 
 namespace mozilla {
@@ -27,13 +30,12 @@ enum class BufferingStrategy
   UNBUFFERED  // Data will be processed as it arrives, in multiple chunks.
 };
 
-/// @return true if @aState is a terminal state.
+/// The result of a call to StreamingLexer::Lex().
-template <typename State>
+enum class TerminalState
-bool IsTerminalState(State aState)
 {
-  return aState == State::SUCCESS ||
+  SUCCESS,
-         aState == State::FAILURE;
+  FAILURE
-}
+};
 
 /**
  * LexerTransition is a type used to give commands to the lexing framework.
@@ -45,33 +47,78 @@ template <typename State>
 class LexerTransition
 {
 public:
-  State NextState() const { return mNextState; }
+  // This is implicit so that Terminate{Success,Failure}() can return a
-  State UnbufferedState() const { return *mUnbufferedState; }
+  // TerminalState and have it implicitly converted to a
-  size_t Size() const { return mSize; }
+  // LexerTransition<State>, which avoids the need for a "<State>"
-  BufferingStrategy Buffering() const { return mBufferingStrategy; }
+  // qualification to the Terminate{Success,Failure}() callsite.
+  MOZ_IMPLICIT LexerTransition(TerminalState aFinalState)
+    : mNextState(aFinalState)
+  {}
+
+  bool NextStateIsTerminal() const
+  {
+    return mNextState.template is<TerminalState>();
+  }
+
+  TerminalState NextStateAsTerminal() const
+  {
+    return mNextState.template as<TerminalState>();
+  }
+
+  State NextState() const
+  {
+    return mNextState.template as<NonTerminalState>().mState;
+  }
+
+  State UnbufferedState() const
+  {
+    return *mNextState.template as<NonTerminalState>().mUnbufferedState;
+  }
+
+  size_t Size() const
+  {
+    return mNextState.template as<NonTerminalState>().mSize;
+  }
+
+  BufferingStrategy Buffering() const
+  {
+    return mNextState.template as<NonTerminalState>().mBufferingStrategy;
+  }
 
 private:
   friend struct Transition;
 
-  LexerTransition(const State& aNextState,
+  LexerTransition(State aNextState,
                   const Maybe<State>& aUnbufferedState,
                   size_t aSize,
                   BufferingStrategy aBufferingStrategy)
-    : mNextState(aNextState)
+    : mNextState(NonTerminalState(aNextState, aUnbufferedState, aSize,
-    , mUnbufferedState(aUnbufferedState)
+                                  aBufferingStrategy))
-    , mSize(aSize)
+  {}
-    , mBufferingStrategy(aBufferingStrategy)
-  {
-    MOZ_ASSERT_IF(mBufferingStrategy == BufferingStrategy::UNBUFFERED,
-                  mUnbufferedState);
-    MOZ_ASSERT_IF(mUnbufferedState,
-                  mBufferingStrategy == BufferingStrategy::UNBUFFERED);
-  }
 
-  State mNextState;
+  struct NonTerminalState
-  Maybe<State> mUnbufferedState;
+  {
-  size_t mSize;
+    State mState;
-  BufferingStrategy mBufferingStrategy;
+    Maybe<State> mUnbufferedState;
+    size_t mSize;
+    BufferingStrategy mBufferingStrategy;
+
+    NonTerminalState(State aState,
+                     const Maybe<State>& aUnbufferedState,
+                     size_t aSize,
+                     BufferingStrategy aBufferingStrategy)
+      : mState(aState)
+      , mUnbufferedState(aUnbufferedState)
+      , mSize(aSize)
+      , mBufferingStrategy(aBufferingStrategy)
+    {
+      MOZ_ASSERT_IF(mBufferingStrategy == BufferingStrategy::UNBUFFERED,
+                    mUnbufferedState);
+      MOZ_ASSERT_IF(mUnbufferedState,
+                    mBufferingStrategy == BufferingStrategy::UNBUFFERED);
+    }
+  };
+  Variant<NonTerminalState, TerminalState> mNextState;
 };
 
 struct Transition
@@ -81,7 +128,6 @@ struct Transition
   static LexerTransition<State>
   To(const State& aNextState, size_t aSize)
   {
-    MOZ_ASSERT(!IsTerminalState(aNextState));
     return LexerTransition<State>(aNextState, Nothing(), aSize,
                                   BufferingStrategy::BUFFERED);
   }
@@ -102,8 +148,6 @@ struct Transition
                const State& aUnbufferedState,
                size_t aSize)
   {
-    MOZ_ASSERT(!IsTerminalState(aNextState));
-    MOZ_ASSERT(!IsTerminalState(aUnbufferedState));
     return LexerTransition<State>(aNextState, Some(aUnbufferedState), aSize,
                                   BufferingStrategy::UNBUFFERED);
   }
@@ -119,23 +163,34 @@ struct Transition
   static LexerTransition<State>
   ContinueUnbuffered(const State& aUnbufferedState)
   {
-    MOZ_ASSERT(!IsTerminalState(aUnbufferedState));
     return LexerTransition<State>(aUnbufferedState, Nothing(), 0,
                                   BufferingStrategy::BUFFERED);
   }
 
   /**
-   * Terminate lexing, ending up in terminal state @aFinalState.
+   * Terminate lexing, ending up in terminal state SUCCESS. (The implicit
+   * LexerTransition constructor will convert the result to a LexerTransition
+   * as needed.)
    *
-   * No more data will be delivered after Terminate() is used.
+   * No more data will be delivered after this function is used.
    */
-  template <typename State>
+  static TerminalState
-  static LexerTransition<State>
+  TerminateSuccess()
-  Terminate(const State& aFinalState)
   {
-    MOZ_ASSERT(IsTerminalState(aFinalState));
+    return TerminalState::SUCCESS;
-    return LexerTransition<State>(aFinalState, Nothing(), 0,
+  }
-                                  BufferingStrategy::BUFFERED);
+
+  /**
+   * Terminate lexing, ending up in terminal state FAILURE. (The implicit
+   * LexerTransition constructor will convert the result to a LexerTransition
+   * as needed.)
+   *
+   * No more data will be delivered after this function is used.
+   */
+  static TerminalState
+  TerminateFailure()
+  {
+    return TerminalState::FAILURE;
   }
 
 private:
@@ -151,7 +206,7 @@ private:
  *
  *  - Create a State type. This should be an |enum class| listing all of the
  *    states that you can be in while lexing the image format you're trying to
- *    read. It must contain the two terminal states SUCCESS and FAILURE.
+ *    read.
  *
  *  - Add an instance of StreamingLexer<State> to your decoder class. Initialize
  *    it with a Transition::To() the state that you want to start lexing in.
@@ -166,9 +221,9 @@ private:
  *
  *  - Write the methods that actually implement lexing for your image format.
  *    These methods should return either Transition::To(), to move on to another
- *    state, or Transition::Terminate(), if lexing has terminated in either
+ *    state, or Transition::Terminate{Success,Failure}(), if lexing has
- *    success or failure. (There are also additional transitions for unbuffered
+ *    terminated in either success or failure. (There are also additional
- *    reads; see below.)
+ *    transitions for unbuffered reads; see below.)
  *
  * That's all there is to it. The StreamingLexer will track your position in the
  * input and buffer enough data so that your lexing methods can process
@@ -208,12 +263,12 @@ public:
   { }
 
   template <typename Func>
-  Maybe<State> Lex(const char* aInput, size_t aLength, Func aFunc)
+  Maybe<TerminalState> Lex(const char* aInput, size_t aLength, Func aFunc)
   {
-    if (IsTerminalState(mTransition.NextState())) {
+    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.NextState());
+      return Some(mTransition.NextStateAsTerminal());
     }
 
     if (mToReadUnbuffered > 0) {
@@ -225,15 +280,15 @@ public:
 
       size_t toRead = std::min(mToReadUnbuffered, aLength);
 
-      // Call aFunc with the unbuffered state to indicate that we're in the middle
+      // Call aFunc with the unbuffered state to indicate that we're in the
-      // of an unbuffered read. We enforce that any state transition passed back
+      // middle of an unbuffered read. We enforce that any state transition
-      // to us is either a terminal states or takes us back to the unbuffered
+      // passed back to us is either a terminal state or takes us back to the
-      // state.
+      // unbuffered state.
       LexerTransition<State> unbufferedTransition =
         aFunc(mTransition.UnbufferedState(), aInput, toRead);
-      if (IsTerminalState(unbufferedTransition.NextState())) {
+      if (unbufferedTransition.NextStateIsTerminal()) {
         mTransition = unbufferedTransition;
-        return Some(mTransition.NextState());  // Done!
+        return Some(mTransition.NextStateAsTerminal());  // Done!
       }
       MOZ_ASSERT(mTransition.UnbufferedState() ==
                    unbufferedTransition.NextState());
@@ -247,8 +302,8 @@ public:
 
       // We're done with the unbuffered read, so transition to the next state.
       mTransition = aFunc(mTransition.NextState(), nullptr, 0);
-      if (IsTerminalState(mTransition.NextState())) {
+      if (mTransition.NextStateIsTerminal()) {
-        return Some(mTransition.NextState());  // Done!
+        return Some(mTransition.NextStateAsTerminal());  // Done!
       }
     } else if (0 < mBuffer.length()) {
       // We're continuing a buffered read.
@@ -272,8 +327,8 @@ public:
       mTransition =
         aFunc(mTransition.NextState(), mBuffer.begin(), mBuffer.length());
       mBuffer.clear();
-      if (IsTerminalState(mTransition.NextState())) {
+      if (mTransition.NextStateIsTerminal()) {
-        return Some(mTransition.NextState());  // Done!
+        return Some(mTransition.NextStateAsTerminal());  // Done!
       }
     }
 
@@ -291,13 +346,13 @@ public:
 
         // Call aFunc with the unbuffered state to indicate that we're in the
         // middle of an unbuffered read. We enforce that any state transition
-        // passed back to us is either a terminal states or takes us back to the
+        // passed back to us is either a terminal state or takes us back to the
         // unbuffered state.
         LexerTransition<State> unbufferedTransition =
           aFunc(mTransition.UnbufferedState(), aInput, toRead);
-        if (IsTerminalState(unbufferedTransition.NextState())) {
+        if (unbufferedTransition.NextStateIsTerminal()) {
           mTransition = unbufferedTransition;
-          return Some(mTransition.NextState());  // Done!
+          return Some(mTransition.NextStateAsTerminal());  // Done!
         }
         MOZ_ASSERT(mTransition.UnbufferedState() ==
                      unbufferedTransition.NextState());
@@ -309,8 +364,8 @@ public:
       aInput += toRead;
       aLength -= toRead;
 
-      if (IsTerminalState(mTransition.NextState())) {
+      if (mTransition.NextStateIsTerminal()) {
-        return Some(mTransition.NextState());  // Done!
+        return Some(mTransition.NextStateAsTerminal());  // Done!
       }
     }
 
@@ -323,9 +378,9 @@ public:
     if (mTransition.Buffering() == BufferingStrategy::UNBUFFERED) {
       LexerTransition<State> unbufferedTransition =
         aFunc(mTransition.UnbufferedState(), aInput, aLength);
-      if (IsTerminalState(unbufferedTransition.NextState())) {
+      if (unbufferedTransition.NextStateIsTerminal()) {
         mTransition = unbufferedTransition;
-        return Some(mTransition.NextState());  // Done!
+        return Some(mTransition.NextStateAsTerminal());  // Done!
       }
       MOZ_ASSERT(mTransition.UnbufferedState() ==
                    unbufferedTransition.NextState());
@@ -333,11 +388,11 @@ public:
       mToReadUnbuffered = mTransition.Size() - aLength;
       return Nothing();  // Need more input.
     }
-    
+
     // If the next state is buffered, buffer what we can and then wait.
     MOZ_ASSERT(mTransition.Buffering() == BufferingStrategy::BUFFERED);
     if (!mBuffer.reserve(mTransition.Size())) {
-      return Some(State::FAILURE);  // Done due to allocation failure.
+      return Some(TerminalState::FAILURE);  // Done due to allocation failure.
     }
     mBuffer.append(aInput, aLength);
     return Nothing();  // Need more input.

image/decoders/nsBMPDecoder.cpp

@@ -437,7 +437,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
   MOZ_ASSERT(aBuffer);
   MOZ_ASSERT(aCount > 0);
 
-  Maybe<State> terminalState =
+  Maybe<TerminalState> terminalState =
     mLexer.Lex(aBuffer, aCount, [=](State aState,
                                     const char* aData, size_t aLength) {
       switch (aState) {
@@ -452,23 +452,13 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
         case State::RLE_DELTA:        return ReadRLEDelta(aData);
         case State::RLE_ABSOLUTE:     return ReadRLEAbsolute(aData, aLength);
         default:
-          MOZ_ASSERT_UNREACHABLE("Unknown State");
+          MOZ_CRASH("Unknown State");
-          return Transition::Terminate(State::FAILURE);
       }
     });
 
-  if (!terminalState) {
+  if (terminalState == Some(TerminalState::FAILURE)) {
-    return;  // Need more data.
-  }
-
-  if (*terminalState == State::FAILURE) {
     PostDataError();
-    return;
   }
-
-  MOZ_ASSERT(*terminalState == State::SUCCESS);
-
-  return;
 }
 
 LexerTransition<nsBMPDecoder::State>
@@ -479,7 +469,7 @@ nsBMPDecoder::ReadFileHeader(const char* aData, size_t aLength)
   bool signatureOk = aData[0] == 'B' && aData[1] == 'M';
   if (!signatureOk) {
     PostDataError();
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // We ignore the filesize (aData + 2) and reserved (aData + 6) fields.
@@ -506,7 +496,7 @@ nsBMPDecoder::ReadInfoHeaderSize(const char* aData, size_t aLength)
                     mH.mBIHSize <= InfoHeaderLength::OS2_V2_MAX);
   if (!bihSizeOk) {
     PostDataError();
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
   // ICO BMPs must have a WinVMPv3 header. nsICODecoder should have already
   // terminated decoding if this isn't the case.
@@ -561,7 +551,7 @@ nsBMPDecoder::ReadInfoHeaderRest(const char* aData, size_t aLength)
                 mH.mHeight != INT_MIN;
   if (!sizeOk) {
     PostDataError();
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Check mBpp and mCompression.
@@ -575,7 +565,7 @@ nsBMPDecoder::ReadInfoHeaderRest(const char* aData, size_t aLength)
       (mH.mBpp == 16 || mH.mBpp == 32));
   if (!bppCompressionOk) {
     PostDataError();
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Post our size to the superclass.
@@ -652,7 +642,7 @@ nsBMPDecoder::ReadBitfields(const char* aData, size_t aLength)
   // We've now read all the headers. If we're doing a metadata decode, we're
   // done.
   if (IsMetadataDecode()) {
-    return Transition::Terminate(State::SUCCESS);
+    return Transition::TerminateSuccess();
   }
 
   // Set up the color table, if present; it'll be filled in by ReadColorTable().
@@ -677,7 +667,7 @@ nsBMPDecoder::ReadBitfields(const char* aData, size_t aLength)
                               IntRect(IntPoint(), targetSize),
                               SurfaceFormat::B8G8R8A8);
   if (NS_FAILED(rv)) {
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
   MOZ_ASSERT(mImageData, "Should have a buffer now");
 
@@ -688,7 +678,7 @@ nsBMPDecoder::ReadBitfields(const char* aData, size_t aLength)
                                  mImageData, mMayHaveTransparency,
                                  /* aFlipVertically = */ true);
     if (NS_FAILED(rv)) {
-      return Transition::Terminate(State::FAILURE);
+      return Transition::TerminateFailure();
     }
   }
 
@@ -719,7 +709,7 @@ nsBMPDecoder::ReadColorTable(const char* aData, size_t aLength)
   // we give up.
   if (mPreGapLength > mH.mDataOffset) {
     PostDataError();
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
   uint32_t gapLength = mH.mDataOffset - mPreGapLength;
   return Transition::To(State::GAP, gapLength);
@@ -866,7 +856,7 @@ nsBMPDecoder::ReadPixelRow(const char* aData)
 
   FinishRow();
   return mCurrentRow == 0
-       ? Transition::Terminate(State::SUCCESS)
+       ? Transition::TerminateSuccess()
        : Transition::To(State::PIXEL_ROW, mPixelRowSize);
 }
 
@@ -874,7 +864,7 @@ LexerTransition<nsBMPDecoder::State>
 nsBMPDecoder::ReadRLESegment(const char* aData)
 {
   if (mCurrentRow == 0) {
-    return Transition::Terminate(State::SUCCESS);
+    return Transition::TerminateSuccess();
   }
 
   uint8_t byte1 = uint8_t(aData[0]);
@@ -909,12 +899,12 @@ nsBMPDecoder::ReadRLESegment(const char* aData)
     mCurrentPos = 0;
     FinishRow();
     return mCurrentRow == 0
-         ? Transition::Terminate(State::SUCCESS)
+         ? Transition::TerminateSuccess()
          : Transition::To(State::RLE_SEGMENT, RLE::SEGMENT_LENGTH);
   }
 
   if (byte2 == RLE::ESCAPE_EOF) {
-    return Transition::Terminate(State::SUCCESS);
+    return Transition::TerminateSuccess();
   }
 
   if (byte2 == RLE::ESCAPE_DELTA) {
@@ -972,7 +962,7 @@ nsBMPDecoder::ReadRLEDelta(const char* aData)
   }
 
   return mCurrentRow == 0
-       ? Transition::Terminate(State::SUCCESS)
+       ? Transition::TerminateSuccess()
        : Transition::To(State::RLE_SEGMENT, RLE::SEGMENT_LENGTH);
 }
 
@@ -985,7 +975,7 @@ nsBMPDecoder::ReadRLEAbsolute(const char* aData, size_t aLength)
   if (mCurrentPos + n > uint32_t(mH.mWidth)) {
     // Bad data. Stop decoding; at least part of the image may have been
     // decoded.
-    return Transition::Terminate(State::SUCCESS);
+    return Transition::TerminateSuccess();
   }
 
   // In absolute mode, n represents the number of pixels that follow, each of

image/decoders/nsBMPDecoder.h

@@ -167,9 +167,7 @@ private:
     PIXEL_ROW,
     RLE_SEGMENT,
     RLE_DELTA,
-    RLE_ABSOLUTE,
+    RLE_ABSOLUTE
-    SUCCESS,
-    FAILURE
   };
 
   // This is the constructor used by DecoderFactory.

image/decoders/nsICODecoder.cpp

@@ -170,14 +170,14 @@ nsICODecoder::ReadHeader(const char* aData)
 {
   // If the third byte is 1, this is an icon. If 2, a cursor.
   if ((aData[2] != 1) && (aData[2] != 2)) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
   mIsCursor = (aData[2] == 2);
 
   // The fifth and sixth bytes specify the number of resources in the file.
   mNumIcons = LittleEndian::readUint16(aData + 4);
   if (mNumIcons == 0) {
-    return Transition::Terminate(ICOState::SUCCESS); // Nothing to do.
+    return Transition::TerminateSuccess(); // Nothing to do.
   }
 
   // Downscale-during-decode can end up decoding different resources in the ICO
@@ -257,7 +257,7 @@ nsICODecoder::ReadDirEntry(const char* aData)
   if (mCurrIcon == mNumIcons) {
     // Ensure the resource we selected has an offset past the ICO headers.
     if (mDirEntry.mImageOffset < FirstResourceOffset()) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
 
     // If this is a cursor, set the hotspot. We use the hotspot from the biggest
@@ -272,7 +272,7 @@ nsICODecoder::ReadDirEntry(const char* aData)
     // attempt to *upscale* while decoding.
     PostSize(mBiggestResourceSize.width, mBiggestResourceSize.height);
     if (IsMetadataDecode()) {
-      return Transition::Terminate(ICOState::SUCCESS);
+      return Transition::TerminateSuccess();
     }
 
     // If the resource we selected matches the downscaler's target size
@@ -309,11 +309,11 @@ nsICODecoder::SniffResource(const char* aData)
     mContainedDecoder->Init();
 
     if (!WriteToContainedDecoder(aData, PNGSIGNATURESIZE)) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
 
     if (mDirEntry.mBytesInRes <= PNGSIGNATURESIZE) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
 
     // Read in the rest of the PNG unbuffered.
@@ -325,7 +325,7 @@ nsICODecoder::SniffResource(const char* aData)
     // Make sure we have a sane size for the bitmap information header.
     int32_t bihSize = LittleEndian::readUint32(aData);
     if (bihSize != static_cast<int32_t>(BITMAPINFOSIZE)) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
 
     // Buffer the first part of the bitmap information header.
@@ -341,13 +341,13 @@ LexerTransition<ICOState>
 nsICODecoder::ReadPNG(const char* aData, uint32_t aLen)
 {
   if (!WriteToContainedDecoder(aData, aLen)) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Raymond Chen says that 32bpp only are valid PNG ICOs
   // http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
   if (!static_cast<nsPNGDecoder*>(mContainedDecoder.get())->IsValidICO()) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   return Transition::ContinueUnbuffered(ICOState::READ_PNG);
@@ -372,7 +372,7 @@ nsICODecoder::ReadBIH(const char* aData)
     // The color table is present only if BPP is <= 8.
     uint16_t numColors = GetNumColors();
     if (numColors == (uint16_t)-1) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
     dataOffset += 4 * numColors;
   }
@@ -393,23 +393,23 @@ nsICODecoder::ReadBIH(const char* aData)
   // will understand, because the BMP decoder doesn't expect the alpha mask that
   // follows the BMP data in an ICO.
   if (!FixBitmapHeight(reinterpret_cast<int8_t*>(mBIHraw))) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Fix the ICO width from the BIH.
   if (!FixBitmapWidth(reinterpret_cast<int8_t*>(mBIHraw))) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Write out the BMP's bitmap info header.
   if (!WriteToContainedDecoder(mBIHraw, sizeof(mBIHraw))) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Check to make sure we have valid color settings.
   uint16_t numColors = GetNumColors();
   if (numColors == uint16_t(-1)) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // Do we have an AND mask on this BMP? If so, we need to read it after we read
@@ -429,7 +429,7 @@ LexerTransition<ICOState>
 nsICODecoder::ReadBMP(const char* aData, uint32_t aLen)
 {
   if (!WriteToContainedDecoder(aData, aLen)) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   return Transition::ContinueUnbuffered(ICOState::READ_BMP);
@@ -466,7 +466,7 @@ nsICODecoder::PrepareForMask()
   // we must have a truncated (and therefore corrupt) AND mask.
   uint32_t expectedLength = mMaskRowSize * GetRealHeight();
   if (maskLength < expectedLength) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
   // If we're downscaling, the mask is the wrong size for the surface we've
@@ -483,7 +483,7 @@ nsICODecoder::PrepareForMask()
                                           /* aHasAlpha = */ true,
                                           /* aFlipVertically = */ true);
     if (NS_FAILED(rv)) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
   }
 
@@ -516,7 +516,7 @@ nsICODecoder::ReadMaskRow(const char* aData)
       static_cast<nsBMPDecoder*>(mContainedDecoder.get());
     uint32_t* imageData = bmpDecoder->GetImageData();
     if (!imageData) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
 
     decoded = imageData + mCurrMaskLine * GetRealWidth();
@@ -566,7 +566,7 @@ nsICODecoder::FinishMask()
       static_cast<nsBMPDecoder*>(mContainedDecoder.get());
     uint8_t* imageData = reinterpret_cast<uint8_t*>(bmpDecoder->GetImageData());
     if (!imageData) {
-      return Transition::Terminate(ICOState::FAILURE);
+      return Transition::TerminateFailure();
     }
 
     // Iterate through the alpha values, copying from mask to image.
@@ -596,10 +596,10 @@ nsICODecoder::FinishResource()
   // entry. If not, we consider the image corrupt.
   if (mContainedDecoder->HasSize() &&
       mContainedDecoder->GetSize() != GetRealSize()) {
-    return Transition::Terminate(ICOState::FAILURE);
+    return Transition::TerminateFailure();
   }
 
-  return Transition::Terminate(ICOState::SUCCESS);
+  return Transition::TerminateSuccess();
 }
 
 void
@@ -609,7 +609,7 @@ nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
   MOZ_ASSERT(aBuffer);
   MOZ_ASSERT(aCount > 0);
 
-  Maybe<ICOState> terminalState =
+  Maybe<TerminalState> terminalState =
     mLexer.Lex(aBuffer, aCount,
                [=](ICOState aState, const char* aData, size_t aLength) {
       switch (aState) {
@@ -640,21 +640,13 @@ nsICODecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
         case ICOState::FINISHED_RESOURCE:
           return FinishResource();
         default:
-          MOZ_ASSERT_UNREACHABLE("Unknown ICOState");
+          MOZ_CRASH("Unknown ICOState");
-          return Transition::Terminate(ICOState::FAILURE);
       }
     });
 
-  if (!terminalState) {
+  if (terminalState == Some(TerminalState::FAILURE)) {
-    return;  // Need more data.
-  }
-
-  if (*terminalState == ICOState::FAILURE) {
     PostDataError();
-    return;
   }
-
-  MOZ_ASSERT(*terminalState == ICOState::SUCCESS);
 }
 
 bool

image/decoders/nsICODecoder.h

@@ -21,8 +21,6 @@ class RasterImage;
 
 enum class ICOState
 {
-  SUCCESS,
-  FAILURE,
   HEADER,
   DIR_ENTRY,
   SKIP_TO_RESOURCE,

image/decoders/nsIconDecoder.cpp

@@ -42,7 +42,7 @@ nsIconDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
   MOZ_ASSERT(aBuffer);
   MOZ_ASSERT(aCount > 0);
 
-  Maybe<State> terminalState =
+  Maybe<TerminalState> terminalState =
     mLexer.Lex(aBuffer, aCount, [=](State aState,
                                     const char* aData, size_t aLength) {
       switch (aState) {
@@ -53,21 +53,13 @@ nsIconDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
         case State::FINISH:
           return Finish();
         default:
-          MOZ_ASSERT_UNREACHABLE("Unknown State");
+          MOZ_CRASH("Unknown State");
-          return Transition::Terminate(State::FAILURE);
       }
     });
 
-  if (!terminalState) {
+  if (terminalState == Some(TerminalState::FAILURE)) {
-    return;  // Need more data.
-  }
-
-  if (*terminalState == State::FAILURE) {
     PostDataError();
-    return;
   }
-
-  MOZ_ASSERT(*terminalState == State::SUCCESS);
 }
 
 LexerTransition<nsIconDecoder::State>
@@ -88,7 +80,7 @@ nsIconDecoder::ReadHeader(const char* aData)
 
   // If we're doing a metadata decode, we're done.
   if (IsMetadataDecode()) {
-    return Transition::Terminate(State::SUCCESS);
+    return Transition::TerminateSuccess();
   }
 
   MOZ_ASSERT(!mImageData, "Already have a buffer allocated?");
@@ -97,7 +89,7 @@ nsIconDecoder::ReadHeader(const char* aData)
                               IntRect(IntPoint(), targetSize),
                               gfx::SurfaceFormat::B8G8R8A8);
   if (NS_FAILED(rv)) {
-    return Transition::Terminate(State::FAILURE);
+    return Transition::TerminateFailure();
   }
   MOZ_ASSERT(mImageData, "Should have a buffer now");
 
@@ -105,7 +97,7 @@ nsIconDecoder::ReadHeader(const char* aData)
     nsresult rv = mDownscaler->BeginFrame(GetSize(), Nothing(),
                                           mImageData, /* aHasAlpha = */ true);
     if (NS_FAILED(rv)) {
-      return Transition::Terminate(State::FAILURE);
+      return Transition::TerminateFailure();
     }
   }
 
@@ -142,7 +134,7 @@ nsIconDecoder::Finish()
   PostFrameStop();
   PostDecodeDone();
 
-  return Transition::Terminate(State::SUCCESS);
+  return Transition::TerminateSuccess();
 }
 
 } // namespace image

image/decoders/nsIconDecoder.h

@@ -50,9 +50,7 @@ private:
   enum class State {
     HEADER,
     ROW_OF_PIXELS,
-    FINISH,
+    FINISH
-    SUCCESS,
-    FAILURE
   };
 
   LexerTransition<State> ReadHeader(const char* aData);

image/test/gtest/TestStreamingLexer.cpp

@@ -15,9 +15,7 @@ enum class TestState
   ONE,
   TWO,
   THREE,
-  UNBUFFERED,
+  UNBUFFERED
-  SUCCESS,
-  FAILURE
 };
 
 void
@@ -41,10 +39,9 @@ DoLex(TestState aState, const char* aData, size_t aLength)
       return Transition::To(TestState::THREE, 3);
     case TestState::THREE:
       CheckData(aData, aLength);
-      return Transition::Terminate(TestState::SUCCESS);
+      return Transition::TerminateSuccess();
     default:
-      EXPECT_TRUE(false);  // Shouldn't get here.
+      MOZ_CRASH("Unknown TestState");
-      return Transition::Terminate(TestState::FAILURE);
   }
 }
 
@@ -65,10 +62,9 @@ DoLexWithUnbuffered(TestState aState, const char* aData, size_t aLength,
       return Transition::To(TestState::THREE, 3);
     case TestState::THREE:
       CheckData(aData, aLength);
-      return Transition::Terminate(TestState::SUCCESS);
+      return Transition::TerminateSuccess();
     default:
-      EXPECT_TRUE(false);
+      MOZ_CRASH("Unknown TestState");
-      return Transition::Terminate(TestState::FAILURE);
   }
 }
 
@@ -80,10 +76,9 @@ DoLexWithUnbufferedTerminate(TestState aState, const char* aData, size_t aLength
       CheckData(aData, aLength);
       return Transition::ToUnbuffered(TestState::TWO, TestState::UNBUFFERED, 3);
     case TestState::UNBUFFERED:
-      return Transition::Terminate(TestState::SUCCESS);
+      return Transition::TerminateSuccess();
     default:
-      EXPECT_TRUE(false);
+      MOZ_CRASH("Unknown TestState");
-      return Transition::Terminate(TestState::FAILURE);
   }
 }
 
@@ -93,9 +88,9 @@ TEST(ImageStreamingLexer, SingleChunk)
   char data[9] = { 1, 2, 3, 1, 2, 3, 1, 2, 3 };
 
   // Test delivering all the data at once.
-  Maybe<TestState> result = lexer.Lex(data, sizeof(data), DoLex);
+  Maybe<TerminalState> result = lexer.Lex(data, sizeof(data), DoLex);
   EXPECT_TRUE(result.isSome());
-  EXPECT_EQ(TestState::SUCCESS, *result);
+  EXPECT_EQ(Some(TerminalState::SUCCESS), result);
 }
 
 TEST(ImageStreamingLexer, SingleChunkWithUnbuffered)
@@ -105,13 +100,13 @@ TEST(ImageStreamingLexer, SingleChunkWithUnbuffered)
   Vector<char> unbufferedVector;
 
   // Test delivering all the data at once.
-  Maybe<TestState> result =
+  Maybe<TerminalState> result =
     lexer.Lex(data, sizeof(data),
               [&](TestState aState, const char* aData, size_t aLength) {
       return DoLexWithUnbuffered(aState, aData, aLength, unbufferedVector);
   });
   EXPECT_TRUE(result.isSome());
-  EXPECT_EQ(TestState::SUCCESS, *result);
+  EXPECT_EQ(Some(TerminalState::SUCCESS), result);
 }
 
 TEST(ImageStreamingLexer, ChunkPerState)
@@ -121,11 +116,11 @@ TEST(ImageStreamingLexer, ChunkPerState)
 
   // Test delivering in perfectly-sized chunks, one per state.
   for (unsigned i = 0 ; i < 3 ; ++i) {
-    Maybe<TestState> result = lexer.Lex(data + 3 * i, 3, DoLex);
+    Maybe<TerminalState> result = lexer.Lex(data + 3 * i, 3, DoLex);
 
     if (i == 2) {
       EXPECT_TRUE(result.isSome());
-      EXPECT_EQ(TestState::SUCCESS, *result);
+      EXPECT_EQ(Some(TerminalState::SUCCESS), result);
     } else {
       EXPECT_TRUE(result.isNothing());
     }
@@ -140,7 +135,7 @@ TEST(ImageStreamingLexer, ChunkPerStateWithUnbuffered)
 
   // Test delivering in perfectly-sized chunks, one per state.
   for (unsigned i = 0 ; i < 3 ; ++i) {
-    Maybe<TestState> result =
+    Maybe<TerminalState> result =
       lexer.Lex(data + 3 * i, 3,
                 [&](TestState aState, const char* aData, size_t aLength) {
         return DoLexWithUnbuffered(aState, aData, aLength, unbufferedVector);
@@ -148,7 +143,7 @@ TEST(ImageStreamingLexer, ChunkPerStateWithUnbuffered)
 
     if (i == 2) {
       EXPECT_TRUE(result.isSome());
-      EXPECT_EQ(TestState::SUCCESS, *result);
+      EXPECT_EQ(Some(TerminalState::SUCCESS), result);
     } else {
       EXPECT_TRUE(result.isNothing());
     }
@@ -162,11 +157,11 @@ TEST(ImageStreamingLexer, OneByteChunks)
 
   // Test delivering in one byte chunks.
   for (unsigned i = 0 ; i < 9 ; ++i) {
-    Maybe<TestState> result = lexer.Lex(data + i, 1, DoLex);
+    Maybe<TerminalState> result = lexer.Lex(data + i, 1, DoLex);
 
     if (i == 8) {
       EXPECT_TRUE(result.isSome());
-      EXPECT_EQ(TestState::SUCCESS, *result);
+      EXPECT_EQ(Some(TerminalState::SUCCESS), result);
     } else {
       EXPECT_TRUE(result.isNothing());
     }
@@ -181,7 +176,7 @@ TEST(ImageStreamingLexer, OneByteChunksWithUnbuffered)
 
   // Test delivering in one byte chunks.
   for (unsigned i = 0 ; i < 9 ; ++i) {
-    Maybe<TestState> result =
+    Maybe<TerminalState> result =
       lexer.Lex(data + i, 1,
                 [&](TestState aState, const char* aData, size_t aLength) {
         return DoLexWithUnbuffered(aState, aData, aLength, unbufferedVector);
@@ -189,7 +184,7 @@ TEST(ImageStreamingLexer, OneByteChunksWithUnbuffered)
 
     if (i == 8) {
       EXPECT_TRUE(result.isSome());
-      EXPECT_EQ(TestState::SUCCESS, *result);
+      EXPECT_EQ(Some(TerminalState::SUCCESS), result);
     } else {
       EXPECT_TRUE(result.isNothing());
     }
@@ -202,23 +197,23 @@ TEST(ImageStreamingLexer, TerminateSuccess)
   char data[9] = { 1, 2, 3, 1, 2, 3, 1, 2, 3 };
 
   // Test that Terminate is "sticky".
-  Maybe<TestState> result =
+  Maybe<TerminalState> result =
     lexer.Lex(data, sizeof(data),
               [&](TestState aState, const char* aData, size_t aLength) {
       EXPECT_TRUE(aState == TestState::ONE);
-      return Transition::Terminate(TestState::SUCCESS);
+      return Transition::TerminateSuccess();
   });
   EXPECT_TRUE(result.isSome());
-  EXPECT_EQ(TestState::SUCCESS, *result);
+  EXPECT_EQ(Some(TerminalState::SUCCESS), result);
 
   result =
     lexer.Lex(data, sizeof(data),
               [&](TestState aState, const char* aData, size_t aLength) {
       EXPECT_TRUE(false);  // Shouldn't get here.
-      return Transition::Terminate(TestState::FAILURE);
+      return Transition::TerminateFailure();
   });
   EXPECT_TRUE(result.isSome());
-  EXPECT_EQ(TestState::SUCCESS, *result);
+  EXPECT_EQ(Some(TerminalState::SUCCESS), result);
 }
 
 TEST(ImageStreamingLexer, TerminateFailure)
@@ -227,23 +222,23 @@ TEST(ImageStreamingLexer, TerminateFailure)
   char data[9] = { 1, 2, 3, 1, 2, 3, 1, 2, 3 };
 
   // Test that Terminate is "sticky".
-  Maybe<TestState> result =
+  Maybe<TerminalState> result =
     lexer.Lex(data, sizeof(data),
               [&](TestState aState, const char* aData, size_t aLength) {
       EXPECT_TRUE(aState == TestState::ONE);
-      return Transition::Terminate(TestState::FAILURE);
+      return Transition::TerminateFailure();
   });
   EXPECT_TRUE(result.isSome());
-  EXPECT_EQ(TestState::FAILURE, *result);
+  EXPECT_EQ(Some(TerminalState::FAILURE), result);
 
   result =
     lexer.Lex(data, sizeof(data),
               [&](TestState aState, const char* aData, size_t aLength) {
       EXPECT_TRUE(false);  // Shouldn't get here.
-      return Transition::Terminate(TestState::FAILURE);
+      return Transition::TerminateFailure();
   });
   EXPECT_TRUE(result.isSome());
-  EXPECT_EQ(TestState::FAILURE, *result);
+  EXPECT_EQ(Some(TerminalState::FAILURE), result);
 }
 
 TEST(ImageStreamingLexer, TerminateUnbuffered)
@@ -253,12 +248,12 @@ TEST(ImageStreamingLexer, TerminateUnbuffered)
 
   // Test that Terminate works during an unbuffered read.
   for (unsigned i = 0 ; i < 9 ; ++i) {
-    Maybe<TestState> result =
+    Maybe<TerminalState> result =
       lexer.Lex(data + i, 1, DoLexWithUnbufferedTerminate);
 
     if (i > 2) {
       EXPECT_TRUE(result.isSome());
-      EXPECT_EQ(TestState::SUCCESS, *result);
+      EXPECT_EQ(Some(TerminalState::SUCCESS), result);
     } else {
       EXPECT_TRUE(result.isNothing());
     }