Bug 1462355 - Part 9. Lock animated imgFrame objects at creation rather than deferring. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 29 May 2018 08:36:13 -0400
changeset 420282 9b516954e1031202b00b924accfb0861a973986f
parent 420281 8d412f560489b690870257077e15f1b1ad3d7b75
child 420283 dfbe62f4ab7535c1388586e309bd01e339ef3142
push id34069
push usernerli@mozilla.com
push dateTue, 29 May 2018 21:42:06 +0000
treeherdermozilla-central@89d79c2258be [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1462355
milestone62.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 1462355 - Part 9. Lock animated imgFrame objects at creation rather than deferring. r=tnikkel
image/Decoder.cpp
image/imgFrame.cpp
image/imgFrame.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -351,32 +351,28 @@ Decoder::AllocateFrameInternal(const gfx
   RawAccessFrameRef ref = frame->RawAccessRef();
   if (!ref) {
     frame->Abort();
     return RawAccessFrameRef();
   }
 
   if (frameNum == 1) {
     MOZ_ASSERT(aPreviousFrame, "Must provide a previous frame when animated");
-    aPreviousFrame->SetRawAccessOnly();
-
     // If we dispose of the first frame by clearing it, then the first frame's
     // refresh area is all of itself.
     // RESTORE_PREVIOUS is invalid (assumed to be DISPOSE_CLEAR).
     DisposalMethod prevDisposal = aPreviousFrame->GetDisposalMethod();
     if (prevDisposal == DisposalMethod::CLEAR ||
         prevDisposal == DisposalMethod::CLEAR_ALL ||
         prevDisposal == DisposalMethod::RESTORE_PREVIOUS) {
       mFirstFrameRefreshArea = aPreviousFrame->GetRect();
     }
   }
 
   if (frameNum > 0) {
-    ref->SetRawAccessOnly();
-
     // Some GIFs are huge but only have a small area that they animate. We only
     // need to refresh that small area when frame 0 comes around again.
     mFirstFrameRefreshArea.UnionRect(mFirstFrameRefreshArea, frame->GetRect());
   }
 
   mFrameCount++;
 
   return ref;
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -309,16 +309,23 @@ imgFrame::InitForDecoder(const nsIntSize
 
     if (!ClearSurface(mRawSurface, mFrameRect.Size(), mFormat)) {
       NS_WARNING("Could not clear allocated buffer");
       mAborted = true;
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
 
+  if (aAnimParams) {
+    // We never want to unlock animated frames because we need the raw frame
+    // buffer for blending with future frames. Adding an extra unmatched lock
+    // here will guarantee that.
+    ++mLockCount;
+  }
+
   return NS_OK;
 }
 
 nsresult
 imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
                            const nsIntSize& aSize,
                            const SurfaceFormat aFormat,
                            SamplingFilter aSamplingFilter,
@@ -495,26 +502,16 @@ imgFrame::DrawableRef()
 }
 
 RawAccessFrameRef
 imgFrame::RawAccessRef(bool aOnlyFinished /*= false*/)
 {
   return RawAccessFrameRef(this, aOnlyFinished);
 }
 
-void
-imgFrame::SetRawAccessOnly()
-{
-  AssertImageDataLocked();
-
-  // Lock our data and throw away the key.
-  LockImageData(false);
-}
-
-
 imgFrame::SurfaceWithFormat
 imgFrame::SurfaceForDrawing(bool               aDoPartialDecode,
                             bool               aDoTile,
                             ImageRegion&       aRegion,
                             SourceSurface*     aSurface)
 {
   MOZ_ASSERT(NS_IsMainThread());
   mMonitor.AssertCurrentThreadOwns();
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -98,28 +98,16 @@ public:
   /**
    * Create a RawAccessFrameRef for the frame.
    *
    * @param aOnlyFinished If true, only return a valid RawAccessFrameRef if
    *                      imgFrame::Finish has been called.
    */
   RawAccessFrameRef RawAccessRef(bool aOnlyFinished = false);
 
-  /**
-   * Make this imgFrame permanently available for raw access.
-   *
-   * This is irrevocable, and should be avoided whenever possible, since it
-   * prevents this imgFrame from being optimized and makes it impossible for its
-   * volatile buffer to be freed.
-   *
-   * It is an error to call this without already holding a RawAccessFrameRef to
-   * this imgFrame.
-   */
-  void SetRawAccessOnly();
-
   bool Draw(gfxContext* aContext, const ImageRegion& aRegion,
             SamplingFilter aSamplingFilter, uint32_t aImageFlags,
             float aOpacity);
 
   nsresult ImageUpdated(const nsIntRect& aUpdateRect);
 
   /**
    * Mark this imgFrame as completely decoded, and set final options.