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 352847 8f8d3c106410aed2252be6180c542aef3b8ad738
parent 352846 39e0894d2dfec61a563e42f93a5d050b9bc86ffb
child 352848 d28b09df98ff4a1fa02b37542b99f7a6b5191071
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, edwin
bugs1296828
milestone51.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 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.