Bug 1377252. Stop using RasterImage::IsUnlocked because it doesn't do what we want. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Thu, 29 Jun 2017 20:09:44 -0500
changeset 366781 bb6cf8e70673681299fcd7218bbba7905eb0e868
parent 366780 b60b291fce967eb0a6238fa57e95cb2525ee4290
child 366782 de6df0b30c56d103c56081d822278f94056c7931
push id92065
push usertnikkel@gmail.com
push dateFri, 30 Jun 2017 01:09:50 +0000
treeherdermozilla-inbound@bb6cf8e70673 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1377252
milestone56.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 1377252. Stop using RasterImage::IsUnlocked because it doesn't do what we want. r=aosmond We currently use RasterImage::IsUnlocked for two different purposes: 1) to determine that we can't throw away the decoded image in WillDrawOpaqueNow 2) to determine when to send the unlockeddraw notification For 1) what we want to check is mLockCount == 0. For 2) what we actually want to check is mAnimationConsumers == 0. This is because images that are in the visible list in background tabs will have mLockCount == 0 but mAnimationConsumers > 0 and if we are drawing an image we need to make sure it will be animated (mAnimationConsumers == 0 stops the animation). This is what VectorImage already does.
image/RasterImage.cpp
image/RasterImage.h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -425,17 +425,17 @@ RasterImage::WillDrawOpaqueNow()
         return false;
       }
     }
   }
 
   // If we are not locked our decoded data could get discard at any time (ie
   // between the call to this function and when we are asked to draw), so we
   // have to return false if we are unlocked.
-  if (IsUnlocked()) {
+  if (mLockCount == 0) {
     return false;
   }
 
   LookupResult result =
     SurfaceCache::LookupBestMatch(ImageKey(this),
                                   RasterSurfaceKey(mSize,
                                                    DefaultSurfaceFlags(),
                                                    PlaybackType::eStatic));
@@ -634,17 +634,17 @@ RasterImage::GetImageContainer(LayerMana
 
   int32_t maxTextureSize = aManager->GetMaxTextureSize();
   if (!mHasSize ||
       mSize.width > maxTextureSize ||
       mSize.height > maxTextureSize) {
     return nullptr;
   }
 
-  if (IsUnlocked()) {
+  if (mAnimationConsumers == 0) {
     SendOnUnlockedDraw(aFlags);
   }
 
   RefPtr<layers::ImageContainer> container = mImageContainer.get();
 
   bool mustRedecode =
     (aFlags & (FLAG_SYNC_DECODE | FLAG_SYNC_DECODE_IF_FAST)) &&
     mLastImageContainerDrawResult != DrawResult::SUCCESS &&
@@ -1446,17 +1446,17 @@ RasterImage::Draw(gfxContext* aContext,
   if (ToSurfaceFlags(aFlags) != DefaultSurfaceFlags()) {
     return DrawResult::BAD_ARGS;
   }
 
   if (!aContext) {
     return DrawResult::BAD_ARGS;
   }
 
-  if (IsUnlocked()) {
+  if (mAnimationConsumers == 0) {
     SendOnUnlockedDraw(aFlags);
   }
 
 
   // If we're not using SamplingFilter::GOOD, we shouldn't high-quality scale or
   // downscale during decode.
   uint32_t flags = aSamplingFilter == SamplingFilter::GOOD
                  ? aFlags
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -311,26 +311,16 @@ private:
                      uint32_t aWhichFrame,
                      uint32_t aFlags);
 
   Pair<DrawResult, RefPtr<layers::Image>>
     GetCurrentImage(layers::ImageContainer* aContainer, uint32_t aFlags);
 
   void UpdateImageContainer();
 
-  // We would like to just check if we have a zero lock count, but we can't do
-  // that for animated images because in EnsureAnimExists we lock the image and
-  // never unlock so that animated images always have their lock count >= 1. In
-  // that case we use our animation consumers count as a proxy for lock count.
-  bool IsUnlocked() {
-    return (mLockCount == 0 ||
-            (!gfxPrefs::ImageMemAnimatedDiscardable() &&
-             (mAnimationState && mAnimationConsumers == 0)));
-  }
-
   //////////////////////////////////////////////////////////////////////////////
   // Decoding.
   //////////////////////////////////////////////////////////////////////////////
 
   /**
    * Creates and runs a decoder, either synchronously or asynchronously
    * according to @aFlags. Decodes at the provided target size @aSize, using
    * decode flags @aFlags. Performs a single-frame decode of this image unless