Back out bug 715308 and bug 721510 due to crashes and intermittent OSX reftest failures. r=joe
authorJustin Lebar <justin.lebar@gmail.com>
Thu, 02 Feb 2012 10:59:01 -0500
changeset 86074 350ba395c507610e74b50fe7d64569ba48a339dd
parent 86073 63f719f6681b2b97c3d7f86c0481bc4b7ac77f02
child 86075 36fb43ea8f0c75c8236cde01188608a3ab8bcafb
push id21990
push userbmo@edmorley.co.uk
push dateFri, 03 Feb 2012 16:46:11 +0000
treeherdermozilla-central@4da18c2e4910 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe
bugs715308, 721510
milestone13.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
Back out bug 715308 and bug 721510 due to crashes and intermittent OSX reftest failures. r=joe
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -66,17 +66,16 @@
 #include "nsIconDecoder.h"
 
 #include "gfxContext.h"
 
 #include "mozilla/Preferences.h"
 #include "mozilla/StdInt.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/TimeStamp.h"
-#include "mozilla/ClearOnShutdown.h"
 
 using namespace mozilla;
 using namespace mozilla::image;
 using namespace mozilla::layers;
 
 // a mask for flags that will affect the decoding
 #define DECODE_FLAGS_MASK (imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA | imgIContainer::FLAG_DECODE_NO_COLORSPACE_CONVERSION)
 #define DECODE_FLAGS_DEFAULT 0
