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 id25016
push userryanvm@gmail.com
push dateSat, 27 Jul 2013 02:25:56 +0000
treeherdermozilla-central@fb48c7d58b8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs887466
milestone25.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 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?