Bug 1291033 (Part 1) - Ensure atomicity of ISurfaceProvider availability changes. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 01 Aug 2016 16:44:39 -0700
changeset 308322 4a2a5e36e0d32612f4ac7c488300cc9800886032
parent 308321 e1ff3a7826a38ec925771dbb2027ce81ea436fe8
child 308323 f0725e82f2869baeb01cb86b8aa6bece0ca26cd6
push id20229
push usercbook@mozilla.com
push dateFri, 05 Aug 2016 10:07:54 +0000
treeherderfx-team@2c234f5a1916 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1291033
milestone51.0a1
Bug 1291033 (Part 1) - Ensure atomicity of ISurfaceProvider availability changes. r=dholbert
image/ISurfaceProvider.h
image/SurfaceCache.cpp
image/SurfaceCache.h
--- a/image/ISurfaceProvider.h
+++ b/image/ISurfaceProvider.h
@@ -13,16 +13,17 @@
 
 #include "mozilla/Maybe.h"
 #include "mozilla/NotNull.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Variant.h"
 #include "mozilla/gfx/2D.h"
 
 #include "imgFrame.h"
+#include "SurfaceCache.h"
 
 namespace mozilla {
 namespace image {
 
 /**
  * An interface for objects which can either store a surface or dynamically
  * generate one.
  */
@@ -32,54 +33,63 @@ 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 a drawable reference to a surface.
   virtual DrawableFrameRef DrawableRef() = 0;
 
-  /// @return true if this ISurfaceProvider is acting as a placeholder, which is
-  /// to say that it doesn't have a surface and hence can't return a
-  /// non-empty DrawableFrameRef yet, but it will be able to in the future.
-  virtual bool IsPlaceholder() const = 0;
-
   /// @return true if DrawableRef() will return a completely decoded surface.
   virtual bool IsFinished() const = 0;
 
   /// @return true if this ISurfaceProvider is locked. (@see SetLocked())
   virtual bool IsLocked() const = 0;
 
   /// If @aLocked is true, hint that this ISurfaceProvider is in use and it
   /// should avoid releasing its resources.
   virtual void SetLocked(bool aLocked) = 0;
 
   /// @return the number of bytes of memory this ISurfaceProvider is expected to
   /// require. Optimizations may result in lower real memory usage. Trivial
   /// overhead is ignored.
   virtual size_t LogicalSizeInBytes() const = 0;
 
+  /// @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)
+  { }
+
   virtual ~ISurfaceProvider() { }
+
+private:
+  AvailabilityState mAvailability;
 };
 
 /**
  * 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)
-    : mSurface(aSurface)
+    : ISurfaceProvider(AvailabilityState::StartAvailable())
+    , mSurface(aSurface)
   { }
 
   DrawableFrameRef DrawableRef() override { return mSurface->DrawableRef(); }
-  bool IsPlaceholder() const override { return false; }
   bool IsFinished() const override { return mSurface->IsFinished(); }
   bool IsLocked() const override { return bool(mLockRef); }
 
   void SetLocked(bool aLocked) override
   {
     if (aLocked == IsLocked()) {
       return;  // Nothing changed.
     }
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -66,16 +66,18 @@ static StaticRefPtr<SurfaceCacheImpl> sI
  * simply an estimate of the size of the surface in bytes, but in the future it
  * may be worth taking into account the cost of rematerializing the surface as
  * well.
  */
 typedef size_t Cost;
 
 // Placeholders do not have surfaces, but need to be given a trivial cost for
 // our invariants to hold.
+// XXX(seth): This is only true of old-style placeholders inserted via
+// InsertPlaceholder().
 static const Cost sPlaceholderCost = 1;
 
 static Cost
 ComputeCost(const IntSize& aSize, uint32_t aBytesPerPixel)
 {
   MOZ_ASSERT(aBytesPerPixel == 1 || aBytesPerPixel == 4);
   return aSize.width * aSize.height * aBytesPerPixel;
 }
@@ -136,18 +138,18 @@ public:
                 const Cost         aCost,
                 const ImageKey     aImageKey,
                 const SurfaceKey&  aSurfaceKey)
     : mProvider(aProvider)
     , mCost(aCost)
     , mImageKey(aImageKey)
     , mSurfaceKey(aSurfaceKey)
   {
-    MOZ_ASSERT(!IsPlaceholder() || mCost == sPlaceholderCost,
-               "Placeholder should have trivial cost");
+    MOZ_ASSERT(aProvider || mCost == sPlaceholderCost,
+               "Old-style placeholders should have trivial cost");
     MOZ_ASSERT(mImageKey, "Must have a valid image key");
   }
 
   already_AddRefed<ISurfaceProvider> Provider() const
   {
     if (MOZ_UNLIKELY(IsPlaceholder())) {
       MOZ_ASSERT_UNREACHABLE("Shouldn't call Provider() on a placeholder");
       return nullptr;
@@ -163,17 +165,21 @@ public:
   {
     if (IsPlaceholder()) {
       return;  // Can't lock a placeholder.
     }
 
     mProvider->SetLocked(aLocked);
   }
 
-  bool IsPlaceholder() const { return !mProvider || mProvider->IsPlaceholder(); }
+  bool IsPlaceholder() const
+  {
+    return !mProvider || mProvider->Availability().IsPlaceholder();
+  }
+
   bool IsLocked() const { return !IsPlaceholder() && mProvider->IsLocked(); }
 
   ImageKey GetImageKey() const { return mImageKey; }
   SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
   CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
 
   bool IsDecoded() const
@@ -444,17 +450,18 @@ private:
 public:
   void InitMemoryReporter() { RegisterWeakMemoryReporter(this); }
 
   Mutex& GetMutex() { return mMutex; }
 
   InsertOutcome Insert(ISurfaceProvider* aProvider,
                        const Cost        aCost,
                        const ImageKey    aImageKey,
-                       const SurfaceKey& aSurfaceKey)
+                       const SurfaceKey& aSurfaceKey,
+                       bool              aSetAvailable)
   {
     // If this is a duplicate surface, refuse to replace the original.
     // XXX(seth): Calling Lookup() and then RemoveEntry() does the lookup
     // twice. We'll make this more efficient in bug 1185137.
     LookupResult result = Lookup(aImageKey, aSurfaceKey, /* aMarkUsed = */ false);
     if (MOZ_UNLIKELY(result)) {
       return InsertOutcome::FAILURE_ALREADY_PRESENT;
     }
@@ -485,16 +492,21 @@ public:
     // Locate the appropriate per-image cache. If there's not an existing cache
     // for this image, create it.
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       cache = new ImageSurfaceCache;
       mImageCaches.Put(aImageKey, cache);
     }
 
+    // If we were asked to mark the cache entry available, do so.
+    if (aSetAvailable) {
+      aProvider->Availability().SetAvailable();
+    }
+
     RefPtr<CachedSurface> surface =
       new CachedSurface(aProvider, aCost, aImageKey, aSurfaceKey);
 
     // We require that locking succeed if the image is locked and we're not
     // inserting a placeholder; the caller may need to know this to handle
     // errors correctly.
     if (cache->IsLocked() && !surface->IsPlaceholder()) {
       surface->SetLocked(true);
@@ -662,16 +674,35 @@ public:
     return aCost <= mMaxCost;
   }
 
   size_t MaximumCapacity() const
   {
     return size_t(mMaxCost);
   }
 
+  void SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider,
+                        const ImageKey             aImageKey,
+                        const SurfaceKey&          aSurfaceKey)
+  {
+    if (!aProvider->Availability().IsPlaceholder()) {
+      MOZ_ASSERT_UNREACHABLE("Calling SurfaceAvailable on non-placeholder");
+      return;
+    }
+
+    // Reinsert the provider, requesting that Insert() mark it available. This
+    // may or may not succeed, depending on whether some other decoder has
+    // beaten us to the punch and inserted a non-placeholder version of this
+    // surface first, but it's fine either way.
+    // XXX(seth): This could be implemented more efficiently; we should be able
+    // to just update our data structures without reinserting.
+    Cost cost = aProvider->LogicalSizeInBytes();
+    Insert(aProvider, cost, aImageKey, aSurfaceKey, /* aSetAvailable = */ true);
+  }
+
   void LockImage(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       cache = new ImageSurfaceCache;
       mImageCaches.Put(aImageKey, cache);
     }
 
