Bug 1296828 (Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders. r=dholbert,edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Wed, 24 Aug 2016 14:50:49 -0700
changeset 311562 8f8d3c106410aed2252be6180c542aef3b8ad738
parent 311561 39e0894d2dfec61a563e42f93a5d050b9bc86ffb
child 311563 d28b09df98ff4a1fa02b37542b99f7a6b5191071
push idunknown
push userunknown
push dateunknown
reviewersdholbert, edwin
bugs1296828
milestone51.0a1
Bug 1296828 (Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders. r=dholbert,edwin
image/AnimationSurfaceProvider.cpp
image/AnimationSurfaceProvider.h
image/DecodedSurfaceProvider.cpp
image/DecodedSurfaceProvider.h
image/DecoderFactory.cpp
image/ISurfaceProvider.h
image/VectorImage.cpp
--- a/image/AnimationSurfaceProvider.cpp
+++ b/image/AnimationSurfaceProvider.cpp
@@ -11,24 +11,24 @@
 #include "Decoder.h"
 
 using namespace mozilla::gfx;
 
 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");
   MOZ_ASSERT(!mDecoder->IsFirstFrameDecode(),
              "Use DecodedSurfaceProvider for single-frame image decodes");
 }
 
 AnimationSurfaceProvider::~AnimationSurfaceProvider()
@@ -106,17 +106,17 @@ AnimationSurfaceProvider::LogicalSizeInB
   // into. The composited surfaces are always BGRA. Although the surface we're
   // decoding into may be paletted, and may be smaller than the real size of the
   // image, we assume the worst case here.
   // XXX(seth): Note that this is actually not accurate yet; we're storing the
   // full sequence of frames, not just the three live surfaces mentioned above.
   // 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);
 }
 
 void
 AnimationSurfaceProvider::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                                  size_t& aHeapSizeOut,
                                                  size_t& aNonHeapSizeOut)
 {
@@ -249,18 +249,18 @@ AnimationSurfaceProvider::AnnounceSurfac
   MOZ_ASSERT(mImage);
 
   // We just got the first frame; let the surface cache know. We deliberately do
   // this outside of mFramesMutex to avoid a potential deadlock with
   // AddSizeOfExcludingThis(), since otherwise we'd be acquiring mFramesMutex
   // 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
 AnimationSurfaceProvider::FinishDecoding()
 {
   mDecodingMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mImage);
   MOZ_ASSERT(mDecoder);
--- a/image/AnimationSurfaceProvider.h
+++ b/image/AnimationSurfaceProvider.h
@@ -25,18 +25,18 @@ namespace image {
 class AnimationSurfaceProvider final
   : public ISurfaceProvider
   , public IDecodingTask
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(AnimationSurfaceProvider, override)
 
   AnimationSurfaceProvider(NotNull<RasterImage*> aImage,
-                           NotNull<Decoder*> aDecoder,
-                           const SurfaceKey& aSurfaceKey);
+                           const SurfaceKey& aSurfaceKey,
+                           NotNull<Decoder*> aDecoder);
 
 
   //////////////////////////////////////////////////////////////////////////////
   // ISurfaceProvider implementation.
   //////////////////////////////////////////////////////////////////////////////
 
 public:
   // We use the ISurfaceProvider constructor of DrawableSurface to indicate that
@@ -91,17 +91,14 @@ private:
   /// The decoder used to decode this animation.
   RefPtr<Decoder> mDecoder;
 
   /// A mutex to protect mFrames. Always taken after mDecodingMutex.
   mutable Mutex mFramesMutex;
 
   /// 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
 } // namespace mozilla
 
 #endif // mozilla_image_AnimationSurfaceProvider_h
--- a/image/DecodedSurfaceProvider.cpp
+++ b/image/DecodedSurfaceProvider.cpp
@@ -11,23 +11,23 @@
 #include "Decoder.h"
 
 using namespace mozilla::gfx;
 
 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");
   MOZ_ASSERT(mDecoder->IsFirstFrameDecode(),
              "Use AnimationSurfaceProvider for animation decodes");
 }
 
 DecodedSurfaceProvider::~DecodedSurfaceProvider()
