Bug 1536997 - Fix broken assert for recycling animated images with WebRender. r=kats

By default, recycling frames for animated images is enabled. However the
first frame is never recycled because we may need the first frame
immediately if the animation is reset. If only the first frame of an
animation is displayed before shutting down a tab, then it will not
correctly register with the layer state manager prior to its
destruction. This trips an assert incorrectly.

Now we just always register with the layer state manager if recycling is
enabled, and never if recycling is disabled. This allows us to remove
from state information at the cost of requiring a restart to toggle to
recycling pref (which is almost never done now that the feature is
stable.)

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

gfx/layers/ipc/SharedSurfacesChild.cpp

@@ -13,6 +13,7 @@
 #include "mozilla/layers/SourceSurfaceSharedData.h"
 #include "mozilla/layers/WebRenderBridgeChild.h"
 #include "mozilla/layers/RenderRootStateManager.h"
+#include "mozilla/StaticPrefs.h"  // for StaticPrefs
 #include "mozilla/SystemGroup.h"  // for SystemGroup
 
 namespace mozilla {
@@ -480,18 +481,15 @@ nsresult SharedSurfacesChild::UpdateAnimation(ImageContainer* aContainer,
 
 AnimationImageKeyData::AnimationImageKeyData(RenderRootStateManager* aManager,
                                              const wr::ImageKey& aImageKey)
-    : SharedSurfacesChild::ImageKeyData(aManager, aImageKey),
-      mRecycling(false) {}
+    : SharedSurfacesChild::ImageKeyData(aManager, aImageKey) {}
 
 AnimationImageKeyData::AnimationImageKeyData(AnimationImageKeyData&& aOther)
     : SharedSurfacesChild::ImageKeyData(std::move(aOther)),
-      mPendingRelease(std::move(aOther.mPendingRelease)),
-      mRecycling(aOther.mRecycling) {}
+      mPendingRelease(std::move(aOther.mPendingRelease)) {}
 
 AnimationImageKeyData& AnimationImageKeyData::operator=(
     AnimationImageKeyData&& aOther) {
   mPendingRelease = std::move(aOther.mPendingRelease);
-  mRecycling = aOther.mRecycling;
   SharedSurfacesChild::ImageKeyData::operator=(std::move(aOther));
   return *this;
 }
@@ -517,7 +515,7 @@ void SharedSurfacesAnimation::Destroy() {
 
   for (const auto& entry : mKeys) {
     MOZ_ASSERT(!entry.mManager->IsDestroyed());
-    if (entry.mRecycling) {
+    if (StaticPrefs::ImageAnimatedDecodeOnDemandRecycle()) {
       entry.mManager->DeregisterAsyncAnimation(entry.mImageKey);
     }
     entry.mManager->AddImageKeyForDiscard(entry.mImageKey);
@@ -533,11 +531,7 @@ void SharedSurfacesAnimation::HoldSurfaceForRecycling(
     return;
   }
 
-  if (!aEntry.mRecycling) {
-    aEntry.mManager->RegisterAsyncAnimation(aEntry.mImageKey, this);
-    aEntry.mRecycling = true;
-  }
-
+  MOZ_ASSERT(StaticPrefs::ImageAnimatedDecodeOnDemandRecycle());
   aEntry.mPendingRelease.AppendElement(aParentSurface);
 }
 
@@ -626,6 +620,10 @@ nsresult SharedSurfacesAnimation::UpdateKey(
 
   if (!found) {
     aKey = aManager->WrBridge()->GetNextImageKey();
+    if (StaticPrefs::ImageAnimatedDecodeOnDemandRecycle()) {
+      aManager->RegisterAsyncAnimation(aKey, this);
+    }
+
     AnimationImageKeyData data(aManager, aKey);
     HoldSurfaceForRecycling(data, aParentSurface, aSurface);
     mKeys.AppendElement(std::move(data));

gfx/layers/ipc/SharedSurfacesChild.h

@@ -200,7 +200,6 @@ class AnimationImageKeyData final : public SharedSurfacesChild::ImageKeyData {
   AnimationImageKeyData& operator=(AnimationImageKeyData&& aOther);
 
   AutoTArray<RefPtr<gfx::SourceSurface>, 2> mPendingRelease;
-  bool mRecycling;
 };
 
 /**

modules/libpref/init/StaticPrefList.h

@@ -2859,10 +2859,10 @@ VARCACHE_PREF(
 )
 
 VARCACHE_PREF(
-  Live,
+  Once,
   "image.animated.decode-on-demand.recycle",
   ImageAnimatedDecodeOnDemandRecycle,
-  RelaxedAtomicBool, false
+  bool, false
 )
 
 VARCACHE_PREF(