Bug 1296762 (Part 3) - Use NotNull for all CachedSurfaces in SurfaceCache. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Fri, 19 Aug 2016 15:09:52 -0700
changeset 354796 4033113ab84efb2dc91552fd395aaea7d55584fc
parent 354795 ef60a93ad25aa956e3ff35b32478d31dd748556e
child 354797 024e7dc7a219e3f276bc2245746bbad67a574ea9
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1296762
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 1296762 (Part 3) - Use NotNull for all CachedSurfaces in SurfaceCache. r=dholbert
image/SurfaceCache.cpp
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -83,41 +83,39 @@ ComputeCost(const IntSize& aSize, uint32
  *
  * To make usage of the weak pointer safe, SurfaceCacheImpl always calls
  * StartTracking after a surface is stored in the cache and StopTracking before
  * it is removed.
  */
 class CostEntry
 {
 public:
-  CostEntry(CachedSurface* aSurface, Cost aCost)
+  CostEntry(NotNull<CachedSurface*> aSurface, Cost aCost)
     : mSurface(aSurface)
     , mCost(aCost)
-  {
-    MOZ_ASSERT(aSurface, "Must have a surface");
-  }
+  { }
 
-  CachedSurface* GetSurface() const { return mSurface; }
+  NotNull<CachedSurface*> Surface() const { return mSurface; }
   Cost GetCost() const { return mCost; }
 
   bool operator==(const CostEntry& aOther) const
   {
     return mSurface == aOther.mSurface &&
            mCost == aOther.mCost;
   }
 
   bool operator<(const CostEntry& aOther) const
   {
     return mCost < aOther.mCost ||
            (mCost == aOther.mCost && mSurface < aOther.mSurface);
   }
 
 private:
-  CachedSurface* mSurface;
-  Cost           mCost;
+  NotNull<CachedSurface*> mSurface;
+  Cost                    mCost;
 };
 
 /**
  * A CachedSurface associates a surface with a key that uniquely identifies that
  * surface.
  */
 class CachedSurface
 {
@@ -158,32 +156,30 @@ public:
   }
 
   bool IsLocked() const { return !IsPlaceholder() && mProvider->IsLocked(); }
   bool IsPlaceholder() const { return mProvider->Availability().IsPlaceholder(); }
   bool IsDecoded() const { return !IsPlaceholder() && mProvider->IsFinished(); }
 
   ImageKey GetImageKey() const { return mImageKey; }
   SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
-  CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
+  CostEntry GetCostEntry() { return image::CostEntry(WrapNotNull(this), mCost); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
 
   // A helper type used by SurfaceCacheImpl::CollectSizeOfSurfaces.
   struct MOZ_STACK_CLASS SurfaceMemoryReport
   {
     SurfaceMemoryReport(nsTArray<SurfaceMemoryCounter>& aCounters,
                         MallocSizeOf                    aMallocSizeOf)
       : mCounters(aCounters)
       , mMallocSizeOf(aMallocSizeOf)
     { }
 
-    void Add(CachedSurface* aCachedSurface)
+    void Add(NotNull<CachedSurface*> aCachedSurface)
     {
-      MOZ_ASSERT(aCachedSurface, "Should have a CachedSurface");
-
       SurfaceMemoryCounter counter(aCachedSurface->GetSurfaceKey(),
                                    aCachedSurface->IsLocked());
 
       if (aCachedSurface->IsPlaceholder()) {
         return;
       }
 
       // Record the memory used by the ISurfaceProvider. This may not have a
@@ -236,27 +232,25 @@ public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(ImageSurfaceCache)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ImageSurfaceCache)
 
   typedef
     nsRefPtrHashtable<nsGenericHashKey<SurfaceKey>, CachedSurface> SurfaceTable;
 
   bool IsEmpty() const { return mSurfaces.Count() == 0; }
 
-  void Insert(const SurfaceKey& aKey, CachedSurface* aSurface)
+  void Insert(const SurfaceKey& aKey, NotNull<CachedSurface*> aSurface)
   {
-    MOZ_ASSERT(aSurface, "Should have a surface");
     MOZ_ASSERT(!mLocked || aSurface->IsPlaceholder() || aSurface->IsLocked(),
                "Inserting an unlocked surface for a locked image");
     mSurfaces.Put(aKey, aSurface);
   }
 
-  void Remove(CachedSurface* aSurface)
+  void Remove(NotNull<CachedSurface*> aSurface)
   {
-    MOZ_ASSERT(aSurface, "Should have a surface");
     MOZ_ASSERT(mSurfaces.GetWeak(aSurface->GetSurfaceKey()),
         "Should not be removing a surface we don't have");
 
     mSurfaces.Remove(aSurface->GetSurfaceKey());
   }
 
   already_AddRefed<CachedSurface> Lookup(const SurfaceKey& aSurfaceKey)
   {
@@ -273,17 +267,17 @@ public:
     mSurfaces.Get(aIdealKey, getter_AddRefs(exactMatch));
     if (exactMatch && exactMatch->IsDecoded()) {
       return MakePair(exactMatch.forget(), MatchType::EXACT);
     }
 
     // There's no perfect match, so find the best match we can.
     RefPtr<CachedSurface> bestMatch;
     for (auto iter = ConstIter(); !iter.Done(); iter.Next()) {
-      CachedSurface* current = iter.UserData();
+      NotNull<CachedSurface*> current = WrapNotNull(iter.UserData());
       const SurfaceKey& currentKey = current->GetSurfaceKey();
 
       // We never match a placeholder.
       if (current->IsPlaceholder()) {
         continue;
       }
       // Matching the playback type and SVG context is required.
       if (currentKey.Playback() != aIdealKey.Playback() ||
@@ -451,34 +445,34 @@ public:
       return InsertOutcome::FAILURE;
     }
 
     // Remove elements in order of cost until we can fit this in the cache. Note
     // that locked surfaces aren't in mCosts, so we never remove them here.
     while (aCost > mAvailableCost) {
       MOZ_ASSERT(!mCosts.IsEmpty(),
                  "Removed everything and it still won't fit");
-      Remove(mCosts.LastElement().GetSurface());
+      Remove(mCosts.LastElement().Surface());
     }
 
     // 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);
+    NotNull<RefPtr<CachedSurface>> surface =
+      WrapNotNull(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);
       if (!surface->IsLocked()) {
         return InsertOutcome::FAILURE;
@@ -488,19 +482,18 @@ public:
     // Insert.
     MOZ_ASSERT(aCost <= mAvailableCost, "Inserting despite too large a cost");
     cache->Insert(aSurfaceKey, surface);
     StartTracking(surface);
 
     return InsertOutcome::SUCCESS;
   }
 
-  void Remove(CachedSurface* aSurface)
+  void Remove(NotNull<CachedSurface*> aSurface)
   {
-    MOZ_ASSERT(aSurface, "Should have a surface");
     ImageKey imageKey = aSurface->GetImageKey();
 
     RefPtr<ImageSurfaceCache> cache = GetImageCache(imageKey);
     MOZ_ASSERT(cache, "Shouldn't try to remove a surface with no image cache");
 
     // If the surface was not a placeholder, tell its image that we discarded it.
     if (!aSurface->IsPlaceholder()) {
       static_cast<Image*>(imageKey)->OnSurfaceDiscarded();
@@ -511,17 +504,17 @@ public:
 
     // Remove the per-image cache if it's unneeded now. (Keep it if the image is
     // locked, since the per-image cache is where we store that state.)
     if (cache->IsEmpty() && !cache->IsLocked()) {
       mImageCaches.Remove(imageKey);
     }
   }
 
-  void StartTracking(CachedSurface* aSurface)
+  void StartTracking(NotNull<CachedSurface*> aSurface)
   {
     CostEntry costEntry = aSurface->GetCostEntry();
     MOZ_ASSERT(costEntry.GetCost() <= mAvailableCost,
                "Cost too large and the caller didn't catch it");
 
     mAvailableCost -= costEntry.GetCost();
 
     if (aSurface->IsLocked()) {
@@ -530,19 +523,18 @@ public:
     } else {
       mCosts.InsertElementSorted(costEntry);
       // This may fail during XPCOM shutdown, so we need to ensure the object is
       // tracked before calling RemoveObject in StopTracking.
       mExpirationTracker.AddObject(aSurface);
     }
   }
 
-  void StopTracking(CachedSurface* aSurface)
+  void StopTracking(NotNull<CachedSurface*> aSurface)
   {
-    MOZ_ASSERT(aSurface, "Should have a surface");
     CostEntry costEntry = aSurface->GetCostEntry();
 
     if (aSurface->IsLocked()) {
       MOZ_ASSERT(mLockedCost >= costEntry.GetCost(), "Costs don't balance");
       mLockedCost -= costEntry.GetCost();
       // XXX(seth): It'd be nice to use an O(log n) lookup here. This is O(n).
       MOZ_ASSERT(!mCosts.Contains(costEntry),
                  "Shouldn't have a cost entry for a locked surface");
@@ -584,22 +576,22 @@ public:
     if (surface->IsPlaceholder()) {
       return LookupResult(MatchType::PENDING);
     }
 
     DrawableSurface drawableSurface = surface->GetDrawableSurface();
     if (!drawableSurface) {
       // The surface was released by the operating system. Remove the cache
       // entry as well.
-      Remove(surface);
+      Remove(WrapNotNull(surface));
       return LookupResult(MatchType::NOT_FOUND);
     }
 
     if (aMarkUsed) {
-      MarkUsed(surface, cache);
+      MarkUsed(WrapNotNull(surface), WrapNotNull(cache));
     }
 
     MOZ_ASSERT(surface->GetSurfaceKey() == aSurfaceKey,
                "Lookup() not returning an exact match?");
     return LookupResult(Move(drawableSurface), MatchType::EXACT);
   }
 
   LookupResult LookupBestMatch(const ImageKey         aImageKey,
@@ -629,29 +621,29 @@ public:
 
       drawableSurface = surface->GetDrawableSurface();
       if (drawableSurface) {
         break;
       }
 
       // The surface was released by the operating system. Remove the cache
       // entry as well.
-      Remove(surface);
+      Remove(WrapNotNull(surface));
     }
 
     MOZ_ASSERT_IF(matchType == MatchType::EXACT,
                   surface->GetSurfaceKey() == aSurfaceKey);
     MOZ_ASSERT_IF(matchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
                   matchType == MatchType::SUBSTITUTE_BECAUSE_PENDING,
       surface->GetSurfaceKey().SVGContext() == aSurfaceKey.SVGContext() &&
       surface->GetSurfaceKey().Playback() == aSurfaceKey.Playback() &&
       surface->GetSurfaceKey().Flags() == aSurfaceKey.Flags());
 
     if (matchType == MatchType::EXACT) {
-      MarkUsed(surface, cache);
+      MarkUsed(WrapNotNull(surface), WrapNotNull(cache));
     }
 
     return LookupResult(Move(drawableSurface), matchType);
   }
 
   bool CanHold(const Cost aCost) const
   {
     return aCost <= mMaxCost;
@@ -698,59 +690,59 @@ public:
   void UnlockImage(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache || !cache->IsLocked()) {
       return;  // Already unlocked.
     }
 
     cache->SetLocked(false);
-    DoUnlockSurfaces(cache);
+    DoUnlockSurfaces(WrapNotNull(cache));
   }
 
   void UnlockEntries(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache || !cache->IsLocked()) {
       return;  // Already unlocked.
     }
 
     // (Note that we *don't* unlock the per-image cache here; that's the
     // difference between this and UnlockImage.)
-    DoUnlockSurfaces(cache);
+    DoUnlockSurfaces(WrapNotNull(cache));
   }
 
   void RemoveImage(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       return;  // No cached surfaces for this image, so nothing to do.
     }
 
     // Discard all of the cached surfaces for this image.
     // XXX(seth): This is O(n^2) since for each item in the cache we are
     // removing an element from the costs array. Since n is expected to be
     // small, performance should be good, but if usage patterns change we should
     // change the data structure used for mCosts.
     for (auto iter = cache->ConstIter(); !iter.Done(); iter.Next()) {
-      StopTracking(iter.UserData());
+      StopTracking(WrapNotNull(iter.UserData()));
     }
 
     // The per-image cache isn't needed anymore, so remove it as well.
     // This implicitly unlocks the image if it was locked.
     mImageCaches.Remove(aImageKey);
   }
 
   void DiscardAll()
   {
     // Remove in order of cost because mCosts is an array and the other data
     // structures are all hash tables. Note that locked surfaces are not
     // removed, since they aren't present in mCosts.
     while (!mCosts.IsEmpty()) {
-      Remove(mCosts.LastElement().GetSurface());
+      Remove(mCosts.LastElement().Surface());
     }
   }
 
   void DiscardForMemoryPressure()
   {
     // Compute our discardable cost. Since locked surfaces aren't discardable,
     // we exclude them.
     const Cost discardableCost = (mMaxCost - mAvailableCost) - mLockedCost;
@@ -766,21 +758,21 @@ public:
       MOZ_ASSERT_UNREACHABLE("Target cost is more than we can discard");
       DiscardAll();
       return;
     }
 
     // Discard surfaces until we've reduced our cost to our target cost.
     while (mAvailableCost < targetCost) {
       MOZ_ASSERT(!mCosts.IsEmpty(), "Removed everything and still not done");
-      Remove(mCosts.LastElement().GetSurface());
+      Remove(mCosts.LastElement().Surface());
     }
   }
 
-  void LockSurface(CachedSurface* aSurface)
+  void LockSurface(NotNull<CachedSurface*> aSurface)
   {
     if (aSurface->IsPlaceholder() || aSurface->IsLocked()) {
       return;
     }
 
     StopTracking(aSurface);
 
     // Lock the surface. This can fail.
@@ -824,17 +816,17 @@ public:
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       return;  // No surfaces for this image.
     }
 
     // Report all surfaces in the per-image cache.
     CachedSurface::SurfaceMemoryReport report(aCounters, aMallocSizeOf);
     for (auto iter = cache->ConstIter(); !iter.Done(); iter.Next()) {
-      report.Add(iter.UserData());
+      report.Add(WrapNotNull(iter.UserData()));
     }
   }
 
 private:
   already_AddRefed<ImageSurfaceCache> GetImageCache(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> imageCache;
     mImageCaches.Get(aImageKey, getter_AddRefs(imageCache));
@@ -846,30 +838,31 @@ private:
   // publicly because we permit multithreaded access to the surface cache, which
   // means that the result would be meaningless: another thread could insert a
   // surface or lock an image at any time.
   bool CanHoldAfterDiscarding(const Cost aCost) const
   {
     return aCost <= mMaxCost - mLockedCost;
   }
 
-  void MarkUsed(CachedSurface* aSurface, ImageSurfaceCache* aCache)
+  void MarkUsed(NotNull<CachedSurface*> aSurface,
+                NotNull<ImageSurfaceCache*> aCache)
   {
     if (aCache->IsLocked()) {
       LockSurface(aSurface);
     } else {
       mExpirationTracker.MarkUsed(aSurface);
     }
   }
 
-  void DoUnlockSurfaces(ImageSurfaceCache* aCache)
+  void DoUnlockSurfaces(NotNull<ImageSurfaceCache*> aCache)
   {
     // Unlock all the surfaces the per-image cache is holding.
     for (auto iter = aCache->ConstIter(); !iter.Done(); iter.Next()) {
-      CachedSurface* surface = iter.UserData();
+      NotNull<CachedSurface*> surface = WrapNotNull(iter.UserData());
       if (surface->IsPlaceholder() || !surface->IsLocked()) {
         continue;
       }
       StopTracking(surface);
       surface->SetLocked(false);
       StartTracking(surface);
     }
   }
@@ -882,32 +875,32 @@ private:
       return;  // No cached surfaces for this image.
     }
 
     RefPtr<CachedSurface> surface = cache->Lookup(aSurfaceKey);
     if (!surface) {
       return;  // Lookup in the per-image cache missed.
     }
 
-    Remove(surface);
+    Remove(WrapNotNull(surface));
   }
 
   struct SurfaceTracker : public nsExpirationTracker<CachedSurface, 2>
   {
     explicit SurfaceTracker(uint32_t aSurfaceCacheExpirationTimeMS)
       : nsExpirationTracker<CachedSurface, 2>(aSurfaceCacheExpirationTimeMS,
                                               "SurfaceTracker")
     { }
 
   protected:
     virtual void NotifyExpired(CachedSurface* aSurface) override
     {
       if (sInstance) {
         MutexAutoLock lock(sInstance->GetMutex());
-        sInstance->Remove(aSurface);
+        sInstance->Remove(WrapNotNull(aSurface));
       }
     }
   };
 
   struct MemoryPressureObserver : public nsIObserver
   {
     NS_DECL_ISUPPORTS