@@ -116,17 +116,17 @@ DecodedSurfaceProvider::SetLocked(bool a
   mLockRef = aLocked ? mSurface->DrawableRef()
                      : DrawableFrameRef();
 }
 
 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);
 }
 
 void
 DecodedSurfaceProvider::Run()
 {
   MutexAutoLock lock(mMutex);
 
@@ -183,18 +183,18 @@ DecodedSurfaceProvider::CheckForNewSurfa
   mSurface = mDecoder->GetCurrentFrameRef().get();
   if (!mSurface) {
     return;  // No surface yet.
   }
 
   // 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
 DecodedSurfaceProvider::FinishDecoding()
 {
   mMutex.AssertCurrentThreadOwns();
   MOZ_ASSERT(mImage);
   MOZ_ASSERT(mDecoder);
--- a/image/DecodedSurfaceProvider.h
+++ b/image/DecodedSurfaceProvider.h
@@ -24,18 +24,18 @@ namespace image {
 class DecodedSurfaceProvider final
   : public ISurfaceProvider
   , public IDecodingTask
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodedSurfaceProvider, override)
 
   DecodedSurfaceProvider(NotNull<RasterImage*> aImage,
-                         NotNull<Decoder*> aDecoder,
-                         const SurfaceKey& aSurfaceKey);
+                         const SurfaceKey& aSurfaceKey,
+                         NotNull<Decoder*> aDecoder);
 
 
   //////////////////////////////////////////////////////////////////////////////
   // ISurfaceProvider implementation.
   //////////////////////////////////////////////////////////////////////////////
 
 public:
   bool IsFinished() const override;
@@ -76,17 +76,14 @@ private:
   /// The decoder that will generate our surface. Dropped after decoding.
   RefPtr<Decoder> mDecoder;
 
   /// Our surface. Initially null until it's generated by the decoder.
   RefPtr<imgFrame> mSurface;
 
   /// 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
 } // namespace mozilla
 
 #endif // mozilla_image_DecodedSurfaceProvider_h
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -140,18 +140,18 @@ DecoderFactory::CreateDecoder(DecoderTyp
   }
 
   // Create a DecodedSurfaceProvider which will manage the decoding process and
   // make this decoder's output available in the surface cache.
   SurfaceKey surfaceKey =
     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.
   InsertOutcome outcome =
     SurfaceCache::Insert(provider, ImageKey(aImage.get()), surfaceKey);
   if (outcome != InsertOutcome::SUCCESS) {
     return nullptr;
   }
@@ -192,18 +192,18 @@ DecoderFactory::CreateAnimationDecoder(D
   }
 
   // Create an AnimationSurfaceProvider which will manage the decoding process
   // and make this decoder's output available in the surface cache.
   SurfaceKey surfaceKey =
     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.
   InsertOutcome outcome =
     SurfaceCache::Insert(provider, ImageKey(aImage.get()), surfaceKey);
   if (outcome != InsertOutcome::SUCCESS) {
     return nullptr;
   }
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -35,16 +35,24 @@ class DrawableSurface;
 class ISurfaceProvider
 {
 public:
   // Subclasses may or may not be XPCOM classes, so we just require that they
   // implement AddRef and Release.
   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();
 
   /// @return true if DrawableRef() will return a completely decoded surface.
   virtual bool IsFinished() const = 0;
 
   /// @return the number of bytes of memory this ISurfaceProvider is expected to
   /// require. Optimizations may result in lower real memory usage. Trivial
@@ -69,19 +77,25 @@ public:
   /// @return the availability state of this ISurfaceProvider, which indicates
   /// whether DrawableRef() could successfully return a surface. Should only be
   /// called from SurfaceCache code as it relies on SurfaceCache for
   /// synchronization.
   AvailabilityState& Availability() { return mAvailability; }
   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() { }
 
   /// @return an eagerly computed drawable reference to a surface. For
   /// dynamically generated animation surfaces, @aFrame specifies the 0-based
   /// index of the desired frame.
   virtual DrawableFrameRef DrawableRef(size_t aFrame) = 0;
 
@@ -94,16 +108,18 @@ protected:
   /// should avoid releasing its resources. Should only be called from
   /// SurfaceCache code as it relies on SurfaceCache for synchronization.
   virtual void SetLocked(bool aLocked) = 0;
 
 private:
   friend class CachedSurface;
   friend class DrawableSurface;
 
+  const ImageKey mImageKey;
+  const SurfaceKey mSurfaceKey;
   AvailabilityState mAvailability;
 };
 
 
 /**
  * A reference to a surface (stored in an imgFrame) that holds the surface in
  * memory, guaranteeing that it can be drawn. If you have a DrawableSurface
  * |surf| and |if (surf)| returns true, then calls to |surf->Draw()| and
@@ -215,20 +231,25 @@ ISurfaceProvider::Surface()
 /**
  * An ISurfaceProvider that stores a single surface.
  */
 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(); }
 
   size_t LogicalSizeInBytes() const override
   {
     gfx::IntSize size = mSurface->GetSize();
     return size.width * size.height * mSurface->GetBytesPerPixel();
   }
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -957,20 +957,20 @@ VectorImage::CreateSurfaceAndShow(const 
   // Take a strong reference to the frame's surface and make sure it hasn't
   // already been purged by the operating system.
   RefPtr<SourceSurface> surface = frame->GetSourceSurface();
   if (!surface) {
     return Show(svgDrawable, 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 =
     new gfxSurfaceDrawable(surface, aParams.size);
   Show(drawable, aParams);
 
   // Send out an invalidation so that surfaces that are still in use get
   // re-locked. See the discussion of the UnlockSurfaces call above.