Bug 1139225 (Part 3) - Make OnDataAvailable threadsafe. r=tn
authorSeth Fowler <seth@mozilla.com>
Mon, 23 Mar 2015 19:37:45 -0700
changeset 264072 3f5eae4fac0f4d72db88c458c8a620384640ef6b
parent 264071 fcaed7860ea4b1d43229d21921f2b87f58986369
child 264073 bb86ca5e5552a56e2186c177ccaeba6e0d2daace
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1139225
milestone39.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 1139225 (Part 3) - Make OnDataAvailable threadsafe. r=tn
image/src/imgRequest.cpp
image/src/imgRequest.h
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -60,26 +60,27 @@ NS_IMPL_ISUPPORTS(imgRequest,
                   nsIStreamListener, nsIRequestObserver,
                   nsIThreadRetargetableStreamListener,
                   nsIChannelEventSink,
                   nsIInterfaceRequestor,
                   nsIAsyncVerifyRedirectCallback)
 
 imgRequest::imgRequest(imgLoader* aLoader)
  : mLoader(aLoader)
- , mProgressTracker(new ProgressTracker())
  , mValidator(nullptr)
  , mInnerWindowId(0)
  , mCORSMode(imgIRequest::CORS_NONE)
  , mReferrerPolicy(mozilla::net::RP_Default)
  , mImageErrorCode(NS_OK)
- , mDecodeRequested(false)
+ , mMutex("imgRequest")
+ , mProgressTracker(new ProgressTracker())
  , mIsMultiPartChannel(false)
  , mGotData(false)
  , mIsInCache(false)
