Bug 910881 - Do not call RasterImage::DoError off the main thread. r=jdm
authorSeth Fowler <seth@mozilla.com>
Thu, 12 Sep 2013 14:40:16 -0700
changeset 154921 8af98453b5119e118aeb3381feb536a8228d2dc8
parent 154920 e1cf000a73c8e59f4c95af4a2a331140eae1f007
child 154922 4b84bbad669dde2f0ef059f2e3bc641804e54254
push idunknown
push userunknown
push dateunknown
reviewersjdm
bugs910881
milestone26.0a1
Bug 910881 - Do not call RasterImage::DoError off the main thread. r=jdm
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -396,16 +396,17 @@ RasterImage::RasterImage(imgStatusTracke
   mDiscardable(false),
   mHasSourceData(false),
   mDecoded(false),
   mHasBeenDecoded(false),
   mAnimationFinished(false),
   mFinishing(false),
   mInUpdateImageContainer(false),
   mWantFullDecode(false),
+  mPendingError(false),
   mScaleRequest(nullptr)
 {
   // Set up the discard tracker node.
   mDiscardTrackerNode.img = this;
   Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(0);
 
   // Statistics
   num_containers++;
@@ -2817,35 +2818,65 @@ RasterImage::IsDecodeFinished()
 // Indempotent error flagging routine. If a decoder is open, shuts it down.
 void
 RasterImage::DoError()
 {
   // If we've flagged an error before, we have nothing to do
   if (mError)
     return;
 
+  // We can't safely handle errors off-main-thread, so dispatch a worker to do it.
+  if (!NS_IsMainThread()) {
+    HandleErrorWorker::DispatchIfNeeded(this);
+    return;
+  }
+
   // If we're mid-decode, shut down the decoder.
   if (mDecoder) {
     MutexAutoLock lock(mDecodingMutex);
     FinishedSomeDecoding(eShutdownIntent_Error);
   }
 
-  // Put the container in an error state
+  // Put the container in an error state.
   mError = true;
 
   if (mDecodeRequest) {
     mDecodeRequest->mStatusTracker->GetDecoderObserver()->OnError();
   } else {
     mStatusTracker->GetDecoderObserver()->OnError();
   }
 
   // Log our error
   LOG_CONTAINER_ERROR;
 }
 
+/* static */ void
+RasterImage::HandleErrorWorker::DispatchIfNeeded(RasterImage* aImage)
+{
+  if (!aImage->mPendingError) {
+    aImage->mPendingError = true;
+    nsRefPtr<HandleErrorWorker> worker = new HandleErrorWorker(aImage);
+    NS_DispatchToMainThread(worker);
+  }
+}
+
+RasterImage::HandleErrorWorker::HandleErrorWorker(RasterImage* aImage)
+  : mImage(aImage)
+{
+  MOZ_ASSERT(mImage, "Should have image");
+}
+
+NS_IMETHODIMP
+RasterImage::HandleErrorWorker::Run()
+{
+  mImage->DoError();
+
+  return NS_OK;
+}
+
 // 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,
                                 uint32_t       /* unused */,
@@ -3183,16 +3214,17 @@ RasterImage::DecodePool::DecodeJob::Run(
   // will enqueue another decode request when it's done.
   if (mImage->mDecoder && mImage->mDecoder->NeedsNewFrame()) {
     FrameNeededWorker::GetNewFrame(mImage);
   }
   // If we aren't yet finished decoding and we have more data in hand, add
   // this request to the back of the list.
   else if (mImage->mDecoder &&
            !mImage->mError &&
+           !mImage->mPendingError &&
            !mImage->IsDecodeFinished() &&
            bytesDecoded < mRequest->mBytesToDecode &&
            bytesDecoded > 0) {
     DecodePool::Singleton()->RequestDecode(mImage);
   } else {
     // Nothing more for us to do - let everyone know what happened.
     DecodeDoneWorker::NotifyFinishedSomeDecoding(mImage, mRequest);
   }
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -662,16 +662,21 @@ private: // data
   bool                       mFinishing:1;
 
   bool                       mInUpdateImageContainer:1;
 
   // Whether, once we are done doing a size decode, we should immediately kick
   // off a full decode.
   bool                       mWantFullDecode:1;
 
+  // Set when a decode worker detects an error off-main-thread. Once the error
+  // is handled on the main thread, mError is set, but mPendingError is used to
+  // stop decode work immediately.
+  bool                       mPendingError:1;
+
   // Decoding
   nsresult WantDecodedFrames();
   nsresult SyncDecode();
   nsresult InitDecoder(bool aDoSizeDecode, bool aIsSynchronous = false);
   nsresult WriteToDecoder(const char *aBuffer, uint32_t aCount);
   nsresult DecodeSomeData(uint32_t aMaxBytes);
   bool     IsDecodeFinished();
   TimeStamp mDrawStartTime;
@@ -694,18 +699,38 @@ private: // data
 
   // We hold on to a bare pointer to a ScaleRequest while it's outstanding so
   // we can mark it as stopped if necessary. The ScaleWorker/DrawWorker duo
   // will inform us when to let go of this pointer.
   ScaleRequest* mScaleRequest;
 
   nsresult ShutdownDecoder(eShutdownIntent aIntent);
 
+  // Error handling.
+  void DoError();
+
+  class HandleErrorWorker : public nsRunnable
+  {
+  public:
+    /**
+     * Called from decoder threads when DoError() is called, since errors can't
+     * be handled safely off-main-thread. Dispatches an event which reinvokes
+     * DoError on the main thread if there isn't one already pending.
+     */
+    static void DispatchIfNeeded(RasterImage* aImage);
+
+    NS_IMETHOD Run();
+
+  private:
+    HandleErrorWorker(RasterImage* aImage);
+
+    nsRefPtr<RasterImage> mImage;
+  };
+
   // Helpers
-  void DoError();
   bool CanDiscard();
   bool CanForciblyDiscard();
   bool DiscardingActive();
   bool StoringSourceData() const;
 
 protected:
   RasterImage(imgStatusTracker* aStatusTracker = nullptr, nsIURI* aURI = nullptr);