@@ -172,18 +171,16 @@ DiscardingEnabled()
   }
 
   return enabled;
 }
 
 namespace mozilla {
 namespace image {
 
-/* static */ nsRefPtr<RasterImage::DecodeWorker> RasterImage::DecodeWorker::sSingleton;
-
 #ifndef DEBUG
 NS_IMPL_ISUPPORTS3(RasterImage, imgIContainer, nsIProperties,
                    nsISupportsWeakReference)
 #else
 NS_IMPL_ISUPPORTS4(RasterImage, imgIContainer, nsIProperties,
                    imgIContainerDebug, nsISupportsWeakReference)
 #endif
 
@@ -192,29 +189,30 @@ RasterImage::RasterImage(imgStatusTracke
   Image(aStatusTracker), // invoke superclass's constructor
   mSize(0,0),
   mFrameDecodeFlags(DECODE_FLAGS_DEFAULT),
   mAnim(nsnull),
   mLoopCount(-1),
   mObserver(nsnull),
   mLockCount(0),
   mDecoder(nsnull),
-  mDecodeRequest(this),
+  mWorker(nsnull),
   mBytesDecoded(0),
   mDecodeCount(0),
 #ifdef DEBUG
   mFramesNotified(0),
 #endif
   mHasSize(false),
   mDecodeOnDraw(false),
   mMultipart(false),
   mDiscardable(false),
   mHasSourceData(false),
   mDecoded(false),
   mHasBeenDecoded(false),
+  mWorkerPending(false),
   mInDecoder(false),
   mAnimationFinished(false)
 {
   // Set up the discard tracker node.
   mDiscardTrackerNode.curr = this;
   mDiscardTrackerNode.prev = mDiscardTrackerNode.next = nsnull;
   Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(0);
 
@@ -1490,19 +1488,21 @@ RasterImage::AddSourceData(const char *a
   else {
 
     // Store the data
     char *newElem = mSourceData.AppendElements(aBuffer, aCount);
     if (!newElem)
       return NS_ERROR_OUT_OF_MEMORY;
 
     // If there's a decoder open, that means we want to do more decoding.
-    // Wake up the worker.
-    if (mDecoder) {
-      DecodeWorker::Singleton()->RequestDecode(this);
+    // Wake up the worker if it's not up already
+    if (mDecoder && !mWorkerPending) {
+      NS_ABORT_IF_FALSE(mWorker, "We should have a worker here!");
+      rv = mWorker->Run();
+      CONTAINER_ENSURE_SUCCESS(rv);
     }
   }
 
   // Statistics
   total_source_bytes += aCount;
   if (mDiscardable)
     discardable_source_bytes += aCount;
   PR_LOG (gCompressedImageAccountingLog, PR_LOG_DEBUG,
@@ -1555,31 +1555,29 @@ RasterImage::SourceDataComplete()
   // If we're not storing any source data, then all the data was written
   // directly to the decoder in the AddSourceData() calls. This means we're
   // done, so we can shut down the decoder.
   if (!StoringSourceData()) {
     nsresult rv = ShutdownDecoder(eShutdownIntent_Done);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
-  // If there's a decoder open, synchronously decode the beginning of the image
-  // to check for errors and get the image's size.  (If we already have the
-  // image's size, this does nothing.)  Then kick off an async decode of the
-  // rest of the image.
-  if (mDecoder) {
-    nsresult rv = DecodeWorker::Singleton()->DecodeUntilSizeAvailable(this);
+  // If there's a decoder open, we need to wake up the worker if it's not
+  // already. This is so the worker can account for the fact that the source
+  // data is complete. For some decoders, DecodingComplete() is only called
+  // when the decoder is Close()-ed, and thus the SourceDataComplete() call
+  // is the only way we can transition to a 'decoded' state. Furthermore,
+  // it's always possible for any image type to have the data stream stop
+  // abruptly at any point, in which case we need to trigger an error.
+  if (mDecoder && !mWorkerPending) {
+    NS_ABORT_IF_FALSE(mWorker, "We should have a worker here!");
+    nsresult rv = mWorker->Run();
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
-  // If DecodeUntilSizeAvailable didn't finish the decode, let the decode worker
-  // finish decoding this image.
-  if (mDecoder) {
-    DecodeWorker::Singleton()->RequestDecode(this);
-  }
-
   // Free up any extra space in the backing buffer
   mSourceData.Compact();
 
   // Log header information
   if (PR_LOG_TEST(gCompressedImageAccountingLog, PR_LOG_DEBUG)) {
     char buf[9];
     get_header_str(buf, mSourceData.Elements(), mSourceData.Length());
     PR_LOG (gCompressedImageAccountingLog, PR_LOG_DEBUG,
@@ -2275,21 +2273,25 @@ RasterImage::InitDecoder(bool aDoSizeDec
   }
 
   // Initialize the decoder
   mDecoder->SetSizeDecode(aDoSizeDecode);
   mDecoder->SetDecodeFlags(mFrameDecodeFlags);
   mDecoder->Init();
   CONTAINER_ENSURE_SUCCESS(mDecoder->GetDecoderError());
 
+  // Create a decode worker
+  mWorker = new imgDecodeWorker(this);
+
   if (!aDoSizeDecode) {
     Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Subtract(mDecodeCount);
     mDecodeCount++;
     Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(mDecodeCount);
   }
+  CONTAINER_ENSURE_TRUE(mWorker, NS_ERROR_OUT_OF_MEMORY);
 
   return NS_OK;
 }
 
 // Flushes, closes, and nulls-out a decoder. Cleans up any related decoding
 // state. It is an error to call this function when there is no initialized
 // decoder.
 // 
@@ -2315,26 +2317,26 @@ RasterImage::ShutdownDecoder(eShutdownIn
   // error routine might re-invoke ShutdownDecoder)
   nsRefPtr<Decoder> decoder = mDecoder;
   mDecoder = nsnull;
 
   mInDecoder = true;
   decoder->Finish();
   mInDecoder = false;
 
-  // Kill off our decode request, if it's pending.  (If not, this call is
-  // harmless.)
-  DecodeWorker::Singleton()->StopDecoding(this);
-
   nsresult decoderStatus = decoder->GetDecoderError();
   if (NS_FAILED(decoderStatus)) {
     DoError();
     return decoderStatus;
   }
 
+  // Kill off the worker
+  mWorker = nsnull;
+  mWorkerPending = false;
+
   // We just shut down the decoder. If we didn't get what we want, but expected
   // to, flag an error
   bool failed = false;
   if (wasSizeDecode && !mHasSize)
     failed = true;
   if (!wasSizeDecode && !mDecoded)
     failed = true;
   if ((aIntent == eShutdownIntent_Done) && failed) {
@@ -2458,30 +2460,32 @@ RasterImage::RequestDecode()
 
   // If we don't have a decoder, create one 
   if (!mDecoder) {
     NS_ABORT_IF_FALSE(mFrames.IsEmpty(), "Trying to decode to non-empty frame-array");
     rv = InitDecoder(/* aDoSizeDecode = */ false);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
+  // If we already have a pending worker, we're done
+  if (mWorkerPending)
+    return NS_OK;
+
   // If we've read all the data we have, we're done
   if (mBytesDecoded == mSourceData.Length())
     return NS_OK;
 
   // If it's a smallish image, it's not worth it to do things async
   if (!mDecoded && !mInDecoder && mHasSourceData && (mSourceData.Length() < gMaxBytesForSyncDecode))
     return SyncDecode();
 
   // If we get this far, dispatch the worker. We do this instead of starting
   // any immediate decoding to guarantee that all our decode notifications are
   // dispatched asynchronously, and to ensure we stay responsive.
-  DecodeWorker::Singleton()->RequestDecode(this);
-
-  return NS_OK;
+  return mWorker->Dispatch();
 }
 
 // Synchronously decodes as much data as possible
 nsresult
 RasterImage::SyncDecode()
 {
   nsresult rv;
 
@@ -2578,20 +2582,16 @@ RasterImage::Draw(gfxContext *aContext,
     ForceDiscard();
 
     mFrameDecodeFlags = DECODE_FLAGS_DEFAULT;
   }
 
   // We use !mDecoded && mHasSourceData to mean discarded.
   if (!mDecoded && mHasSourceData) {
       mDrawStartTime = TimeStamp::Now();
-
-      // We're drawing this image, so indicate that we should decode it as soon
-      // as possible.
-      DecodeWorker::Singleton()->MarkAsASAP(this);
   }
 
   // If a synchronous draw is requested, flush anything that might be sitting around
   if (aFlags & FLAG_SYNC_DECODE) {
     nsresult rv = SyncDecode();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -2759,17 +2759,153 @@ RasterImage::DoError()
 
   // Put the container in an error state
   mError = true;
 
   // Log our error
   LOG_CONTAINER_ERROR;
 }
 
-// nsIInputStream callback to copy the incoming image data directly to the
+// Decodes some data, then re-posts itself to the end of the event queue if
+// there's more processing to be done
+NS_IMETHODIMP
+imgDecodeWorker::Run()
+{
+  nsresult rv;
+
+  // If we shutdown the decoder in this function, we could lose ourselves
+  nsCOMPtr<nsIRunnable> kungFuDeathGrip(this);
+
+  // The container holds a strong reference to us. Cycles are bad.
+  nsCOMPtr<imgIContainer> iContainer(do_QueryReferent(mContainer));
+  if (!iContainer)
+    return NS_OK;
+  RasterImage* image = static_cast<RasterImage*>(iContainer.get());
+
+  NS_ABORT_IF_FALSE(image->mInitialized,
+                    "Worker active for uninitialized container!");
+
+  // If we were pending, we're not anymore
+  image->mWorkerPending = false;
+
+  // If an error is flagged, it probably happened while we were waiting
+  // in the event queue. Bail early, but no need to bother the run queue
+  // by returning an error.
+  if (image->mError)
+    return NS_OK;
+
+  // If we don't have a decoder, we must have finished already (for example,
+  // a synchronous decode request came while the worker was pending).
+  if (!image->mDecoder)
+    return NS_OK;
+
+  nsRefPtr<Decoder> decoderKungFuDeathGrip = image->mDecoder;
+
+  // Size decodes are cheap and we more or less want them to be
+  // synchronous. Write all the data in that case, otherwise write a
+  // chunk
+  PRUint32 maxBytes = image->mDecoder->IsSizeDecode()
+    ? image->mSourceData.Length() : gDecodeBytesAtATime;
+
+  // Loop control
+  bool haveMoreData = true;
+  PRInt32 chunkCount = 0;
+  TimeStamp start = TimeStamp::Now();
+  TimeStamp deadline = start + TimeDuration::FromMilliseconds(gMaxMSBeforeYield);
+
+  // We keep decoding chunks until one of three possible events occur:
+  // 1) We don't have any data left to decode
+  // 2) The decode completes
+  // 3) We hit the deadline and need to yield to keep the UI snappy
+  while (haveMoreData && !image->IsDecodeFinished() &&
+         (TimeStamp::Now() < deadline)) {
+
+    // Decode a chunk of data
+    chunkCount++;
+    rv = image->DecodeSomeData(maxBytes);
+    if (NS_FAILED(rv)) {
+      image->DoError();
+      return rv;
+    }
+
+    // Figure out if we still have more data
+    haveMoreData =
+      image->mSourceData.Length() > image->mBytesDecoded;
+  }
+
+  TimeDuration decodeLatency = TimeStamp::Now() - start;
+  if (chunkCount && !image->mDecoder->IsSizeDecode()) {
+      Telemetry::Accumulate(Telemetry::IMAGE_DECODE_LATENCY, PRInt32(decodeLatency.ToMicroseconds()));
+      Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, chunkCount);
+  }
+  // accumulate the total decode time
+  mDecodeTime += decodeLatency;
+
+  // Flush invalidations _after_ we've written everything we're going to.
+  // Furthermore, if we have all of the data, we don't want to do progressive
+  // display at all. In that case, let Decoder::PostFrameStop() do the
+  // flush once the whole frame is ready.
+  if (!image->mHasSourceData) {
+    image->mInDecoder = true;
+    image->mDecoder->FlushInvalidations();
+    image->mInDecoder = false;
+  }
+
+  // If the decode finished, shutdown the decoder
+  if (image->mDecoder && image->IsDecodeFinished()) {
+
+    if (!image->mDecoder->IsSizeDecode()) {
+        Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME, PRInt32(mDecodeTime.ToMicroseconds()));
+
+        // We only record the speed for some decoders. The rest have SpeedHistogram return HistogramCount.
+        Telemetry::ID id = image->mDecoder->SpeedHistogram();
+        if (id < Telemetry::HistogramCount) {
+            PRInt32 KBps = PRInt32((image->mBytesDecoded/1024.0)/mDecodeTime.ToSeconds());
+            Telemetry::Accumulate(id, KBps);
+        }
+    }
+
+    rv = image->ShutdownDecoder(RasterImage::eShutdownIntent_Done);
+    if (NS_FAILED(rv)) {
+      image->DoError();
+      return rv;
+    }
+  }
+
+  // If Conditions 1 & 2 are still true, then the only reason we bailed was
+  // because we hit the deadline. Repost ourselves to the end of the event
+  // queue.
+  if (image->mDecoder && !image->IsDecodeFinished() && haveMoreData)
+    return this->Dispatch();
+
+  // Otherwise, return success
+  return NS_OK;
+}
+
+// Queues the worker up at the end of the event queue
+NS_METHOD imgDecodeWorker::Dispatch()
+{
+  // The container holds a strong reference to us. Cycles are bad.
+  nsCOMPtr<imgIContainer> iContainer(do_QueryReferent(mContainer));
+  if (!iContainer)
+    return NS_OK;
+  RasterImage* image = static_cast<RasterImage*>(iContainer.get());
+
+  // We should not be called if there's already a pending worker
+  NS_ABORT_IF_FALSE(!image->mWorkerPending,
+                    "Trying to queue up worker with one already pending!");
+
+  // Flag that we're pending
+  image->mWorkerPending = true;
+
+  // Dispatch
+  return NS_DispatchToCurrentThread(this);
+}
+
+// nsIInputStream callback to copy the incoming image data directly to the 
 // RasterImage without processing. The RasterImage is passed as the closure.
 // Always reads everything it gets, even if the data is erroneous.
 NS_METHOD
 RasterImage::WriteToRasterImage(nsIInputStream* /* unused */,
                                 void*          aClosure,
                                 const char*    aFromRawSegment,
                                 PRUint32       /* unused */,
                                 PRUint32       aCount,
@@ -2796,265 +2932,24 @@ RasterImage::WriteToRasterImage(nsIInput
 
 bool
 RasterImage::ShouldAnimate()
 {
   return Image::ShouldAnimate() && mFrames.Length() >= 2 &&
          !mAnimationFinished;
 }
 
+//******************************************************************************
 /* readonly attribute PRUint32 framesNotified; */
 #ifdef DEBUG
 NS_IMETHODIMP
 RasterImage::GetFramesNotified(PRUint32 *aFramesNotified)
 {
   NS_ENSURE_ARG_POINTER(aFramesNotified);
 
   *aFramesNotified = mFramesNotified;
 
   return NS_OK;
 }
 #endif
 
-/* static */ RasterImage::DecodeWorker*
-RasterImage::DecodeWorker::Singleton()
-{
-  if (!sSingleton) {
-    sSingleton = new DecodeWorker();
-    ClearOnShutdown(&sSingleton);
-  }
-
-  return sSingleton;
-}
-
-void
-RasterImage::DecodeWorker::MarkAsASAP(RasterImage* aImg)
-{
-  DecodeRequest* request = &aImg->mDecodeRequest;
-
-  // If we're already an ASAP request, there's nothing to do here.
-  if (request->mIsASAP) {
-    return;
-  }
-
-  request->mIsASAP = true;
-
-  if (request->isInList()) {
-    // If the decode request is in a list, it must be in the normal decode
-    // requests list -- if it had been in the ASAP list, then mIsASAP would
-    // have been true above.  Move the request to the ASAP list.
-    request->remove();
-    mASAPDecodeRequests.insertBack(request);
-
-    // Since request is in a list, one of the decode worker's lists is
-    // non-empty, so the worker should be pending in the event loop.
-    //
-    // (Note that this invariant only holds while we are not in Run(), because
-    // DecodeSomeOfImage adds requests to the decode worker using
-    // AddDecodeRequest, not RequestDecode, and AddDecodeRequest does not call
-    // EnsurePendingInEventLoop.  Therefore, it is an error to call MarkAsASAP
-    // from within DecodeWorker::Run.)
-    MOZ_ASSERT(mPendingInEventLoop);
-  }
-}
-
-void
-RasterImage::DecodeWorker::AddDecodeRequest(DecodeRequest* aRequest)
-{
-  if (aRequest->isInList()) {
-    // The image is already in our list of images to decode, so we don't have
-    // to do anything here.
-    return;
-  }
-
-  if (aRequest->mIsASAP) {
-    mASAPDecodeRequests.insertBack(aRequest);
-  } else {
-    mNormalDecodeRequests.insertBack(aRequest);
-  }
-}
-
-void
-RasterImage::DecodeWorker::RequestDecode(RasterImage* aImg)
-{
-  AddDecodeRequest(&aImg->mDecodeRequest);
-  EnsurePendingInEventLoop();
-}
-
-void
-RasterImage::DecodeWorker::EnsurePendingInEventLoop()
-{
-  if (!mPendingInEventLoop) {
-    mPendingInEventLoop = true;
-    NS_DispatchToCurrentThread(this);
-  }
-}
-
-void
-RasterImage::DecodeWorker::StopDecoding(RasterImage* aImg)
-{
-  DecodeRequest* request = &aImg->mDecodeRequest;
-  if (request->isInList()) {
-    request->remove();
-  }
-  request->mDecodeTime = TimeDuration(0);
-  request->mIsASAP = false;
-}
-
-NS_IMETHODIMP
-RasterImage::DecodeWorker::Run()
-{
-  // We just got called back by the event loop; therefore, we're no longer
-  // pending.
-  mPendingInEventLoop = false;
-
-  TimeStamp eventStart = TimeStamp::Now();
-
-  // Now decode until we either run out of time or run out of images.
-  do {
-    // Try to get an ASAP request to handle.  If there isn't one, try to get a
-    // normal request.  If no normal request is pending either, then we're done
-    // here.
-    DecodeRequest* request = mASAPDecodeRequests.popFirst();
-    if (!request)
-      request = mNormalDecodeRequests.popFirst();
-    if (!request)
-      break;
-
-    RasterImage *image = request->mImage;
-    DecodeSomeOfImage(image);
-
-    // If we aren't yet finished decoding and we have more data in hand, add
-    // this request to the back of the list.
-    if (image->mDecoder &&
-        !image->mError &&
-        !image->IsDecodeFinished() &&
-        image->mSourceData.Length() > image->mBytesDecoded) {
-      AddDecodeRequest(request);
-    }
-
-  } while ((TimeStamp::Now() - eventStart).ToMilliseconds() <= gMaxMSBeforeYield);
-
-  // If decode requests are pending, re-post ourself to the event loop.
-  if (!mASAPDecodeRequests.isEmpty() || !mNormalDecodeRequests.isEmpty()) {
-    EnsurePendingInEventLoop();
-  }
-
-  Telemetry::Accumulate(Telemetry::IMAGE_DECODE_LATENCY,
-                        PRUint32((TimeStamp::Now() - eventStart).ToMilliseconds()));
-
-  return NS_OK;
-}
-
-nsresult
-RasterImage::DecodeWorker::DecodeUntilSizeAvailable(RasterImage* aImg)
-{
-  return DecodeSomeOfImage(aImg, DECODE_TYPE_UNTIL_SIZE);
-}
-
-nsresult
-RasterImage::DecodeWorker::DecodeSomeOfImage(
-  RasterImage* aImg,
-  DecodeType aDecodeType /* = DECODE_TYPE_NORMAL */)
-{
-  NS_ABORT_IF_FALSE(aImg->mInitialized,
-                    "Worker active for uninitialized container!");
-
-  if (aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize)
-    return NS_OK;
-
-  // If an error is flagged, it probably happened while we were waiting
-  // in the event queue.
-  if (aImg->mError)
-    return NS_OK;
-
-  // If mDecoded or we don't have a decoder, we must have finished already (for
-  // example, a synchronous decode request came while the worker was pending).
-  if (!aImg->mDecoder || aImg->mDecoded)
-    return NS_OK;
-
-  nsRefPtr<Decoder> decoderKungFuDeathGrip = aImg->mDecoder;
-
-  PRUint32 maxBytes;
-  if (aImg->mDecoder->IsSizeDecode()) {
-    // Decode all available data if we're a size decode; they're cheap, and we
-    // want them to be more or less synchronous.
-    maxBytes = aImg->mSourceData.Length();
-  } else {
-    // We're only guaranteed to decode this many bytes, so in particular,
-    // gDecodeBytesAtATime should be set high enough for us to read the size
-    // from most images.
-    maxBytes = gDecodeBytesAtATime;
-  }
-
-  PRInt32 chunkCount = 0;
-  TimeStamp start = TimeStamp::Now();
-  TimeStamp deadline = start + TimeDuration::FromMilliseconds(gMaxMSBeforeYield);
-
-  // Decode some chunks of data.
-  do {
-    chunkCount++;
-    nsresult rv = aImg->DecodeSomeData(maxBytes);
-    if (NS_FAILED(rv)) {
-      aImg->DoError();
-      return rv;
-    }
-
-    // We keep decoding chunks until either:
-    //  * we're an UNTIL_SIZE decode and we get the size,
-    //  * we don't have any data left to decode,
-    //  * the decode completes, or
-    //  * we run out of time.
-
-    if (aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize)
-      break;
-  }
-  while (aImg->mSourceData.Length() > aImg->mBytesDecoded &&
-         !aImg->IsDecodeFinished() &&
-         TimeStamp::Now() < deadline);
-
-  aImg->mDecodeRequest.mDecodeTime += (TimeStamp::Now() - start);
-
-  if (chunkCount && !aImg->mDecoder->IsSizeDecode()) {
-    Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, chunkCount);
-  }
-
-  // Flush invalidations _after_ we've written everything we're going to.
-  // Furthermore, if we have all of the data, we don't want to do progressive
-  // display at all. In that case, let Decoder::PostFrameStop() do the
-  // flush once the whole frame is ready.
-  if (!aImg->mHasSourceData) {
-    aImg->mInDecoder = true;
-    aImg->mDecoder->FlushInvalidations();
-    aImg->mInDecoder = false;
-  }
-
-  // If the decode finished, shut down the decoder.
-  if (aImg->mDecoder && aImg->IsDecodeFinished()) {
-
-    // Do some telemetry if this isn't a size decode.
-    DecodeRequest* request = &aImg->mDecodeRequest;
-    if (!aImg->mDecoder->IsSizeDecode()) {
-      Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME,
-                            PRInt32(request->mDecodeTime.ToMicroseconds()));
-
-      // We record the speed for only some decoders. The rest have
-      // SpeedHistogram return HistogramCount.
-      Telemetry::ID id = aImg->mDecoder->SpeedHistogram();
-      if (id < Telemetry::HistogramCount) {
-          PRInt32 KBps = PRInt32(request->mImage->mBytesDecoded /
-                                 (1024 * request->mDecodeTime.ToSeconds()));
-          Telemetry::Accumulate(id, KBps);
-      }
-    }
-
-    nsresult rv = aImg->ShutdownDecoder(RasterImage::eShutdownIntent_Done);
-    if (NS_FAILED(rv)) {
-      aImg->DoError();
-      return rv;
-    }
-  }
-
-  return NS_OK;
-}
-
 } // namespace image
 } // namespace mozilla
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -61,17 +61,16 @@
 #include "nsITimer.h"
 #include "nsWeakReference.h"
 #include "nsTArray.h"
 #include "imgFrame.h"
 #include "nsThreadUtils.h"
 #include "DiscardTracker.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Telemetry.h"
-#include "mozilla/LinkedList.h"
 #ifdef DEBUG
   #include "imgIContainerDebug.h"
 #endif
 
 class imgIDecoder;
 class imgIContainerObserver;
 class nsIInputStream;
 
@@ -160,16 +159,17 @@ class nsIInputStream;
 
 namespace mozilla {
 namespace layers {
 class LayerManager;
 class ImageContainer;
 }
 namespace image {
 
+class imgDecodeWorker;
 class Decoder;
 
 class RasterImage : public Image
                   , public nsIProperties
                   , public nsSupportsWeakReference
 #ifdef DEBUG
                   , public imgIContainerDebug
 #endif
@@ -372,133 +372,16 @@ private:
     Anim() :
       firstFrameRefreshArea(),
       currentAnimationFrameIndex(0),
       lastCompositedFrameIndex(-1) {}
     ~Anim() {}
   };
 
   /**
-   * DecodeWorker keeps a linked list of DecodeRequests to keep track of the
-   * images it needs to decode.
-   *
-   * Each RasterImage has a single DecodeRequest member.
-   */
-  struct DecodeRequest : public LinkedListElement<DecodeRequest>
-  {
-    DecodeRequest(RasterImage* aImage)
-      : mImage(aImage)
-      , mIsASAP(false)
-    {
-    }
-
-    RasterImage* const mImage;
-
-    /* Keeps track of how much time we've burned decoding this particular decode
-     * request. */
-    TimeDuration mDecodeTime;
-
-    /* True if we need to handle this decode as soon as possible. */
-    bool mIsASAP;
-  };
-
-  /*
-   * DecodeWorker is a singleton class we use when decoding large images.
-   *
-   * When we wish to decode an image larger than
-   * image.mem.max_bytes_for_sync_decode, we call DecodeWorker::RequestDecode()
-   * for the image.  This adds the image to a queue of pending requests and posts
-   * the DecodeWorker singleton to the event queue, if it's not already pending
-   * there.
-   *
-   * When the DecodeWorker is run from the event queue, it decodes the image (and
-   * all others it's managing) in chunks, periodically yielding control back to
-   * the event loop.
-   *
-   * An image being decoded may have one of two priorities: normal or ASAP.  ASAP
-   * images are always decoded before normal images.  (We currently give ASAP
-   * priority to images which appear onscreen but are not yet decoded.)
-   */
-  class DecodeWorker : public nsRunnable
-  {
-  public:
-    static DecodeWorker* Singleton();
-
-    /**
-     * Ask the DecodeWorker to asynchronously decode this image.
-     */
-    void RequestDecode(RasterImage* aImg);
-
-    /**
-     * Give this image ASAP priority; it will be decoded before all non-ASAP
-     * images.  You can call MarkAsASAP before or after you call RequestDecode
-     * for the image, but if you MarkAsASAP before you call RequestDecode, you
-     * still need to call RequestDecode.
-     *
-     * StopDecoding() resets the image's ASAP flag.
-     */
-    void MarkAsASAP(RasterImage* aImg);
-
-    /**
-     * Ask the DecodeWorker to stop decoding this image.  Internally, we also
-     * call this function when we finish decoding an image.
-     *
-     * Since the DecodeWorker keeps raw pointers to RasterImages, make sure you
-     * call this before a RasterImage is destroyed!
-     */
-    void StopDecoding(RasterImage* aImg);
-
-    /**
-     * Synchronously decode the beginning of the image until we run out of
-     * bytes or we get the image's size.  Note that this done on a best-effort
-     * basis; if the size is burried too deep in the image, we'll give up.
-     *
-     * @return NS_ERROR if an error is encountered, and NS_OK otherwise.  (Note
-     *         that we return NS_OK even when the size was not found.)
-     */
-    nsresult DecodeUntilSizeAvailable(RasterImage* aImg);
-
-    NS_IMETHOD Run();
-
-  private: /* statics */
-    static nsRefPtr<DecodeWorker> sSingleton;
-
-  private: /* methods */
-    DecodeWorker()
-      : mPendingInEventLoop(false)
-    {}
-
-    /* Post ourselves to the event loop if we're not currently pending. */
-    void EnsurePendingInEventLoop();
-
-    /* Add the given request to the appropriate list of decode requests, but
-     * don't ensure that we're pending in the event loop. */
-    void AddDecodeRequest(DecodeRequest* aRequest);
-
-    enum DecodeType {
-      DECODE_TYPE_NORMAL,
-      DECODE_TYPE_UNTIL_SIZE
-    };
-
-    /* Decode some chunks of the given image.  If aDecodeType is UNTIL_SIZE,
-     * decode until we have the image's size, then stop. */
-    nsresult DecodeSomeOfImage(RasterImage* aImg,
-                               DecodeType aDecodeType = DECODE_TYPE_NORMAL);
-
-  private: /* members */
-
-    LinkedList<DecodeRequest> mASAPDecodeRequests;
-    LinkedList<DecodeRequest> mNormalDecodeRequests;
-
-    /* True if we've posted ourselves to the event loop and expect Run() to
-     * be called sometime in the future. */
-    bool mPendingInEventLoop;
-  };
-
-  /**
    * Advances the animation. Typically, this will advance a single frame, but it
    * may advance multiple frames. This may happen if we have infrequently
    * "ticking" refresh drivers (e.g. in background tabs), or extremely short-
    * lived animation frames.
    *
    * @param aTime the time that the animation should advance to. This will
    *              typically be <= TimeStamp::Now().
    *
@@ -636,21 +519,22 @@ private: // data
   PRUint32                   mLockCount;
   DiscardTrackerNode         mDiscardTrackerNode;
 
   // Source data members
   FallibleTArray<char>       mSourceData;
   nsCString                  mSourceDataMimeType;
   nsCString                  mURIString;
 
+  friend class imgDecodeWorker;
   friend class DiscardTracker;
 
   // Decoder and friends
   nsRefPtr<Decoder>              mDecoder;
-  DecodeRequest                  mDecodeRequest;
+  nsRefPtr<imgDecodeWorker>      mWorker;
   PRUint32                       mBytesDecoded;
 
   // How many times we've decoded this image.
   // This is currently only used for statistics
   PRInt32                        mDecodeCount;
 
   // Cached value for GetImageContainer.
   nsRefPtr<mozilla::layers::ImageContainer> mImageContainer;
@@ -665,16 +549,18 @@ private: // data
   bool                       mMultipart:1;     // Multipart?
   bool                       mDiscardable:1;   // Is container discardable?
   bool                       mHasSourceData:1; // Do we have source data?
 
   // Do we have the frames in decoded form?
   bool                       mDecoded:1;
   bool                       mHasBeenDecoded:1;
 
+  // Helpers for decoder
+  bool                       mWorkerPending:1;
   bool                       mInDecoder:1;
 
   // Whether the animation can stop, due to running out
   // of frames, or no more owning request
   bool                       mAnimationFinished:1;
 
   // Decoding
   nsresult WantDecodedFrames();
@@ -700,16 +586,39 @@ private: // data
   bool CanForciblyDiscard();
   bool DiscardingActive();
   bool StoringSourceData();
 
 protected:
   bool ShouldAnimate();
 };
 
+// XXXdholbert These helper classes should move to be inside the
+// scope of the RasterImage class.
+// Decoding Helper Class
+//
+// We use this class to mimic the interactivity benefits of threading
+// in a single-threaded event loop. We want to progressively decode
+// and keep a responsive UI while we're at it, so we have a runnable
+// class that does a bit of decoding, and then "yields" by dispatching
+// itself to the end of the event queue.
+class imgDecodeWorker : public nsRunnable
+{
+  public:
+    imgDecodeWorker(imgIContainer* aContainer) {
+      mContainer = do_GetWeakReference(aContainer);
+    }
+    NS_IMETHOD Run();
+    NS_METHOD  Dispatch();
+
+  private:
+    nsWeakPtr mContainer;
+    TimeDuration mDecodeTime; // the default constructor initializes to 0
+};
+
 // Asynchronous Decode Requestor
 //
 // We use this class when someone calls requestDecode() from within a decode
 // notification. Since requestDecode() involves modifying the decoder's state
 // (for example, possibly shutting down a header-only decode and starting a
 // full decode), we don't want to do this from inside a decoder.
 class imgDecodeRequestor : public nsRunnable
 {