@@ -1034,29 +1065,31 @@ SurfaceCache::Insert(NotNull<ISurfacePro
                      const SurfaceKey&          aSurfaceKey)
 {
   if (!sInstance) {
     return InsertOutcome::FAILURE;
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
   Cost cost = aProvider->LogicalSizeInBytes();
-  return sInstance->Insert(aProvider.get(), cost, aImageKey, aSurfaceKey);
+  return sInstance->Insert(aProvider.get(), cost, aImageKey, aSurfaceKey,
+                           /* aSetAvailable = */ false);
 }
 
 /* static */ InsertOutcome
 SurfaceCache::InsertPlaceholder(const ImageKey    aImageKey,
                                 const SurfaceKey& aSurfaceKey)
 {
   if (!sInstance) {
     return InsertOutcome::FAILURE;
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
-  return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey);
+  return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey,
+                           /* aSetAvailable = */ false);
 }
 
 /* static */ bool
 SurfaceCache::CanHold(const IntSize& aSize, uint32_t aBytesPerPixel /* = 4 */)
 {
   if (!sInstance) {
     return false;
   }
@@ -1071,16 +1104,29 @@ SurfaceCache::CanHold(size_t aSize)
   if (!sInstance) {
     return false;
   }
 
   return sInstance->CanHold(aSize);
 }
 
 /* static */ void
