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 308315 4a2a5e36e0d32612f4ac7c488300cc9800886032
parent 308314 e1ff3a7826a38ec925771dbb2027ce81ea436fe8
child 308316 f0725e82f2869baeb01cb86b8aa6bece0ca26cd6
push id30531
push usercbook@mozilla.com
push dateFri, 05 Aug 2016 10:01:39 +0000
treeherdermozilla-central@d320ef56876f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1291033
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 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.