Bug 1296828 (Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders. r=dholbert,edwin

image/AnimationSurfaceProvider.cpp

@@ -16,14 +16,14 @@ namespace mozilla {
 namespace image {
 
 AnimationSurfaceProvider::AnimationSurfaceProvider(NotNull<RasterImage*> aImage,
-                                                   NotNull<Decoder*> aDecoder,
-                                                   const SurfaceKey& aSurfaceKey)
-  : ISurfaceProvider(AvailabilityState::StartAsPlaceholder())
+                                                   const SurfaceKey& aSurfaceKey,
+                                                   NotNull<Decoder*> aDecoder)
+  : ISurfaceProvider(ImageKey(aImage.get()), aSurfaceKey,
+                     AvailabilityState::StartAsPlaceholder())
   , mImage(aImage.get())
   , mDecodingMutex("AnimationSurfaceProvider::mDecoder")
   , mDecoder(aDecoder.get())
   , mFramesMutex("AnimationSurfaceProvider::mFrames")
-  , mSurfaceKey(aSurfaceKey)
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
@@ -111,7 +111,7 @@ AnimationSurfaceProvider::LogicalSizeInBytes() const
   // Unfortunately there's no way to know in advance how many frames an
   // animation has, so we really can't do better here. This will become correct
   // once bug 1289954 is complete.
-  IntSize size = mSurfaceKey.Size();
+  IntSize size = GetSurfaceKey().Size();
   return 3 * size.width * size.height * sizeof(uint32_t);
 }
 
@@ -254,8 +254,8 @@ AnimationSurfaceProvider::AnnounceSurfaceAvailable()
   // and then the surface cache lock, while the memory reporting code would
   // acquire the surface cache lock and then mFramesMutex.
   SurfaceCache::SurfaceAvailable(WrapNotNull(this),
-                                 ImageKey(mImage.get()),
-                                 mSurfaceKey);
+                                 GetImageKey(),
+                                 GetSurfaceKey());
 }
 
 void

image/AnimationSurfaceProvider.h

@@ -30,8 +30,8 @@ public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AnimationSurfaceProvider, override)
 
   AnimationSurfaceProvider(NotNull<RasterImage*> aImage,
-                           NotNull<Decoder*> aDecoder,
-                           const SurfaceKey& aSurfaceKey);
+                           const SurfaceKey& aSurfaceKey,
+                           NotNull<Decoder*> aDecoder);
 
 
   //////////////////////////////////////////////////////////////////////////////
@@ -96,9 +96,6 @@ private:
 
   /// The frames of this animation, in order.
   nsTArray<RawAccessFrameRef> mFrames;
-
-  /// The key under which we're stored as a cache entry in the surface cache.
-  const SurfaceKey mSurfaceKey;
 };
 
 } // namespace image

image/DecodedSurfaceProvider.cpp

@@ -16,13 +16,13 @@ namespace mozilla {
 namespace image {
 
 DecodedSurfaceProvider::DecodedSurfaceProvider(NotNull<RasterImage*> aImage,
-                                               NotNull<Decoder*> aDecoder,
-                                               const SurfaceKey& aSurfaceKey)
-  : ISurfaceProvider(AvailabilityState::StartAsPlaceholder())
+                                               const SurfaceKey& aSurfaceKey,
+                                               NotNull<Decoder*> aDecoder)
+  : ISurfaceProvider(ImageKey(aImage.get()), aSurfaceKey,
+                     AvailabilityState::StartAsPlaceholder())
   , mImage(aImage.get())
   , mMutex("mozilla::image::DecodedSurfaceProvider")
   , mDecoder(aDecoder.get())
-  , mSurfaceKey(aSurfaceKey)
 {
   MOZ_ASSERT(!mDecoder->IsMetadataDecode(),
              "Use MetadataDecodingTask for metadata decodes");
@@ -121,7 +121,7 @@ size_t
 DecodedSurfaceProvider::LogicalSizeInBytes() const
 {
   // Single frame images are always 32bpp.
-  IntSize size = mSurfaceKey.Size();
+  IntSize size = GetSurfaceKey().Size();
   return size.width * size.height * sizeof(uint32_t);
 }
 
@@ -188,8 +188,8 @@ DecodedSurfaceProvider::CheckForNewSurface()
   // We just got a surface for the first time; let the surface cache know.
   MOZ_ASSERT(mImage);
   SurfaceCache::SurfaceAvailable(WrapNotNull(this),
-                                 ImageKey(mImage.get()),
-                                 mSurfaceKey);
+                                 GetImageKey(),
+                                 GetSurfaceKey());
 }
 
 void

image/DecodedSurfaceProvider.h

@@ -29,8 +29,8 @@ public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodedSurfaceProvider, override)
 
   DecodedSurfaceProvider(NotNull<RasterImage*> aImage,
-                         NotNull<Decoder*> aDecoder,
-                         const SurfaceKey& aSurfaceKey);
+                         const SurfaceKey& aSurfaceKey,
+                         NotNull<Decoder*> aDecoder);
 
 
   //////////////////////////////////////////////////////////////////////////////