+SurfaceCache::SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider,
+                               const ImageKey             aImageKey,
+                               const SurfaceKey&          aSurfaceKey)
+{
+  if (!sInstance) {
+    return;
+  }
+
+  MutexAutoLock lock(sInstance->GetMutex());
+  sInstance->SurfaceAvailable(aProvider, aImageKey, aSurfaceKey);
+}
+
+/* static */ void
 SurfaceCache::LockImage(Image* aImageKey)
 {
   if (sInstance) {
     MutexAutoLock lock(sInstance->GetMutex());
     return sInstance->LockImage(aImageKey);
   }
 }
 
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -24,16 +24,17 @@
 #include "SVGImageContext.h"         // for SVGImageContext
 
 namespace mozilla {
 namespace image {
 
 class Image;
 class ISurfaceProvider;
 class LookupResult;
+class SurfaceCacheImpl;
 struct SurfaceMemoryCounter;
 
 /*
  * ImageKey contains the information we need to look up all SurfaceCache entries
  * for a particular image.
  */
 typedef Image* ImageKey;
 
@@ -113,16 +114,46 @@ VectorSurfaceKey(const gfx::IntSize& aSi
                  float aAnimationTime)
 {
   // We don't care about aFlags for VectorImage because none of the flags we
   // have right now influence VectorImage's rendering. If we add a new flag that
   // *does* affect how a VectorImage renders, we'll have to change this.
   return SurfaceKey(aSize, aSVGContext, aAnimationTime, DefaultSurfaceFlags());
 }
 
+
+/**
+ * AvailabilityState is used to track whether an ISurfaceProvider has a surface
+ * available or is just a placeholder.
+ *
+ * To ensure that availability changes are atomic (and especially that internal
+ * SurfaceCache code doesn't have to deal with asynchronous availability
+ * changes), an ISurfaceProvider which starts as a placeholder can only reveal
+ * the fact that it now has a surface available via a call to
+ * SurfaceCache::SurfaceAvailable().
+ */
+class AvailabilityState
+{
+public:
+  static AvailabilityState StartAvailable() { return AvailabilityState(true); }
+  static AvailabilityState StartAsPlaceholder() { return AvailabilityState(false); }
+
+  bool IsAvailable() const { return mIsAvailable; }
+  bool IsPlaceholder() const { return !mIsAvailable; }
+
+private:
+  friend class SurfaceCacheImpl;
+
+  explicit AvailabilityState(bool aIsAvailable) : mIsAvailable(aIsAvailable) { }
+
+  void SetAvailable() { mIsAvailable = true; }
+
+  bool mIsAvailable;
+};
+
 enum class InsertOutcome : uint8_t {
   SUCCESS,                 // Success (but see Insert documentation).
   FAILURE,                 // Couldn't insert (e.g., for capacity reasons).
   FAILURE_ALREADY_PRESENT  // A surface with the same key is already present.
 };
 
 /**
  * SurfaceCache is an ImageLib-global service that allows caching of decoded
@@ -279,16 +310,47 @@ struct SurfaceCache
    *         FAILURE if the placeholder could not be inserted for some reason.
    *         FAILURE_ALREADY_PRESENT if an entry with the same ImageKey and
    *           SurfaceKey already exists in the cache.
    */
   static InsertOutcome InsertPlaceholder(const ImageKey    aImageKey,
                                          const SurfaceKey& aSurfaceKey);
 
   /**
+   * Mark the cache entry @aProvider as having an available surface. This turns
+   * a placeholder cache entry into a normal cache entry. The cache entry
+   * becomes locked if the associated image is locked; otherwise, it starts in
+   * the unlocked state.
+   *
+   * If the cache entry containing @aProvider has already been evicted from the
+   * surface cache, this function has no effect.
+   *
+   * It's illegal to call this function if @aProvider is not a placeholder; by
+   * definition, non-placeholder ISurfaceProviders should have a surface
+   * available already.
+   *
+   * XXX(seth): We're currently in a transitional state where two notions of
+   * placeholder exist: the old one (placeholders are an "empty" cache entry
+   * inserted via InsertPlaceholder(), which then gets replaced by inserting a
+   * real cache entry with the same keys via Insert()) and the new one (where
+   * the same cache entry, inserted via Insert(), starts in a placeholder state
+   * and then transitions to being a normal cache entry via this function). The
+   * old mechanism will be removed in bug 1292392.
+   *
+   * @param aProvider       The cache entry that now has a surface available.
+   * @param aImageKey       Key data identifying which image the cache entry
+   *                        belongs to.
+   * @param aSurfaceKey     Key data which uniquely identifies the requested
+   *                        cache entry.
+   */
+  static void SurfaceAvailable(NotNull<ISurfaceProvider*> aProvider,
+                               const ImageKey    aImageKey,
+                               const SurfaceKey& aSurfaceKey);
+
+  /**
    * Checks if a surface of a given size could possibly be stored in the cache.
    * If CanHold() returns false, Insert() will always fail to insert the
    * surface, but the inverse is not true: Insert() may take more information
    * into account than just image size when deciding whether to cache the
    * surface, so Insert() may still fail even if CanHold() returns true.
    *
    * Use CanHold() to avoid the need to create a temporary surface when we know
    * for sure the cache can't hold it.