+ , mDecodeRequested(false)
  , mNewPartPending(false)
 { }
 
 imgRequest::~imgRequest()
 {
   if (mLoader) {
     mLoader->RemoveFromUncachedImages(this);
   }
@@ -146,16 +147,18 @@ nsresult imgRequest::Init(nsIURI *aURI,
 
 void imgRequest::ClearLoader() {
   mLoader = nullptr;
 }
 
 already_AddRefed<ProgressTracker>
 imgRequest::GetProgressTracker()
 {
+  MutexAutoLock lock(mMutex);
+
   if (mImage) {
     MOZ_ASSERT(!mProgressTracker,
                "Should have given mProgressTracker to mImage");
     return mImage->GetProgressTracker();
   } else {
     MOZ_ASSERT(mProgressTracker,
                "Should have mProgressTracker until we create mImage");
     nsRefPtr<ProgressTracker> progressTracker = mProgressTracker;
@@ -358,16 +361,30 @@ void imgRequest::EvictFromCache()
 // Helper-method used by EvictFromCache()
 void imgRequest::ContinueEvict()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   RemoveFromCache();
 }
 
+void
+imgRequest::RequestDecode()
+{
+  MutexAutoLock lock(mMutex);
+  mDecodeRequested = true;
+}
+
+bool
+imgRequest::IsDecodeRequested() const
+{
+  MutexAutoLock lock(mMutex);
+  return mDecodeRequested;
+}
+
 nsresult imgRequest::GetURI(ImageURL **aURI)
 {
   MOZ_ASSERT(aURI);
 
   LOG_FUNC(GetImgLog(), "imgRequest::GetURI");
 
   if (mURI) {
     *aURI = mURI;
@@ -403,21 +420,29 @@ nsresult imgRequest::GetSecurityInfo(nsI
   LOG_FUNC(GetImgLog(), "imgRequest::GetSecurityInfo");
 
   // Missing security info means this is not a security load
   // i.e. it is not an error when security info is missing
   NS_IF_ADDREF(*aSecurityInfo = mSecurityInfo);
   return NS_OK;
 }
 
-void imgRequest::RemoveFromCache()
+void
+imgRequest::RemoveFromCache()
 {
   LOG_SCOPE(GetImgLog(), "imgRequest::RemoveFromCache");
 
-  if (mIsInCache && mLoader) {
+  bool isInCache = false;
+
+  {
+    MutexAutoLock lock(mMutex);
+    isInCache = mIsInCache;
+  }
+
+  if (isInCache && mLoader) {
     // mCacheEntry is nulled out when we have no more observers.
     if (mCacheEntry) {
       mLoader->RemoveFromCache(mCacheEntry);
     } else {
       mLoader->RemoveFromCache(mURI);
     }
   }
 
@@ -425,16 +450,24 @@ void imgRequest::RemoveFromCache()
 }
 
 bool imgRequest::HasConsumers()
 {
   nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
   return progressTracker && progressTracker->ObserverCount() > 0;
 }
 
+already_AddRefed<Image>
+imgRequest::GetImage() const
+{
+  MutexAutoLock lock(mMutex);
+  nsRefPtr<Image> image = mImage;
+  return image.forget();
+}
+
 int32_t imgRequest::Priority() const
 {
   int32_t priority = nsISupportsPriority::PRIORITY_NORMAL;
   nsCOMPtr<nsISupportsPriority> p = do_QueryInterface(mRequest);
   if (p)
     p->GetPriority(&priority);
   return priority;
 }
@@ -452,28 +485,42 @@ void imgRequest::AdjustPriority(imgReque
   if (!progressTracker->FirstObserverIs(proxy))
     return;
 
   nsCOMPtr<nsISupportsPriority> p = do_QueryInterface(mChannel);
   if (p)
     p->AdjustPriority(delta);
 }
 
-void imgRequest::SetIsInCache(bool incache)
+bool
+imgRequest::HasTransferredData() const
 {
-  LOG_FUNC_WITH_PARAM(GetImgLog(), "imgRequest::SetIsCacheable", "incache", incache);
-  mIsInCache = incache;
+  MutexAutoLock lock(mMutex);
+  return mGotData;
 }
 
-void imgRequest::UpdateCacheEntrySize()
+void
+imgRequest::SetIsInCache(bool aInCache)
 {
-  if (mCacheEntry) {
-    size_t size = mImage->SizeOfSourceWithComputedFallback(moz_malloc_size_of);
-    mCacheEntry->SetDataSize(size);
+  LOG_FUNC_WITH_PARAM(GetImgLog(),
+                      "imgRequest::SetIsCacheable", "aInCache", aInCache);
+  MutexAutoLock lock(mMutex);
+  mIsInCache = aInCache;
+}
+
+void
+imgRequest::UpdateCacheEntrySize()
+{
+  if (!mCacheEntry) {
+    return;
   }
+
+  nsRefPtr<Image> image = GetImage();
+  size_t size = image->SizeOfSourceWithComputedFallback(moz_malloc_size_of);
+  mCacheEntry->SetDataSize(size);
 }
 
 void imgRequest::SetCacheValidation(imgCacheEntry* aCacheEntry, nsIRequest* aRequest)
 {
   /* get the expires info */
   if (aCacheEntry) {
     nsCOMPtr<nsICachingChannel> cacheChannel(do_QueryInterface(aRequest));
     if (cacheChannel) {
@@ -580,78 +627,87 @@ imgRequest::CacheChanged(nsIRequest* aNe
   }
 
   // When we get here, app caches differ or app cache is involved
   // just in one of the loads what we also consider as a change
   // in a loading cache.
   return true;
 }
 
+bool
+imgRequest::GetMultipart() const
+{
+  MutexAutoLock lock(mMutex);
+  return mIsMultiPartChannel;
+}
+
 /** nsIRequestObserver methods **/
 
 /* void onStartRequest (in nsIRequest request, in nsISupports ctxt); */
 NS_IMETHODIMP imgRequest::OnStartRequest(nsIRequest *aRequest, nsISupports *ctxt)
 {
   LOG_SCOPE(GetImgLog(), "imgRequest::OnStartRequest");
 
-  mNewPartPending = true;
+  nsRefPtr<Image> image;
 
   // Figure out if we're multipart.
-  nsCOMPtr<nsIMultiPartChannel> mpchan(do_QueryInterface(aRequest));
-  nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
-  if (mpchan) {
-    mIsMultiPartChannel = true;
-  } else {
-    MOZ_ASSERT(!mIsMultiPartChannel, "Something went wrong");
+  nsCOMPtr<nsIMultiPartChannel> multiPartChannel = do_QueryInterface(aRequest);
+  MOZ_ASSERT(multiPartChannel || !mIsMultiPartChannel,
+             "Stopped being multipart?");
+
+  {
+    MutexAutoLock lock(mMutex);
+    mNewPartPending = true;
+    image = mImage;
+    mIsMultiPartChannel = bool(multiPartChannel);
   }
 
   // If we're not multipart, we shouldn't have an image yet.
-  if (mImage && !mIsMultiPartChannel) {
+  if (image && !multiPartChannel) {
     MOZ_ASSERT_UNREACHABLE("Already have an image for a non-multipart request");
     Cancel(NS_IMAGELIB_ERROR_FAILURE);
     return NS_ERROR_FAILURE;
   }
 
   /*
    * If mRequest is null here, then we need to set it so that we'll be able to
    * cancel it if our Cancel() method is called.  Note that this can only
    * happen for multipart channels.  We could simply not null out mRequest for
    * non-last parts, if GetIsLastPart() were reliable, but it's not.  See
    * https://bugzilla.mozilla.org/show_bug.cgi?id=339610
    */
   if (!mRequest) {
-    NS_ASSERTION(mpchan,
-                 "We should have an mRequest here unless we're multipart");
-    nsCOMPtr<nsIChannel> chan;
-    mpchan->GetBaseChannel(getter_AddRefs(chan));
-    mRequest = chan;
+    nsCOMPtr<nsIMultiPartChannel> multiPartChannel = do_QueryInterface(aRequest);
+    MOZ_ASSERT(multiPartChannel, "Should have mRequest unless we're multipart");
+    nsCOMPtr<nsIChannel> baseChannel;
+    multiPartChannel->GetBaseChannel(getter_AddRefs(baseChannel));
+    mRequest = baseChannel;
   }
 
   nsCOMPtr<nsIChannel> channel(do_QueryInterface(aRequest));
-  if (channel)
+  if (channel) {
     channel->GetSecurityInfo(getter_AddRefs(mSecurityInfo));
 
-  /* Get our principal */
-  nsCOMPtr<nsIChannel> chan(do_QueryInterface(aRequest));
-  if (chan) {
+    /* Get our principal */
     nsCOMPtr<nsIScriptSecurityManager> secMan = nsContentUtils::GetSecurityManager();
     if (secMan) {
-      nsresult rv = secMan->GetChannelResultPrincipal(chan,
+      nsresult rv = secMan->GetChannelResultPrincipal(channel,
                                                       getter_AddRefs(mPrincipal));
       if (NS_FAILED(rv)) {
         return rv;
       }
     }
   }
 
   SetCacheValidation(mCacheEntry, aRequest);
 
   mApplicationCache = GetApplicationCache(aRequest);
 
   // Shouldn't we be dead already if this gets hit?  Probably multipart/x-mixed-replace...
+  nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
   if (progressTracker->ObserverCount() == 0) {
     this->Cancel(NS_IMAGELIB_ERROR_FAILURE);
   }
 
   // Try to retarget OnDataAvailable to a decode thread.
   nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aRequest);
   nsCOMPtr<nsIThreadRetargetableRequest> retargetable =
     do_QueryInterface(aRequest);
@@ -674,16 +730,18 @@ NS_IMETHODIMP imgRequest::OnStartRequest
 }
 
 /* void onStopRequest (in nsIRequest request, in nsISupports ctxt, in nsresult status); */
 NS_IMETHODIMP imgRequest::OnStopRequest(nsIRequest *aRequest, nsISupports *ctxt, nsresult status)
 {
   LOG_FUNC(GetImgLog(), "imgRequest::OnStopRequest");
   MOZ_ASSERT(NS_IsMainThread(), "Can't send notifications off-main-thread");
 
+  nsRefPtr<Image> image = GetImage();
+
   // XXXldb What if this is a non-last part of a multipart request?
   // xxx before we release our reference to mRequest, lets
   // save the last status that we saw so that the
   // imgRequestProxy will have access to it.
   if (mRequest) {
     mRequest = nullptr;  // we no longer need the request
   }
 
@@ -695,55 +753,55 @@ NS_IMETHODIMP imgRequest::OnStopRequest(
   }
 
   bool lastPart = true;
   nsCOMPtr<nsIMultiPartChannel> mpchan(do_QueryInterface(aRequest));
   if (mpchan)
     mpchan->GetIsLastPart(&lastPart);
 
   bool isPartial = false;
-  if (mImage && (status == NS_ERROR_NET_PARTIAL_TRANSFER)) {
+  if (image && (status == NS_ERROR_NET_PARTIAL_TRANSFER)) {
     isPartial = true;
     status = NS_OK; // fake happy face
   }
 
   // Tell the image that it has all of the source data. Note that this can
   // trigger a failure, since the image might be waiting for more non-optional
   // data and this is the point where we break the news that it's not coming.
-  if (mImage) {
-    nsresult rv = mImage->OnImageDataComplete(aRequest, ctxt, status, lastPart);
+  if (image) {
+    nsresult rv = image->OnImageDataComplete(aRequest, ctxt, status, lastPart);
 
     // If we got an error in the OnImageDataComplete() call, we don't want to
     // proceed as if nothing bad happened. However, we also want to give
     // precedence to failure status codes from necko, since presumably they're
     // more meaningful.
     if (NS_FAILED(rv) && NS_SUCCEEDED(status))
       status = rv;
   }
 
   // If the request went through, update the cache entry size. Otherwise,
   // cancel the request, which removes us from the cache.
-  if (mImage && NS_SUCCEEDED(status) && !isPartial) {
+  if (image && NS_SUCCEEDED(status) && !isPartial) {
     // We update the cache entry size here because this is where we finish
     // loading compressed source data, which is part of our size calculus.
     UpdateCacheEntrySize();
   }
   else if (isPartial) {
     // Remove the partial image from the cache.
     this->EvictFromCache();
   }
   else {
     mImageErrorCode = status;
 
     // if the error isn't "just" a partial transfer
     // stops animations, removes from cache
     this->Cancel(status);
   }
 
-  if (!mImage) {
+  if (!image) {
     // We have to fire the OnStopRequest notifications ourselves because there's
     // no image capable of doing so.
     Progress progress =
       LoadCompleteProgress(lastPart, /* aError = */ false, status);
 
     nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
     progressTracker->SyncNotifyProgress(progress);
   }
@@ -767,197 +825,246 @@ imgRequest::CheckListenerChain()
 {
   // TODO Might need more checking here.
   NS_ASSERTION(NS_IsMainThread(), "Should be on the main thread!");
   return NS_OK;
 }
 
 /** nsIStreamListener methods **/
 
-/* void onDataAvailable (in nsIRequest request, in nsISupports ctxt, in nsIInputStream inStr, in unsigned long long sourceOffset, in unsigned long count); */
+struct NewPartResult MOZ_FINAL
+{
+  explicit NewPartResult(Image* aExistingImage)
+    : mImage(aExistingImage)
+    , mIsFirstPart(!aExistingImage)
+    , mSucceeded(false)
+  { }
+
+  nsAutoCString mContentType;
+  nsAutoCString mContentDisposition;
+  nsRefPtr<Image> mImage;
+  const bool mIsFirstPart;
+  bool mSucceeded;
+};
+
+static NewPartResult
+PrepareForNewPart(nsIRequest* aRequest, nsIInputStream* aInStr, uint32_t aCount,
+                  ImageURL* aURI, bool aIsMultipart, Image* aExistingImage,
+                  ProgressTracker* aProgressTracker, uint32_t aInnerWindowId)
+{
+  NewPartResult result(aExistingImage);
+
+  mimetype_closure closure;
+  closure.newType = &result.mContentType;
+
+  // Look at the first few bytes and see if we can tell what the data is from
+  // that since servers tend to lie. :(
+  uint32_t out;
+  aInStr->ReadSegments(sniff_mimetype_callback, &closure, aCount, &out);
+
+  nsCOMPtr<nsIChannel> chan(do_QueryInterface(aRequest));
+  if (result.mContentType.IsEmpty()) {
+    nsresult rv = NS_ERROR_FAILURE;
+    if (chan) {
+      rv = chan->GetContentType(result.mContentType);
+      chan->GetContentDispositionHeader(result.mContentDisposition);
+    }
+
+    if (NS_FAILED(rv)) {
+      PR_LOG(GetImgLog(), PR_LOG_ERROR,
+             ("imgRequest::PrepareForNewPart -- Content type unavailable from the channel\n"));
+      return result;
+    }
+  }
+
+  PR_LOG(GetImgLog(), PR_LOG_DEBUG,
+         ("imgRequest::PrepareForNewPart -- Got content type %s\n",
+          result.mContentType.get()));
+
+  // XXX If server lied about mimetype and it's SVG, we may need to copy
+  // the data and dispatch back to the main thread, AND tell the channel to
+  // dispatch there in the future.
+
+  // Create the new image and give it ownership of our ProgressTracker.
+  if (aIsMultipart) {
+    // Create the ProgressTracker and image for this part.
+    nsRefPtr<ProgressTracker> progressTracker = new ProgressTracker();
+    nsRefPtr<Image> partImage =
+      ImageFactory::CreateImage(aRequest, progressTracker, result.mContentType,
+                                aURI, /* aIsMultipart = */ true,
+                                aInnerWindowId);
+
+    if (result.mIsFirstPart) {
+      // First part for a multipart channel. Create the MultipartImage wrapper.
+      MOZ_ASSERT(aProgressTracker, "Shouldn't have given away tracker yet");
+      result.mImage = new MultipartImage(partImage, aProgressTracker);
+    } else {
+      // Transition to the new part.
+      auto multipartImage = static_cast<MultipartImage*>(aExistingImage);
+      multipartImage->BeginTransitionToPart(partImage);
+    }
+  } else {
+    MOZ_ASSERT(!aExistingImage, "New part for non-multipart channel?");
+    MOZ_ASSERT(aProgressTracker, "Shouldn't have given away tracker yet");
+
+    // Create an image using our progress tracker.
+    result.mImage =
+      ImageFactory::CreateImage(aRequest, aProgressTracker, result.mContentType,
+                                aURI, /* aIsMultipart = */ false,
+                                aInnerWindowId);
+  }
+
+  MOZ_ASSERT(result.mImage);
+  if (!result.mImage->HasError() || aIsMultipart) {
+    // We allow multipart images to fail to initialize (which generally
+    // indicates a bad content type) without cancelling the load, because
+    // subsequent parts might be fine.
+    result.mSucceeded = true;
+  }
+
+  return result;
+}
+
+class FinishPreparingForNewPartRunnable MOZ_FINAL : public nsRunnable
+{
+public:
+  FinishPreparingForNewPartRunnable(imgRequest* aImgRequest,
+                                    NewPartResult&& aResult)
+    : mImgRequest(aImgRequest)
+    , mResult(aResult)
+  {
+    MOZ_ASSERT(aImgRequest);
+  }
+
+  NS_IMETHOD Run() MOZ_OVERRIDE
+  {
+    mImgRequest->FinishPreparingForNewPart(mResult);
+    return NS_OK;
+  }
+
+private:
+  nsRefPtr<imgRequest> mImgRequest;
+  NewPartResult mResult;
+};
+
+void
+imgRequest::FinishPreparingForNewPart(const NewPartResult& aResult)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  mContentType = aResult.mContentType;
+
+  SetProperties(aResult.mContentType, aResult.mContentDisposition);
+
+  if (aResult.mIsFirstPart) {
+    // Notify listeners that we have an image.
+    nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
+    progressTracker->OnImageAvailable();
+    MOZ_ASSERT(progressTracker->HasImage());
+  }
+
+  if (IsDecodeRequested()) {
+    aResult.mImage->RequestDecode();
+  }
+}
+
 NS_IMETHODIMP
-imgRequest::OnDataAvailable(nsIRequest *aRequest, nsISupports *ctxt,
-                            nsIInputStream *inStr, uint64_t sourceOffset,
-                            uint32_t count)
+imgRequest::OnDataAvailable(nsIRequest* aRequest, nsISupports* aContext,
+                            nsIInputStream* aInStr, uint64_t aOffset,
+                            uint32_t aCount)
 {
-  LOG_SCOPE_WITH_PARAM(GetImgLog(), "imgRequest::OnDataAvailable", "count", count);
+  LOG_SCOPE_WITH_PARAM(GetImgLog(), "imgRequest::OnDataAvailable", "count", aCount);
 
   NS_ASSERTION(aRequest, "imgRequest::OnDataAvailable -- no request!");
 
-  nsresult rv;
-  mGotData = true;
-
-  if (mNewPartPending) {
-    LOG_SCOPE(GetImgLog(), "imgRequest::OnDataAvailable |New part; finding MIME type|");
-
-    mNewPartPending = false;
+  nsRefPtr<Image> image;
+  nsRefPtr<ProgressTracker> progressTracker;
+  bool isMultipart = false;
+  bool newPartPending = false;
 
-    mimetype_closure closure;
-    nsAutoCString newType;
-    closure.newType = &newType;
-
-    /* look at the first few bytes and see if we can tell what the data is from that
-     * since servers tend to lie. :(
-     */
-    uint32_t out;
-    inStr->ReadSegments(sniff_mimetype_callback, &closure, count, &out);
+  // Retrieve and update our state.
+  {
+    MutexAutoLock lock(mMutex);
+    mGotData = true;
+    image = mImage;
+    progressTracker = mProgressTracker;
+    isMultipart = mIsMultiPartChannel;
+    newPartPending = mNewPartPending;
+    mNewPartPending = false;
+  }
 
-    nsCOMPtr<nsIChannel> chan(do_QueryInterface(aRequest));
-    if (newType.IsEmpty()) {
-      LOG_SCOPE(GetImgLog(), "imgRequest::OnDataAvailable |sniffing of mimetype failed|");
-
-      rv = NS_ERROR_FAILURE;
-      if (chan) {
-        rv = chan->GetContentType(newType);
-      }
+  // If this is a new part, we need to sniff its content type and create an
+  // appropriate image.
+  if (newPartPending) {
+    NewPartResult result = PrepareForNewPart(aRequest, aInStr, aCount, mURI,
+                                             isMultipart, image,
+                                             progressTracker, mInnerWindowId);
+    bool succeeded = result.mSucceeded;
 
-      if (NS_FAILED(rv)) {
-        PR_LOG(GetImgLog(), PR_LOG_ERROR,
-               ("[this=%p] imgRequest::OnDataAvailable -- Content type unavailable from the channel\n",
-                this));
+    if (result.mImage) {
+      image = result.mImage;
 
-        this->Cancel(NS_IMAGELIB_ERROR_FAILURE);
-
-        return NS_BINDING_ABORTED;
+      // Update our state to reflect this new part.
+      {
+        MutexAutoLock lock(mMutex);
+        mImage = image;
+        mProgressTracker = nullptr;
       }
 
-      LOG_MSG(GetImgLog(), "imgRequest::OnDataAvailable", "Got content type from the channel");
-    }
-
-    mContentType = newType;
-    SetProperties(chan);
-    bool firstPart = !mImage;
-
-    LOG_MSG_WITH_PARAM(GetImgLog(), "imgRequest::OnDataAvailable", "content type", mContentType.get());
-
-    // XXX If server lied about mimetype and it's SVG, we may need to copy
-    // the data and dispatch back to the main thread, AND tell the channel to
-    // dispatch there in the future.
-
-    // Create the new image and give it ownership of our ProgressTracker.
-    if (mIsMultiPartChannel) {
-      // Create the ProgressTracker and image for this part.
-      nsRefPtr<ProgressTracker> progressTracker = new ProgressTracker();
-      nsRefPtr<Image> image =
-        ImageFactory::CreateImage(aRequest, progressTracker, mContentType,
-                                  mURI, /* aIsMultipart = */ true,
-                                  static_cast<uint32_t>(mInnerWindowId));
-
-      if (!mImage) {
-        // First part for a multipart channel. Create the MultipartImage wrapper.
-        MOZ_ASSERT(mProgressTracker, "Shouldn't have given away tracker yet");
-        mImage = new MultipartImage(image, mProgressTracker);
-        mProgressTracker = nullptr;
-      } else {
-        // Transition to the new part.
-        static_cast<MultipartImage*>(mImage.get())->BeginTransitionToPart(image);
-      }
-    } else {
-      MOZ_ASSERT(!mImage, "New part for non-multipart channel?");
-      MOZ_ASSERT(mProgressTracker, "Shouldn't have given away tracker yet");
-
-      // Create an image using our progress tracker.
-      mImage =
-        ImageFactory::CreateImage(aRequest, mProgressTracker, mContentType,
-                                  mURI, /* aIsMultipart = */ false,
-                                  static_cast<uint32_t>(mInnerWindowId));
-      mProgressTracker = nullptr;
-    }
-
-    if (firstPart) {
-      // Notify listeners that we have an image.
-      nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
-
+      // Some property objects are not threadsafe, and we need to send
+      // OnImageAvailable on the main thread, so finish on the main thread.
       if (NS_IsMainThread()) {
-        progressTracker->OnImageAvailable();
-        MOZ_ASSERT(progressTracker->HasImage());
+        FinishPreparingForNewPart(result);
       } else {
         nsCOMPtr<nsIRunnable> runnable =
-          NS_NewRunnableMethod(progressTracker, &ProgressTracker::OnImageAvailable);
+          new FinishPreparingForNewPartRunnable(this, Move(result));
         NS_DispatchToMainThread(runnable);
       }
     }
 
-    if (mImage->HasError() && !mIsMultiPartChannel) { // Probably bad mimetype
-      // We allow multipart images to fail to initialize without cancelling the
-      // load because subsequent images might be fine; thus only single part
-      // images end up here.
-      this->Cancel(NS_IMAGELIB_ERROR_FAILURE);
+    if (!succeeded) {
+      // Something went wrong; probably a content type issue.
+      Cancel(NS_IMAGELIB_ERROR_FAILURE);
       return NS_BINDING_ABORTED;
     }
-
-    MOZ_ASSERT(!mProgressTracker, "Should've given tracker to image");
-    MOZ_ASSERT(mImage, "Should have image");
-
-    if (mDecodeRequested) {
-      mImage->StartDecoding();
-    }
   }
 
   // Notify the image that it has new data.
-  rv = mImage->OnImageDataAvailable(aRequest, ctxt, inStr, sourceOffset, count);
+  nsresult rv =
+    image->OnImageDataAvailable(aRequest, aContext, aInStr, aOffset, aCount);
 
   if (NS_FAILED(rv)) {
     PR_LOG(GetImgLog(), PR_LOG_WARNING,
            ("[this=%p] imgRequest::OnDataAvailable -- "
             "copy to RasterImage failed\n", this));
-    this->Cancel(NS_IMAGELIB_ERROR_FAILURE);
+    Cancel(NS_IMAGELIB_ERROR_FAILURE);
     return NS_BINDING_ABORTED;
   }
 
   return NS_OK;
 }
 
-class SetPropertiesEvent : public nsRunnable
+void
+imgRequest::SetProperties(const nsACString& aContentType,
+                          const nsACString& aContentDisposition)
 {
-public:
-  SetPropertiesEvent(imgRequest* aImgRequest, nsIChannel* aChan)
-    : mImgRequest(aImgRequest)
-    , mChan(aChan)
-  {
-    MOZ_ASSERT(!NS_IsMainThread(), "Should be created off the main thread");
-    MOZ_ASSERT(aImgRequest, "aImgRequest cannot be null");
-  }
-  NS_IMETHOD Run()
-  {
-    MOZ_ASSERT(NS_IsMainThread(), "Should run on the main thread only");
-    MOZ_ASSERT(mImgRequest, "mImgRequest cannot be null");
-    mImgRequest->SetProperties(mChan);
-    return NS_OK;
-  }
-private:
-  nsRefPtr<imgRequest> mImgRequest;
-  nsCOMPtr<nsIChannel> mChan;
-};
-
-void
-imgRequest::SetProperties(nsIChannel* aChan)
-{
-  // Force execution on main thread since some property objects are non
-  // threadsafe.
-  if (!NS_IsMainThread()) {
-    NS_DispatchToMainThread(new SetPropertiesEvent(this, aChan));
-    return;
-  }
   /* set our mimetype as a property */
-  nsCOMPtr<nsISupportsCString> contentType(do_CreateInstance("@mozilla.org/supports-cstring;1"));
+  nsCOMPtr<nsISupportsCString> contentType =
+    do_CreateInstance("@mozilla.org/supports-cstring;1");
   if (contentType) {
-    contentType->SetData(mContentType);
+    contentType->SetData(aContentType);
     mProperties->Set("type", contentType);
   }
 
   /* set our content disposition as a property */
-  nsAutoCString disposition;
-  if (aChan) {
-    aChan->GetContentDispositionHeader(disposition);
-  }
-  if (!disposition.IsEmpty()) {
-    nsCOMPtr<nsISupportsCString> contentDisposition(do_CreateInstance("@mozilla.org/supports-cstring;1"));
+  if (!aContentDisposition.IsEmpty()) {
+    nsCOMPtr<nsISupportsCString> contentDisposition =
+      do_CreateInstance("@mozilla.org/supports-cstring;1");
     if (contentDisposition) {
-      contentDisposition->SetData(disposition);
+      contentDisposition->SetData(aContentDisposition);
       mProperties->Set("content-disposition", contentDisposition);
     }
   }
 }
 
 static NS_METHOD sniff_mimetype_callback(nsIInputStream* in,
                                          void* data,
                                          const char* fromRawSegment,
--- a/image/src/imgRequest.h
+++ b/image/src/imgRequest.h
@@ -14,17 +14,17 @@
 #include "nsIPrincipal.h"
 
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsProxyRelease.h"
 #include "nsStringGlue.h"
 #include "nsError.h"
 #include "nsIAsyncVerifyRedirectCallback.h"
-#include "mozilla/Atomics.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/net/ReferrerPolicy.h"
 
 class imgCacheValidator;
 class imgLoader;
 class imgRequestProxy;
 class imgCacheEntry;
 class imgMemoryReporter;
 class imgRequestNotifyRunnable;
@@ -37,16 +37,18 @@ class nsIURI;
 namespace mozilla {
 namespace image {
 class Image;
 class ImageURL;
 class ProgressTracker;
 } // namespace image
 } // namespace mozilla
 
+struct NewPartResult;
+
 class imgRequest final : public nsIStreamListener,
                              public nsIThreadRetargetableStreamListener,
                              public nsIChannelEventSink,
                              public nsIInterfaceRequestor,
                              public nsIAsyncVerifyRedirectCallback
 {
   virtual ~imgRequest();
 
@@ -84,17 +86,17 @@ public:
 
   // Called or dispatched by cancel for main thread only execution.
   void ContinueCancel(nsresult aStatus);
 
   // Called or dispatched by EvictFromCache for main thread only execution.
   void ContinueEvict();
 
   // Request that we start decoding the image as soon as data becomes available.
-  void RequestDecode() { mDecodeRequested = true; }
+  void RequestDecode();
 
   inline uint64_t InnerWindowID() const {
     return mInnerWindowId;
   }
 
   // Set the cache validation information (expiry time, whether we must
   // validate, etc) on the cache entry based on the request information.
   // If this function is called multiple times, the information set earliest
@@ -102,17 +104,17 @@ public:
   static void SetCacheValidation(imgCacheEntry* aEntry, nsIRequest* aRequest);
 
   // Check if application cache of the original load is different from
   // application cache of the new load.  Also lack of application cache
   // on one of the loads is considered a change of a loading cache since
   // HTTP cache may contain a different data then app cache.
   bool CacheChanged(nsIRequest* aNewRequest);
 
-  bool GetMultipart() const { return mIsMultiPartChannel; }
+  bool GetMultipart() const;
 
   // The CORS mode for which we loaded this image.
   int32_t GetCORSMode() const { return mCORSMode; }
 
   // The Referrer Policy in effect when loading this image.
   ReferrerPolicy GetReferrerPolicy() const { return mReferrerPolicy; }
 
   // The principal for the document that loaded this image. Used when trying to
@@ -180,55 +182,61 @@ private:
   // PRIORITY_NORMAL if it doesn't support nsISupportsPriority.
   int32_t Priority() const;
 
   // Adjust the priority of the underlying network request by the given delta
   // on behalf of the given proxy.
   void AdjustPriority(imgRequestProxy *aProxy, int32_t aDelta);
 
   // Return whether we've seen some data at this point
-  bool HasTransferredData() const { return mGotData; }
+  bool HasTransferredData() const;
 
   // Set whether this request is stored in the cache. If it isn't, regardless
   // of whether this request has a non-null mCacheEntry, this imgRequest won't
   // try to update or modify the image cache.
   void SetIsInCache(bool cacheable);
 
   bool HasConsumers();
 
+  /// Returns true if RequestDecode() was called.
+  bool IsDecodeRequested() const;
+
 public:
   NS_DECL_NSISTREAMLISTENER
   NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
   NS_DECL_NSIREQUESTOBSERVER
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIINTERFACEREQUESTOR
   NS_DECL_NSIASYNCVERIFYREDIRECTCALLBACK
 
   // Sets properties for this image; will dispatch to main thread if needed.
-  void SetProperties(nsIChannel* aChan);
+  void SetProperties(const nsACString& aContentType,
+                     const nsACString& aContentDisposition);
 
 private:
   friend class imgMemoryReporter;
+  friend class FinishPreparingForNewPartRunnable;
+
+  already_AddRefed<Image> GetImage() const;
+
+  void FinishPreparingForNewPart(const NewPartResult& aResult);
 
   // Weak reference to parent loader; this request cannot outlive its owner.
   imgLoader* mLoader;
   nsCOMPtr<nsIRequest> mRequest;
   // The original URI we were loaded with. This is the same as the URI we are
   // keyed on in the cache. We store a string here to avoid off main thread
   // refcounting issues with nsStandardURL.
   nsRefPtr<ImageURL> mURI;
   // The URI of the resource we ended up loading after all redirects, etc.
   nsCOMPtr<nsIURI> mCurrentURI;
   // The principal of the document which loaded this image. Used when validating for CORS.
   nsCOMPtr<nsIPrincipal> mLoadingPrincipal;
   // The principal of this image.
   nsCOMPtr<nsIPrincipal> mPrincipal;
-  // Progress tracker -- transferred to mImage, when it gets instantiated.
-  nsRefPtr<ProgressTracker> mProgressTracker;
-  nsRefPtr<Image> mImage;
   nsCOMPtr<nsIProperties> mProperties;
   nsCOMPtr<nsISupports> mSecurityInfo;
   nsCOMPtr<nsIChannel> mChannel;
   nsCOMPtr<nsIInterfaceRequestor> mPrevChannelSink;
   nsCOMPtr<nsIApplicationCache> mApplicationCache;
 
   nsCOMPtr<nsITimedChannel> mTimedChannel;
 
@@ -249,17 +257,23 @@ private:
   // default, imgIRequest::CORS_NONE.
   int32_t mCORSMode;
 
   // The Referrer Policy (defined in ReferrerPolicy.h) used for this image.
   ReferrerPolicy mReferrerPolicy;
 
   nsresult mImageErrorCode;
 
-  mozilla::Atomic<bool> mDecodeRequested;
+  mutable mozilla::Mutex mMutex;
 
+  // Member variables protected by mMutex. Note that *all* flags in our bitfield
+  // are protected by mMutex; if you're adding a new flag that isn'protected, it
+  // must not be a part of this bitfield.
+  nsRefPtr<ProgressTracker> mProgressTracker;
+  nsRefPtr<Image> mImage;
   bool mIsMultiPartChannel : 1;
   bool mGotData : 1;
   bool mIsInCache : 1;
+  bool mDecodeRequested : 1;
   bool mNewPartPending : 1;
 };
 
 #endif