@@ -81,9 +81,6 @@ private:
 
   /// A drawable reference to our service; used for locking.
   DrawableFrameRef mLockRef;
-
-  /// The key under which we're stored as a cache entry in the surface cache.
-  SurfaceKey mSurfaceKey;
 };
 
 } // namespace image

image/DecoderFactory.cpp

@@ -145,8 +145,8 @@ DecoderFactory::CreateDecoder(DecoderType aType,
     RasterSurfaceKey(aOutputSize, aSurfaceFlags, PlaybackType::eStatic);
   NotNull<RefPtr<DecodedSurfaceProvider>> provider =
     WrapNotNull(new DecodedSurfaceProvider(aImage,
-                                           WrapNotNull(decoder),
-                                           surfaceKey));
+                                           surfaceKey,
+                                           WrapNotNull(decoder)));
 
   // Attempt to insert the surface provider into the surface cache right away so
   // we won't trigger any more decoders with the same parameters.
@@ -197,8 +197,8 @@ DecoderFactory::CreateAnimationDecoder(DecoderType aType,
     RasterSurfaceKey(aIntrinsicSize, aSurfaceFlags, PlaybackType::eAnimated);
   NotNull<RefPtr<AnimationSurfaceProvider>> provider =
     WrapNotNull(new AnimationSurfaceProvider(aImage,
-                                             WrapNotNull(decoder),
-                                             surfaceKey));
+                                             surfaceKey,
+                                             WrapNotNull(decoder)));
 
   // Attempt to insert the surface provider into the surface cache right away so
   // we won't trigger any more decoders with the same parameters.

image/ISurfaceProvider.h

@@ -40,6 +40,14 @@ public:
   NS_IMETHOD_(MozExternalRefCountType) AddRef() = 0;
   NS_IMETHOD_(MozExternalRefCountType) Release() = 0;
 
+  /// @return key data used for identifying which image this ISurfaceProvider is
+  /// associated with in the surface cache.
+  ImageKey GetImageKey() const { return mImageKey; }
+
+  /// @return key data used to uniquely identify this ISurfaceProvider's cache
+  /// entry in the surface cache.
+  const SurfaceKey& GetSurfaceKey() const { return mSurfaceKey; }
+
   /// @return a (potentially lazily computed) drawable reference to a surface.
   virtual DrawableSurface Surface();
 
@@ -74,9 +82,15 @@ public:
   const AvailabilityState& Availability() const { return mAvailability; }
 
 protected:
-  explicit ISurfaceProvider(AvailabilityState aAvailability)
-    : mAvailability(aAvailability)
-  { }
+  ISurfaceProvider(const ImageKey aImageKey,
+                   const SurfaceKey& aSurfaceKey,
+                   AvailabilityState aAvailability)
+    : mImageKey(aImageKey)
+    , mSurfaceKey(aSurfaceKey)
+    , mAvailability(aAvailability)
+  {
+    MOZ_ASSERT(aImageKey, "Must have a valid image key");
+  }
 
   virtual ~ISurfaceProvider() { }
 
@@ -99,6 +113,8 @@ private:
   friend class CachedSurface;
   friend class DrawableSurface;
 
+  const ImageKey mImageKey;
+  const SurfaceKey mSurfaceKey;
   AvailabilityState mAvailability;
 };
 
@@ -220,10 +236,15 @@ class SimpleSurfaceProvider final : public ISurfaceProvider
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SimpleSurfaceProvider, override)
 
-  explicit SimpleSurfaceProvider(NotNull<imgFrame*> aSurface)
-    : ISurfaceProvider(AvailabilityState::StartAvailable())
+  SimpleSurfaceProvider(const ImageKey aImageKey,
+                        const SurfaceKey& aSurfaceKey,
+                        NotNull<imgFrame*> aSurface)
+    : ISurfaceProvider(aImageKey, aSurfaceKey,
+                       AvailabilityState::StartAvailable())
     , mSurface(aSurface)
-  { }
+  {
+    MOZ_ASSERT(aSurfaceKey.Size() == mSurface->GetSize());
+  }
 
   bool IsFinished() const override { return mSurface->IsFinished(); }
 

image/VectorImage.cpp

@@ -962,10 +962,10 @@ VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams)
   }
 
   // Attempt to cache the frame.
+  SurfaceKey surfaceKey = VectorSurfaceKey(aParams.size, aParams.svgContext);
   NotNull<RefPtr<ISurfaceProvider>> provider =
-    WrapNotNull(new SimpleSurfaceProvider(frame));
-  SurfaceCache::Insert(provider, ImageKey(this),
-                       VectorSurfaceKey(aParams.size, aParams.svgContext));
+    WrapNotNull(new SimpleSurfaceProvider(ImageKey(this), surfaceKey, frame));
+  SurfaceCache::Insert(provider, ImageKey(this), surfaceKey);
 
   // Draw.
   RefPtr<gfxDrawable> drawable =