Bug 795940 - Part 0.2 - Change from singleton ScaleWorker and DrawWorker (and on-demand allocated ScaleRequest) to nsIRunnables, allocated on demand, that know how to run themselves. r=jrmuizel
authorJoe Drew <joe@drew.ca>
Fri, 12 Oct 2012 18:20:19 -0400
changeset 110690 05a2876b0d0264e02f424751558bae6d5024cf0d
parent 110689 b0a701fd2322d04c1c6c338b8c0c8035e672d86b
child 110691 510cab1158baced8c9dd8903a6e03899e57b1c86
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersjrmuizel
bugs795940
milestone19.0a1
Bug 795940 - Part 0.2 - Change from singleton ScaleWorker and DrawWorker (and on-demand allocated ScaleRequest) to nsIRunnables, allocated on demand, that know how to run themselves. r=jrmuizel
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -139,18 +139,16 @@ DiscardingEnabled()
 
   return enabled;
 }
 
 namespace mozilla {
 namespace image {
 
 /* static */ StaticRefPtr<RasterImage::DecodeWorker> RasterImage::DecodeWorker::sSingleton;
-/* static */ nsRefPtr<RasterImage::ScaleWorker> RasterImage::ScaleWorker::sSingleton;
-/* static */ nsRefPtr<RasterImage::DrawWorker> RasterImage::DrawWorker::sSingleton;
 static nsCOMPtr<nsIThread> sScaleWorkerThread = nullptr;
 
 #ifndef DEBUG
 NS_IMPL_ISUPPORTS2(RasterImage, imgIContainer, nsIProperties)
 #else
 NS_IMPL_ISUPPORTS3(RasterImage, imgIContainer, nsIProperties,
                    imgIContainerDebug)
 #endif
@@ -231,18 +229,16 @@ RasterImage::~RasterImage()
 void
 RasterImage::Initialize()
 {
   InitPrefCaches();
 
   // Create our singletons now, so we don't have to worry about what thread
   // they're created on.
   DecodeWorker::Singleton();
-  DrawWorker::Singleton();
-  ScaleWorker::Singleton();
 }
 
 nsresult
 RasterImage::Init(imgIDecoderObserver *aObserver,
                   const char* aMimeType,
                   const char* aURIString,
                   uint32_t aFlags)
 {
@@ -2616,145 +2612,89 @@ RasterImage::SyncDecode()
     rv = ShutdownDecoder(eShutdownIntent_Done);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
   // All good if no errors!
   return mError ? NS_ERROR_FAILURE : NS_OK;
 }
 
-/* static */ RasterImage::ScaleWorker*
-RasterImage::ScaleWorker::Singleton()
+nsresult
+RasterImage::ScaleRunner::Run()
 {
-  if (!sSingleton) {
-    sSingleton = new ScaleWorker();
-    ClearOnShutdown(&sSingleton);
-  }
-
-  return sSingleton;
-}
-
-nsresult
-RasterImage::ScaleWorker::Run()
-{
-  if (!mInitialized) {
-    PR_SetCurrentThreadName("Image Scaler");
-    mInitialized = true;
-  }
-
-  ScaleRequest* request;
-  {
-    MutexAutoLock lock(ScaleWorker::Singleton()->mRequestsMutex);
-    request = mScaleRequests.popFirst();
-  }
+  // An alias just for ease of typing
+  ScaleRequest* request = mScaleRequest;
 
   request->done = mozilla::gfx::Scale(request->srcData, request->srcRect.width, request->srcRect.height, request->srcStride,
                                       request->dstData, request->dstSize.width, request->dstSize.height, request->dstStride,
                                       request->srcFormat);
 
   // OK, we've got a new scaled image. Let's get the main thread to unlock and
   // redraw it.
-  DrawWorker::Singleton()->RequestDraw(request);
+  DrawRunner* runner = new DrawRunner(mScaleRequest.forget());
+  NS_DispatchToMainThread(runner, NS_DISPATCH_NORMAL);
 
   return NS_OK;
 }
 
-// Note: you MUST call RequestScale with the ScaleWorker mutex held.
-bool
-RasterImage::ScaleWorker::RequestScale(ScaleRequest* request,
-                                       RasterImage* image,
-                                       imgFrame* aSrcFrame)
+RasterImage::ScaleRunner::ScaleRunner(RasterImage* aImage, const gfxSize& aScale, imgFrame* aSrcFrame)
 {
-  mRequestsMutex.AssertCurrentThreadOwns();
+  nsAutoPtr<ScaleRequest> request(new ScaleRequest(aImage, aScale, aSrcFrame));
 
   // Destination is unconditionally ARGB32 because that's what the scaler
   // outputs.
   request->dstFrame = new imgFrame();
   nsresult rv = request->dstFrame->Init(0, 0, request->dstSize.width, request->dstSize.height,
                                         gfxASurface::ImageFormatARGB32);
 
   if (NS_FAILED(rv) || !request->GetSurfaces(aSrcFrame)) {
-    return false;
-  }
-
-  mScaleRequests.insertBack(request);
-
-  if (!sScaleWorkerThread) {
-    NS_NewThread(getter_AddRefs(sScaleWorkerThread), this, NS_DISPATCH_NORMAL);
-    ClearOnShutdown(&sScaleWorkerThread);
-  }
-  else {
-    sScaleWorkerThread->Dispatch(this, NS_DISPATCH_NORMAL);
+    return;
   }
 
-  image->SetResultPending(request);
-
-  return true;
+  aImage->SetResultPending(request);
+
+  mScaleRequest = request;
 }
 
-/* static */ RasterImage::DrawWorker*
-RasterImage::DrawWorker::Singleton()
-{
-  if (!sSingleton) {
-    sSingleton = new DrawWorker();
-    ClearOnShutdown(&sSingleton);
-  }
-
-  return sSingleton;
-}
+RasterImage::DrawRunner::DrawRunner(ScaleRequest* request)
+ : mScaleRequest(request)
+{}
 
 nsresult
-RasterImage::DrawWorker::Run()
+RasterImage::DrawRunner::Run()
 {
-  ScaleRequest* request;
-  {
-    MutexAutoLock lock(ScaleWorker::Singleton()->mRequestsMutex);
-    request = mDrawRequests.popFirst();
-  }
-
   // ScaleWorker is finished with this request, so we can unlock the data now.
-  request->ReleaseSurfaces();
+  mScaleRequest->ReleaseSurfaces();
 
   // Only set the scale result if the request finished successfully.
-  if (request->done) {
-    RasterImage* image = request->weakImage;
+  if (mScaleRequest->done) {
+    RasterImage* image = mScaleRequest->weakImage;
     if (image) {
       nsCOMPtr<imgIContainerObserver> observer(do_QueryReferent(image->mObserver));
       if (observer) {
-        imgFrame *scaledFrame = request->dstFrame.get();
+        imgFrame *scaledFrame = mScaleRequest->dstFrame.get();
         scaledFrame->ImageUpdated(scaledFrame->GetRect());
-        observer->FrameChanged(&request->srcRect);
+        observer->FrameChanged(&mScaleRequest->srcRect);
       }
 
-      image->SetScaleResult(request);
+      image->SetScaleResult(mScaleRequest);
     }
   }
 
   // Scaling failed. Reset the scale result on our image.
   else {
-    RasterImage* image = request->weakImage;
+    RasterImage* image = mScaleRequest->weakImage;
     if (image) {
       image->SetScaleResult(nullptr);
     }
   }
 
-  // We're all done with this ScaleRequest; now it dies.
-  delete request;
-
   return NS_OK;
 }
 
-void
-RasterImage::DrawWorker::RequestDraw(ScaleRequest* request)
-{
-  MutexAutoLock lock(ScaleWorker::Singleton()->mRequestsMutex);
-  mDrawRequests.insertBack(request);
-  NS_DispatchToMainThread(this, NS_DISPATCH_NORMAL);
-}
-
 static inline bool
 IsDownscale(const gfxSize& scale)
 {
   if (scale.width > 1.0)
     return false;
   if (scale.height > 1.0)
     return false;
   if (scale.width == 1.0 && scale.height == 1.0)
@@ -2813,17 +2753,16 @@ RasterImage::DrawWithPreDownscaleIfNeede
   nsIntRect framerect = frame->GetRect();
   gfxMatrix userSpaceToImageSpace = aUserSpaceToImageSpace;
   gfxMatrix imageSpaceToUserSpace = aUserSpaceToImageSpace;
   imageSpaceToUserSpace.Invert();
   gfxSize scale = imageSpaceToUserSpace.ScaleFactors(true);
   nsIntRect subimage = aSubimage;
 
   if (CanScale(aFilter, scale)) {
-    MutexAutoLock lock(ScaleWorker::Singleton()->mRequestsMutex);
     // If scale factor is still the same that we scaled for and
     // ScaleWorker isn't still working, then we can use pre-downscaled frame.
     // If scale factor has changed, order new request.
     // FIXME: Current implementation doesn't support pre-downscale
     // mechanism for multiple sizes from same src, since we cache
     // pre-downscaled frame only for the latest requested scale.
     // The solution is to cache more than one scaled image frame
     // for each RasterImage.
@@ -2834,21 +2773,24 @@ RasterImage::DrawWithPreDownscaleIfNeede
       // Since we're switching to a scaled image, we we need to transform the
       // area of the subimage to draw accordingly, since imgFrame::Draw()
       // doesn't know about scaled frames.
       subimage.ScaleRoundOut(scale.width, scale.height);
     }
 
     // If we're not waiting for exactly this result, ask for it.
     else if (!(mScaleResult.status == SCALE_PENDING && mScaleResult.scale == scale)) {
-      ScaleRequest* request = new ScaleRequest(this, scale, frame);
-
-      if (!ScaleWorker::Singleton()->RequestScale(request, this, frame)) {
-        // Requesting a scale failed. Not much we can do.
-        delete request;
+      nsRefPtr<ScaleRunner> runner = new ScaleRunner(this, scale, frame);
+      if (runner->IsOK()) {
+        if (!sScaleWorkerThread) {
+          NS_NewNamedThread("Image Scaler", getter_AddRefs(sScaleWorkerThread));
+          ClearOnShutdown(&sScaleWorkerThread);
+        }
+
+        sScaleWorkerThread->Dispatch(runner, NS_DISPATCH_NORMAL);
       }
     }
   }
 
   nsIntMargin padding(framerect.x, framerect.y,
                       mSize.width - framerect.XMost(),
                       mSize.height - framerect.YMost());
 
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -12,17 +12,16 @@
  * @author  Chris Saari <saari@netscape.com>
  * @author  Arron Mogge <paper@animecity.nu>
  * @author  Andrew Smith <asmith15@learn.senecac.on.ca>
  */
 
 #ifndef mozilla_imagelib_RasterImage_h_
 #define mozilla_imagelib_RasterImage_h_
 
-#include "mozilla/Mutex.h"
 #include "Image.h"
 #include "nsCOMArray.h"
 #include "nsCOMPtr.h"
 #include "imgIContainer.h"
 #include "nsIProperties.h"
 #include "nsITimer.h"
 #include "nsWeakReference.h"
 #include "nsTArray.h"
@@ -469,17 +468,17 @@ private:
     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;
   };
 
-  struct ScaleRequest : public LinkedListElement<ScaleRequest>
+  struct ScaleRequest
   {
     ScaleRequest(RasterImage* aImage, const gfxSize& aScale, imgFrame* aSrcFrame)
       : scale(aScale)
       , dstLocked(false)
       , done(false)
     {
       MOZ_ASSERT(!aSrcFrame->GetIsPaletted());
       MOZ_ASSERT(aScale.width > 0 && aScale.height > 0);
@@ -589,62 +588,39 @@ private:
      : status(SCALE_INVALID)
     {}
 
     gfxSize scale;
     nsAutoPtr<imgFrame> frame;
     ScaleStatus status;
   };
 
-  class ScaleWorker : public nsRunnable
+  class ScaleRunner : public nsRunnable
   {
   public:
-    static ScaleWorker* Singleton();
+    ScaleRunner(RasterImage* aImage, const gfxSize& aScale, imgFrame* aSrcFrame);
 
     NS_IMETHOD Run();
 
-  /* statics */
-    static nsRefPtr<ScaleWorker> sSingleton;
+    bool IsOK() const { return !!mScaleRequest; }
+
+  private:
+    nsAutoPtr<ScaleRequest> mScaleRequest;
+  };
 
-  private: /* methods */
-    ScaleWorker()
-      : mRequestsMutex("RasterImage.ScaleWorker.mRequestsMutex")
-      , mInitialized(false)
-    {};
+  class DrawRunner : public nsRunnable
+  {
+  public:
+    DrawRunner(ScaleRequest* request);
 
-    // Note: you MUST call RequestScale with the ScaleWorker mutex held.
-    bool RequestScale(ScaleRequest* request, RasterImage* image, imgFrame* aSrcFrame);
+    NS_IMETHOD Run();
 
   private: /* members */
 
-    friend class RasterImage;
-    LinkedList<ScaleRequest> mScaleRequests;
-    Mutex mRequestsMutex;
-    bool mInitialized;
-  };
-
-  class DrawWorker : public nsRunnable
-  {
-  public:
-    static DrawWorker* Singleton();
-
-    NS_IMETHOD Run();
-
-  /* statics */
-    static nsRefPtr<DrawWorker> sSingleton;
-
-  private: /* methods */
-    DrawWorker() {};
-
-    void RequestDraw(ScaleRequest* request);
-
-  private: /* members */
-
-    friend class RasterImage;
-    LinkedList<ScaleRequest> mDrawRequests;
+    nsAutoPtr<ScaleRequest> mScaleRequest;
   };
 
   void DrawWithPreDownscaleIfNeeded(imgFrame *aFrame,
                                     gfxContext *aContext,
                                     gfxPattern::GraphicsFilter aFilter,
                                     const gfxMatrix &aUserSpaceToImageSpace,
                                     const gfxRect &aFill,
                                     const nsIntRect &aSubimage);