Bug 1465619 - Part 5. Move actual drawing in imgFrame::Draw outside the monitor. r=tnikkel

Since imgFrame::Draw will limit the drawing to only look at pixels that
we have written to and posted an invalidation for, there is no need to
hold the monitor while doing so. By taking the most expensive operation
outside the lock, we will minimize our chances of hitting contention
with the decoder thread.

A later part in this series will require that a surface be freed outside
the lock because it may end up reacquiring it. In addition to the
contention win, this change should facilitate that.

Differential Revision: https://phabricator.services.mozilla.com/D7510

image/imgFrame.cpp

@@ -562,26 +562,33 @@ bool imgFrame::Draw(gfxContext* aContext, const ImageRegion& aRegion,
     return false;
   }
 
-  MonitorAutoLock lock(mMonitor);
-
-  // Possibly convert this image into a GPU texture, this may also cause our
-  // mLockedSurface to be released and the OS to release the underlying memory.
-  Optimize(aContext->GetDrawTarget());
-
-  bool doPartialDecode = !AreAllPixelsWritten();
-
-  RefPtr<SourceSurface> surf = GetSourceSurfaceInternal();
-  if (!surf) {
-    return false;
-  }
-
-  gfxRect imageRect(0, 0, mImageSize.width, mImageSize.height);
-  bool doTile = !imageRect.Contains(aRegion.Rect()) &&
-                !(aImageFlags & imgIContainer::FLAG_CLAMP);
-
+  // Perform the draw and freeing of the surface outside the lock. We want to
+  // avoid contention with the decoder if we can.
+  RefPtr<SourceSurface> surf;
+  SurfaceWithFormat surfaceResult;
   ImageRegion region(aRegion);
-  SurfaceWithFormat surfaceResult =
-    SurfaceForDrawing(doPartialDecode, doTile, region, surf);
+  gfxRect imageRect(0, 0, mImageSize.width, mImageSize.height);
+
+  {
+    MonitorAutoLock lock(mMonitor);
+
+    // Possibly convert this image into a GPU texture, this may also cause our
+    // mLockedSurface to be released and the OS to release the underlying memory.
+    Optimize(aContext->GetDrawTarget());
+
+    bool doPartialDecode = !AreAllPixelsWritten();
+
+    surf = GetSourceSurfaceInternal();
+    if (!surf) {
+      return false;
+    }
+
+    bool doTile = !imageRect.Contains(aRegion.Rect()) &&
+                  !(aImageFlags & imgIContainer::FLAG_CLAMP);
+
+    surfaceResult =
+      SurfaceForDrawing(doPartialDecode, doTile, region, surf);
+  }
 
   if (surfaceResult.IsValid()) {
     gfxUtils::DrawPixelSnapped(aContext, surfaceResult.mDrawable,

image/imgFrame.h

@@ -265,6 +265,17 @@ private: // methods
     SurfaceWithFormat(gfxDrawable* aDrawable, SurfaceFormat aFormat)
       : mDrawable(aDrawable), mFormat(aFormat)
     { }
+    SurfaceWithFormat(SurfaceWithFormat&& aOther)
+      : mDrawable(std::move(aOther.mDrawable)), mFormat(aOther.mFormat)
+    { }
+    SurfaceWithFormat& operator=(SurfaceWithFormat&& aOther)
+    {
+      mDrawable = std::move(aOther.mDrawable);
+      mFormat = aOther.mFormat;
+      return *this;
+    }
+    SurfaceWithFormat& operator=(const SurfaceWithFormat& aOther) = delete;
+    SurfaceWithFormat(const SurfaceWithFormat& aOther) = delete;
     bool IsValid() { return !!mDrawable; }
   };