From 807acf738f63d95fbecfb9129092aaa2379889ba Mon Sep 17 00:00:00 2001
From: wolfbeast <mcwerewolf@gmail.com>
Date: Tue, 20 Nov 2018 20:38:49 +0100
Subject: Bug 1462355 - Part 1a. Make imgFrame animation parameters threadsafe.

We currently choose to set the animation parameters (blend method, blend
rect, disposal method, timeout) in imgFrame::Finish instead of
imgFrame::InitForDecoder. The decoders themselves already have access to
the necessary information at the time InitForDecoder is called, so there
is no reason to do this. Moving the configuration to initialization will
allow us to relax the mutex protection on these parameters.

This part simply reorganizes imgFrame, and subsequent parts will
introduce the necessary changes to SurfacePipe and decoders.
---
 image/imgFrame.cpp | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

(limited to 'image/imgFrame.cpp')

diff --git a/image/imgFrame.cpp b/image/imgFrame.cpp
index 5da2ccec5..af84d8cbb 100644
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -161,13 +161,13 @@ imgFrame::imgFrame()
   : mMonitor("imgFrame")
   , mDecoded(0, 0, 0, 0)
   , mLockCount(0)
+  , mAborted(false)
+  , mFinished(false)
+  , mOptimizable(false)
   , mTimeout(FrameTimeout::FromRawMilliseconds(100))
   , mDisposalMethod(DisposalMethod::NOT_SPECIFIED)
   , mBlendMethod(BlendMethod::OVER)
   , mHasNoAlpha(false)
-  , mAborted(false)
-  , mFinished(false)
-  , mOptimizable(false)
   , mPalettedImageData(nullptr)
   , mPaletteDepth(0)
   , mNonPremult(false)
@@ -192,7 +192,8 @@ imgFrame::InitForDecoder(const nsIntSize& aImageSize,
                          const nsIntRect& aRect,
                          SurfaceFormat aFormat,
                          uint8_t aPaletteDepth /* = 0 */,
-                         bool aNonPremult /* = false */)
+                         bool aNonPremult /* = false */,
+                         const Maybe<AnimationParams>& aAnimParams /* = Nothing() */)
 {
   // Assert for properties that should be verified by decoders,
   // warn for properties related to bad content.
@@ -205,6 +206,15 @@ imgFrame::InitForDecoder(const nsIntSize& aImageSize,
   mImageSize = aImageSize;
   mFrameRect = aRect;
 
+  if (aAnimParams) {
+    mBlendRect = aAnimParams->mBlendRect;
+    mTimeout = aAnimParams->mTimeout;
+    mBlendMethod = aAnimParams->mBlendMethod;
+    mDisposalMethod = aAnimParams->mDisposalMethod;
+  } else {
+    mBlendRect = aRect;
+  }
+
   // We only allow a non-trivial frame rect (i.e., a frame rect that doesn't
   // cover the entire image) for paletted animation frames. We never draw those
   // frames directly; we just use FrameAnimator to composite them and produce a
@@ -242,13 +252,13 @@ imgFrame::InitForDecoder(const nsIntSize& aImageSize,
   } else {
     MOZ_ASSERT(!mImageSurface, "Called imgFrame::InitForDecoder() twice?");
 
-    mVBuf = AllocateBufferForImage(mFrameRect.Size(), mFormat);
-    if (!mVBuf) {
+    mRawSurface = AllocateBufferForImage(mFrameRect.Size(), mFormat);
+    if (!mRawSurface) {
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    mImageSurface = CreateLockedSurface(mVBuf, mFrameRect.Size(), mFormat);
+    mImageSurface = CreateLockedSurface(mRawSurface, mFrameRect.Size(), mFormat);
 
     if (!mImageSurface) {
       NS_WARNING("Failed to create ImageSurface");
@@ -256,7 +266,7 @@ imgFrame::InitForDecoder(const nsIntSize& aImageSize,
       return NS_ERROR_OUT_OF_MEMORY;
     }
 
-    if (!ClearSurface(mVBuf, mFrameRect.Size(), mFormat)) {
+    if (!ClearSurface(mRawSurface, mFrameRect.Size(), mFormat)) {
       NS_WARNING("Could not clear allocated buffer");
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
@@ -607,12 +617,7 @@ imgFrame::ImageUpdatedInternal(const nsIntRect& aUpdateRect)
 }
 
 void
-imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
-                 DisposalMethod aDisposalMethod /* = DisposalMethod::KEEP */,
-                 FrameTimeout aTimeout
-                   /* = FrameTimeout::FromRawMilliseconds(0) */,
-                 BlendMethod aBlendMethod /* = BlendMethod::OVER */,
-                 const Maybe<IntRect>& aBlendRect /* = Nothing() */)
+imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */)
 {
   MonitorAutoLock lock(mMonitor);
   MOZ_ASSERT(mLockCount > 0, "Image data should be locked");
@@ -621,10 +626,6 @@ imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
     mHasNoAlpha = true;
   }
 
-  mDisposalMethod = aDisposalMethod;
-  mTimeout = aTimeout;
-  mBlendMethod = aBlendMethod;
-  mBlendRect = aBlendRect;
   ImageUpdatedInternal(GetRect());
   mFinished = true;
 
@@ -844,7 +845,7 @@ imgFrame::GetAnimationData() const
   bool hasAlpha = mFormat == SurfaceFormat::B8G8R8A8;
 
   return AnimationData(data, PaletteDataLength(), mTimeout, GetRect(),
-                       mBlendMethod, mBlendRect, mDisposalMethod, hasAlpha);
+                       mBlendMethod, Some(mBlendRect), mDisposalMethod, hasAlpha);
 }
 
 void
-- 
cgit v1.2.3