Bug 1291059 - Yield in the GIF decoder at the beginning of a new frame, not at the end of the previous frame. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 29 Jul 2016 16:44:16 -0700
changeset 349122 8248ff77be0fa18183018e081ad62c9fb050b6d6
parent 349121 b617b61db290b61a34ad0471460989b816e8142b
child 349123 02f8c8bd057d7ccc5096c880ec3458174008426c
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1291059
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 1291059 - Yield in the GIF decoder at the beginning of a new frame, not at the end of the previous frame. r=edwin
image/decoders/nsGIFDecoder2.cpp
image/decoders/nsGIFDecoder2.h
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -465,30 +465,30 @@ nsGIFDecoder2::DoDecode(SourceBufferIter
       case State::SCREEN_DESCRIPTOR:
         return ReadScreenDescriptor(aData);
       case State::GLOBAL_COLOR_TABLE:
         return ReadGlobalColorTable(aData, aLength);
       case State::FINISHED_GLOBAL_COLOR_TABLE:
         return FinishedGlobalColorTable();
       case State::BLOCK_HEADER:
         return ReadBlockHeader(aData);
-      case State::BLOCK_HEADER_AFTER_YIELD:
-        return Transition::To(State::BLOCK_HEADER, BLOCK_HEADER_LEN);
       case State::EXTENSION_HEADER:
         return ReadExtensionHeader(aData);
       case State::GRAPHIC_CONTROL_EXTENSION:
         return ReadGraphicControlExtension(aData);
       case State::APPLICATION_IDENTIFIER:
         return ReadApplicationIdentifier(aData);
       case State::NETSCAPE_EXTENSION_SUB_BLOCK:
         return ReadNetscapeExtensionSubBlock(aData);
       case State::NETSCAPE_EXTENSION_DATA:
         return ReadNetscapeExtensionData(aData);
       case State::IMAGE_DESCRIPTOR:
         return ReadImageDescriptor(aData);
+      case State::FINISH_IMAGE_DESCRIPTOR:
+        return FinishImageDescriptor(aData);
       case State::LOCAL_COLOR_TABLE:
         return ReadLocalColorTable(aData, aLength);
       case State::FINISHED_LOCAL_COLOR_TABLE:
         return FinishedLocalColorTable();
       case State::IMAGE_DATA_BLOCK:
         return ReadImageDataBlock(aData);
       case State::IMAGE_DATA_SUB_BLOCK:
         return ReadImageDataSubBlock(aData);
@@ -749,39 +749,50 @@ nsGIFDecoder2::ReadNetscapeExtensionData
     default:
       return Transition::TerminateFailure();
   }
 }
 
 LexerTransition<nsGIFDecoder2::State>
 nsGIFDecoder2::ReadImageDescriptor(const char* aData)
 {
-  if (mGIFStruct.images_decoded == 1) {
-    if (!HasAnimation()) {
-      // We should've already called PostIsAnimated(); this must be a corrupt
-      // animated image with a first frame timeout of zero. Signal that we're
-      // animated now, before the first-frame decode early exit below, so that
-      // RasterImage can detect that this happened.
-      PostIsAnimated(FrameTimeout::FromRawMilliseconds(0));
-    }
+  // On the first frame, we don't need to yield, and none of the other checks
+  // below apply, so we can just jump right into FinishImageDescriptor().
+  if (mGIFStruct.images_decoded == 0) {
+    return FinishImageDescriptor(aData);
+  }
 
-    if (IsFirstFrameDecode()) {
-      // We're about to get a second frame, but we only want the first. Stop
-      // decoding now.
-      FinishInternal();
-      return Transition::TerminateSuccess();
-    }
-
-    if (mDownscaler) {
-      MOZ_ASSERT_UNREACHABLE("Doing downscale-during-decode for an animated "
-                             "image?");
-      mDownscaler.reset();
-    }
+  if (!HasAnimation()) {
+    // We should've already called PostIsAnimated(); this must be a corrupt
+    // animated image with a first frame timeout of zero. Signal that we're
+    // animated now, before the first-frame decode early exit below, so that
+    // RasterImage can detect that this happened.
+    PostIsAnimated(FrameTimeout::FromRawMilliseconds(0));
   }
 
+  if (IsFirstFrameDecode()) {
+    // We're about to get a second frame, but we only want the first. Stop
+    // decoding now.
+    FinishInternal();
+    return Transition::TerminateSuccess();
+  }
+
+  if (mDownscaler) {
+    MOZ_ASSERT_UNREACHABLE("Doing downscale-during-decode for an animated "
+                           "image?");
+    mDownscaler.reset();
+  }
+
+  // Yield to allow access to the previous frame before we start a new one.
+  return Transition::ToAfterYield(State::FINISH_IMAGE_DESCRIPTOR);
+}
+
+LexerTransition<nsGIFDecoder2::State>
+nsGIFDecoder2::FinishImageDescriptor(const char* aData)
+{
   IntRect frameRect;
 
   // Get image offsets with respect to the screen origin.
   frameRect.x = LittleEndian::readUint16(aData + 0);
   frameRect.y = LittleEndian::readUint16(aData + 2);
   frameRect.width = LittleEndian::readUint16(aData + 4);
   frameRect.height = LittleEndian::readUint16(aData + 6);
 
@@ -967,17 +978,17 @@ nsGIFDecoder2::ReadImageDataBlock(const 
 
 LexerTransition<nsGIFDecoder2::State>
 nsGIFDecoder2::ReadImageDataSubBlock(const char* aData)
 {
   const uint8_t subBlockLength = aData[0];
   if (subBlockLength == 0) {
     // We hit the block terminator.
     EndImageFrame();
-    return Transition::ToAfterYield(State::BLOCK_HEADER_AFTER_YIELD);
+    return Transition::To(State::BLOCK_HEADER, BLOCK_HEADER_LEN);
   }
 
   if (mGIFStruct.pixels_remaining == 0) {
     // We've already written to the entire image; we should've hit the block
     // terminator at this point. This image is corrupt, but we'll tolerate it.
 
     if (subBlockLength == GIF_TRAILER) {
       // This GIF is missing the block terminator for the final block; we'll put
--- a/image/decoders/nsGIFDecoder2.h
+++ b/image/decoders/nsGIFDecoder2.h
@@ -75,23 +75,23 @@ private:
   {
     FAILURE,
     SUCCESS,
     GIF_HEADER,
     SCREEN_DESCRIPTOR,
     GLOBAL_COLOR_TABLE,
     FINISHED_GLOBAL_COLOR_TABLE,
     BLOCK_HEADER,
-    BLOCK_HEADER_AFTER_YIELD,
     EXTENSION_HEADER,
     GRAPHIC_CONTROL_EXTENSION,
     APPLICATION_IDENTIFIER,
     NETSCAPE_EXTENSION_SUB_BLOCK,
     NETSCAPE_EXTENSION_DATA,
     IMAGE_DESCRIPTOR,
+    FINISH_IMAGE_DESCRIPTOR,
     LOCAL_COLOR_TABLE,
     FINISHED_LOCAL_COLOR_TABLE,
     IMAGE_DATA_BLOCK,
     IMAGE_DATA_SUB_BLOCK,
     LZW_DATA,
     SKIP_LZW_DATA,
     FINISHED_LZW_DATA,
     SKIP_SUB_BLOCKS,
@@ -105,16 +105,17 @@ private:
   LexerTransition<State> FinishedGlobalColorTable();
   LexerTransition<State> ReadBlockHeader(const char* aData);
   LexerTransition<State> ReadExtensionHeader(const char* aData);
   LexerTransition<State> ReadGraphicControlExtension(const char* aData);
   LexerTransition<State> ReadApplicationIdentifier(const char* aData);
   LexerTransition<State> ReadNetscapeExtensionSubBlock(const char* aData);
   LexerTransition<State> ReadNetscapeExtensionData(const char* aData);
   LexerTransition<State> ReadImageDescriptor(const char* aData);
+  LexerTransition<State> FinishImageDescriptor(const char* aData);
   LexerTransition<State> ReadLocalColorTable(const char* aData, size_t aLength);
   LexerTransition<State> FinishedLocalColorTable();
   LexerTransition<State> ReadImageDataBlock(const char* aData);
   LexerTransition<State> ReadImageDataSubBlock(const char* aData);
   LexerTransition<State> ReadLZWData(const char* aData, size_t aLength);
   LexerTransition<State> SkipSubBlocks(const char* aData);
 
   // The StreamingLexer used to manage input. The initial size of the buffer is