Bug 887466 - Rearrange SyncDecode so that we never try to finish a size decode with the decode lock held. r=seth
authorJoe Drew <joe@drew.ca>
Fri, 26 Jul 2013 14:57:41 -0400
changeset 140197 ddbd70f50d73736d5bfa5845600af86c290171a2
parent 140196 a04093b3aaa4bcffa8efae07d9f854bc6c74c103
child 140198 7b60ddc950079d153aa29fc7d2f16d88bbb62d1c
push idunknown
push userunknown
push dateunknown
reviewersseth
bugs887466
milestone25.0a1
Bug 887466 - Rearrange SyncDecode so that we never try to finish a size decode with the decode lock held. r=seth
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -2320,49 +2320,49 @@ RasterImage::RequestDecodeCore(RequestDe
 
   return NS_OK;
 }
 
 // Synchronously decodes as much data as possible
 nsresult
 RasterImage::SyncDecode()
 {
-  MutexAutoLock imgLock(mDecodingMutex);
-
-  if (mDecodeRequest) {
-    // If the image is waiting for decode work to be notified, go ahead and do that.
-    if (mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE) {
-      nsresult rv = FinishedSomeDecoding();
-      CONTAINER_ENSURE_SUCCESS(rv);
-    }
-  }
-
-  nsresult rv;
-
   PROFILER_LABEL_PRINTF("RasterImage", "SyncDecode", "%s", GetURIString().get());;
 
-  // We really have no good way of forcing a synchronous decode if we're being
-  // called in a re-entrant manner (ie, from an event listener fired by a
-  // decoder), because the decoding machinery is already tied up. We thus explicitly
-  // disallow this type of call in the API, and check for it in API methods.
-  NS_ABORT_IF_FALSE(!mInDecoder, "Yikes, forcing sync in reentrant call!");
-
   // If we have a size decoder open, make sure we get the size
   if (mDecoder && mDecoder->IsSizeDecode()) {
     nsresult rv = DecodePool::Singleton()->DecodeUntilSizeAvailable(this);
     CONTAINER_ENSURE_SUCCESS(rv);
 
     // If we didn't get the size out of the image, we won't until we get more
     // data, so signal that we want a full decode and give up for now.
     if (!mHasSize) {
       mWantFullDecode = true;
       return NS_ERROR_NOT_AVAILABLE;
     }
   }
 
+  MutexAutoLock imgLock(mDecodingMutex);
+
+  // We really have no good way of forcing a synchronous decode if we're being
+  // called in a re-entrant manner (ie, from an event listener fired by a
+  // decoder), because the decoding machinery is already tied up. We thus explicitly
+  // disallow this type of call in the API, and check for it in API methods.
+  NS_ABORT_IF_FALSE(!mInDecoder, "Yikes, forcing sync in reentrant call!");
+
+  if (mDecodeRequest) {
+    // If the image is waiting for decode work to be notified, go ahead and do that.
+    if (mDecodeRequest->mRequestStatus == DecodeRequest::REQUEST_WORK_DONE) {
+      nsresult rv = FinishedSomeDecoding();
+      CONTAINER_ENSURE_SUCCESS(rv);
+    }
+  }
+
+  nsresult rv;
+
   // If we're decoded already, or decoding until the size was available
   // finished us as a side-effect, no worries
   if (mDecoded)
     return NS_OK;
 
   // If we don't have any bytes to flush to the decoder, we can't do anything.
   // mBytesDecoded can be bigger than mSourceData.Length() if we're not storing
   // the source data.
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -637,19 +637,19 @@ private: // data
   mozilla::Mutex             mDecodingMutex;
 
   FallibleTArray<char>       mSourceData;
 
   // Decoder and friends
   nsRefPtr<Decoder>          mDecoder;
   nsRefPtr<DecodeRequest>    mDecodeRequest;
   uint32_t                   mBytesDecoded;
-  // END LOCKED MEMBER VARIABLES
 
   bool                       mInDecoder;
+  // END LOCKED MEMBER VARIABLES
 
   // Boolean flags (clustered together to conserve space):
   bool                       mHasSize:1;       // Has SetSize() been called?
   bool                       mDecodeOnDraw:1;  // Decoding on draw?
   bool                       mMultipart:1;     // Multipart?
   bool                       mDiscardable:1;   // Is container discardable?
   bool                       mHasSourceData:1; // Do we have source data?