Bug 1765911 - Support cloning Patterns with internal weak references. r=aosmond

Currently calling Pattern::Clone() produces a heap-allocated Pattern with RefPtrs
to any internal resources such as SourceSurfaces or GradientStops. While this is
okay if the Pattern is transient, this causes problems if the clone is long-lived
and causes via the RefPtr the stored resource to stay alive, even when there are
no other external references to the resource.

In the case of DrawTargetWebgl's PathCache, we need to be able to create Pattern
clones that have internal weak references, while also still supporting the original
use-case of strong references. To this end we template-ize some of the Pattern
derivatives so that we can make either strong or weak versions according to use-case.
This case way we store a weak clone of a Pattern in the PathCache key, the reference
will automatically invalidate itself when all other external strong references to it
go away, without having to do external crawling of the PathCache to constantly prune
the last reference manually.

Differential Revision: https://phabricator.services.mozilla.com/D144381

dom/canvas/DrawTargetWebgl.cpp

@@ -1807,7 +1807,7 @@ already_AddRefed<PathCacheEntry> PathCache::FindOrInsertEntry(
   }
   Pattern* pattern = nullptr;
   if (aPattern) {
-    pattern = aPattern->Clone();
+    pattern = aPattern->CloneWeak();
     if (!pattern) {
       return nullptr;
     }

dom/canvas/DrawTargetWebglInternal.h

@@ -72,6 +72,8 @@ class CacheEntry : public RefCounted<CacheEntry> {
   const IntRect& GetBounds() const { return mBounds; }
   HashNumber GetHash() const { return mHash; }
 
+  virtual bool IsValid() const { return true; }
+
  protected:
   virtual void RemoveFromList() = 0;
 
@@ -173,7 +175,9 @@ class TextureHandle : public RefCounted<TextureHandle>,
   void SetCacheEntry(const RefPtr<CacheEntry>& aEntry) { mCacheEntry = aEntry; }
 
   // Note as used if there is corresponding surface or cache entry.
-  bool IsUsed() const { return mSurface || mCacheEntry; }
+  bool IsUsed() const {
+    return mSurface || (mCacheEntry && mCacheEntry->IsValid());
+  }
 
  private:
   bool mValid = true;
@@ -360,6 +364,9 @@ class PathCacheEntry : public CacheEntryImpl<PathCacheEntry> {
 
   const Point& GetOrigin() const { return mOrigin; }
 
+  // Valid if either a mask (no pattern) or there is valid pattern.
+  bool IsValid() const override { return !mPattern || mPattern->IsValid(); }
+
  private:
   // The actual path geometry supplied
   SkPath mPath;

gfx/2d/2D.h

@@ -251,7 +251,7 @@ struct ShadowOptions {
  * matching DrawTarget. Not adhering to this condition will make a draw call
  * fail.
  */
-class GradientStops : public external::AtomicRefCounted<GradientStops> {
+class GradientStops : public SupportsThreadSafeWeakPtr<GradientStops> {
  public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(GradientStops)
   virtual ~GradientStops() = default;
@@ -275,8 +275,15 @@ class Pattern {
 
   virtual PatternType GetType() const = 0;
 
-  /** Instantiate a new clone with the same pattern type and values. */
+  /** Instantiate a new clone with the same pattern type and values. Any
-  virtual Pattern* Clone() const { return nullptr; }
+   * internal strong references will be converted to weak references. */
+  virtual Pattern* CloneWeak() const { return nullptr; }
+
+  /** Whether the pattern holds an internal weak reference. */
+  virtual bool IsWeak() const { return false; }
+
+  /** Whether any internal weak references still point to a target. */
+  virtual bool IsValid() const { return true; }
 
   /** Determine if the pattern type and values exactly match. */
   virtual bool operator==(const Pattern& aOther) const = 0;
@@ -285,6 +292,19 @@ class Pattern {
 
  protected:
   Pattern() = default;
+
+  // Utility functions to check if a weak reference is still valid.
+  template <typename T>
+  static inline bool IsRefValid(const RefPtr<T>& aPtr) {
+    // RefPtrs are always valid.
+    return true;
+  }
+
+  template <typename T>
+  static inline bool IsRefValid(const ThreadSafeWeakPtr<T>& aPtr) {
+    // Weak refs are only valid if they aren't dead.
+    return !aPtr.IsDead();
+  }
 };
 
 class ColorPattern : public Pattern {
@@ -295,7 +315,7 @@ class ColorPattern : public Pattern {
 
   PatternType GetType() const override { return PatternType::COLOR; }
 
-  Pattern* Clone() const override { return new ColorPattern(mColor); }
+  Pattern* CloneWeak() const override { return new ColorPattern(mColor); }
 
   bool operator==(const Pattern& aOther) const override {
     if (aOther.GetType() != PatternType::COLOR) {
@@ -313,150 +333,201 @@ class ColorPattern : public Pattern {
  * stored in a separate object and are backend dependent. This class itself
  * may be used on the stack.
  */
-class LinearGradientPattern : public Pattern {
+template <template <typename> typename REF = RefPtr>
+class LinearGradientPatternT : public Pattern {
+  typedef LinearGradientPatternT<ThreadSafeWeakPtr> Weak;
+
  public:
   /// For constructor parameter description, see member data documentation.
-  LinearGradientPattern(const Point& aBegin, const Point& aEnd,
+  LinearGradientPatternT(const Point& aBegin, const Point& aEnd,
-                        already_AddRefed<GradientStops> aStops,
+                         RefPtr<GradientStops> aStops,
-                        const Matrix& aMatrix = Matrix())
+                         const Matrix& aMatrix = Matrix())
-      : mBegin(aBegin), mEnd(aEnd), mStops(aStops), mMatrix(aMatrix) {}
+      : mBegin(aBegin),
+        mEnd(aEnd),
+        mStops(std::move(aStops)),
+        mMatrix(aMatrix) {}
 
   PatternType GetType() const override { return PatternType::LINEAR_GRADIENT; }
 
-  Pattern* Clone() const override {
+  Pattern* CloneWeak() const override {
-    return new LinearGradientPattern(mBegin, mEnd, do_AddRef(mStops), mMatrix);
+    return new Weak(mBegin, mEnd, do_AddRef(mStops), mMatrix);
+  }
+
+  bool IsWeak() const override {
+    return std::is_same<decltype(*this), Weak>::value;
+  }
+
+  bool IsValid() const override { return IsRefValid(mStops); }
+
+  template <template <typename> typename T>
+  bool operator==(const LinearGradientPatternT<T>& aOther) const {
+    return mBegin == aOther.mBegin && mEnd == aOther.mEnd &&
+           mStops == aOther.mStops && mMatrix.ExactlyEquals(aOther.mMatrix);
   }
 
   bool operator==(const Pattern& aOther) const override {
     if (aOther.GetType() != PatternType::LINEAR_GRADIENT) {
       return false;
     }
-    const LinearGradientPattern& other =
+    return aOther.IsWeak()
-        static_cast<const LinearGradientPattern&>(aOther);
+               ? *this == static_cast<const Weak&>(aOther)
-    return mBegin == other.mBegin && mEnd == other.mEnd &&
+               : *this == static_cast<const LinearGradientPatternT<>&>(aOther);
-           mStops == other.mStops && mMatrix.ExactlyEquals(other.mMatrix);
   }
 
-  Point mBegin;  //!< Start of the linear gradient
+  Point mBegin;              //!< Start of the linear gradient
-  Point mEnd;    /**< End of the linear gradient - NOTE: In the case
+  Point mEnd;                /**< End of the linear gradient - NOTE: In the case
-                      of a zero length gradient it will act as the
+                                  of a zero length gradient it will act as the
-                      color of the last stop. */
+                                  color of the last stop. */
-  RefPtr<GradientStops>
+  REF<GradientStops> mStops; /**< GradientStops object for this gradient, this
-      mStops;     /**< GradientStops object for this gradient, this
+                                  should match the backend type of the draw
-                       should match the backend type of the draw
+                                  target this pattern will be used with. */
-                       target this pattern will be used with. */
+  Matrix mMatrix;            /**< A matrix that transforms the pattern into
-  Matrix mMatrix; /**< A matrix that transforms the pattern into
+                                  user space */
-                       user space */
 };
 
+typedef LinearGradientPatternT<> LinearGradientPattern;
+
 /**
  * This class is used for Radial Gradient Patterns, the gradient stops are
  * stored in a separate object and are backend dependent. This class itself
  * may be used on the stack.
  */
-class RadialGradientPattern : public Pattern {
+template <template <typename> typename REF = RefPtr>
+class RadialGradientPatternT : public Pattern {
+  typedef RadialGradientPatternT<ThreadSafeWeakPtr> Weak;
+
  public:
   /// For constructor parameter description, see member data documentation.
-  RadialGradientPattern(const Point& aCenter1, const Point& aCenter2,
+  RadialGradientPatternT(const Point& aCenter1, const Point& aCenter2,
-                        Float aRadius1, Float aRadius2,
+                         Float aRadius1, Float aRadius2,
-                        already_AddRefed<GradientStops> aStops,
+                         RefPtr<GradientStops> aStops,
-                        const Matrix& aMatrix = Matrix())
+                         const Matrix& aMatrix = Matrix())
       : mCenter1(aCenter1),
         mCenter2(aCenter2),
         mRadius1(aRadius1),
         mRadius2(aRadius2),
-        mStops(aStops),
+        mStops(std::move(aStops)),
         mMatrix(aMatrix) {}
 
   PatternType GetType() const override { return PatternType::RADIAL_GRADIENT; }
 
-  Pattern* Clone() const override {
+  Pattern* CloneWeak() const override {
-    return new RadialGradientPattern(mCenter1, mCenter2, mRadius1, mRadius2,
+    return new Weak(mCenter1, mCenter2, mRadius1, mRadius2, do_AddRef(mStops),
-                                     do_AddRef(mStops), mMatrix);
+                    mMatrix);
+  }
+
+  bool IsWeak() const override {
+    return std::is_same<decltype(*this), Weak>::value;
+  }
+
+  bool IsValid() const override { return IsRefValid(mStops); }
+
+  template <template <typename> typename T>
+  bool operator==(const RadialGradientPatternT<T>& aOther) const {
+    return mCenter1 == aOther.mCenter1 && mCenter2 == aOther.mCenter2 &&
+           mRadius1 == aOther.mRadius1 && mRadius2 == aOther.mRadius2 &&
+           mStops == aOther.mStops && mMatrix.ExactlyEquals(aOther.mMatrix);
   }
 
   bool operator==(const Pattern& aOther) const override {
     if (aOther.GetType() != PatternType::RADIAL_GRADIENT) {
       return false;
     }
-    const RadialGradientPattern& other =
+    return aOther.IsWeak()
-        static_cast<const RadialGradientPattern&>(aOther);
+               ? *this == static_cast<const Weak&>(aOther)
-    return mCenter1 == other.mCenter1 && mCenter2 == other.mCenter2 &&
+               : *this == static_cast<const RadialGradientPatternT<>&>(aOther);
-           mRadius1 == other.mRadius1 && mRadius2 == other.mRadius2 &&
-           mStops == other.mStops && mMatrix.ExactlyEquals(other.mMatrix);
   }
 
-  Point mCenter1;  //!< Center of the inner (focal) circle.
+  Point mCenter1;            //!< Center of the inner (focal) circle.
-  Point mCenter2;  //!< Center of the outer circle.
+  Point mCenter2;            //!< Center of the outer circle.
-  Float mRadius1;  //!< Radius of the inner (focal) circle.
+  Float mRadius1;            //!< Radius of the inner (focal) circle.
-  Float mRadius2;  //!< Radius of the outer circle.
+  Float mRadius2;            //!< Radius of the outer circle.
-  RefPtr<GradientStops>
+  REF<GradientStops> mStops; /**< GradientStops object for this gradient, this
-      mStops;      /**< GradientStops object for this gradient, this
+                                  should match the backend type of the draw
-                        should match the backend type of the draw target
+                                target this pattern will be used with. */
-                        this pattern will be used with. */
   Matrix mMatrix;  //!< A matrix that transforms the pattern into user space
 };
 
+typedef RadialGradientPatternT<> RadialGradientPattern;
+
 /**
  * This class is used for Conic Gradient Patterns, the gradient stops are
  * stored in a separate object and are backend dependent. This class itself
  * may be used on the stack.
  */
-class ConicGradientPattern : public Pattern {
+template <template <typename> typename REF = RefPtr>
+class ConicGradientPatternT : public Pattern {
+  typedef ConicGradientPatternT<ThreadSafeWeakPtr> Weak;
+
  public:
   /// For constructor parameter description, see member data documentation.
-  ConicGradientPattern(const Point& aCenter, Float aAngle, Float aStartOffset,
+  ConicGradientPatternT(const Point& aCenter, Float aAngle, Float aStartOffset,
-                       Float aEndOffset, already_AddRefed<GradientStops> aStops,
+                        Float aEndOffset, RefPtr<GradientStops> aStops,
-                       const Matrix& aMatrix = Matrix())
+                        const Matrix& aMatrix = Matrix())
       : mCenter(aCenter),
         mAngle(aAngle),
         mStartOffset(aStartOffset),
         mEndOffset(aEndOffset),
-        mStops(aStops),
+        mStops(std::move(aStops)),
         mMatrix(aMatrix) {}
 
   PatternType GetType() const override { return PatternType::CONIC_GRADIENT; }
 
-  Pattern* Clone() const override {
+  Pattern* CloneWeak() const override {
-    return new ConicGradientPattern(mCenter, mAngle, mStartOffset, mEndOffset,
+    return new Weak(mCenter, mAngle, mStartOffset, mEndOffset,
-                                    do_AddRef(mStops), mMatrix);
+                    do_AddRef(mStops), mMatrix);
+  }
+
+  bool IsWeak() const override {
+    return std::is_same<decltype(*this), Weak>::value;
+  }
+
+  bool IsValid() const override { return IsRefValid(mStops); }
+
+  template <template <typename> typename T>
+  bool operator==(const ConicGradientPatternT<T>& aOther) const {
+    return mCenter == aOther.mCenter && mAngle == aOther.mAngle &&
+           mStartOffset == aOther.mStartOffset &&
+           mEndOffset == aOther.mEndOffset && mStops == aOther.mStops &&
+           mMatrix.ExactlyEquals(aOther.mMatrix);
   }
 
   bool operator==(const Pattern& aOther) const override {
     if (aOther.GetType() != PatternType::CONIC_GRADIENT) {
       return false;
     }
-    const ConicGradientPattern& other =
+    return aOther.IsWeak()
-        static_cast<const ConicGradientPattern&>(aOther);
+               ? *this == static_cast<const Weak&>(aOther)
-    return mCenter == other.mCenter && mAngle == other.mAngle &&
+               : *this == static_cast<const ConicGradientPatternT<>&>(aOther);
-           mStartOffset == other.mStartOffset &&
-           mEndOffset == other.mEndOffset && mStops == other.mStops &&
-           mMatrix.ExactlyEquals(other.mMatrix);
   }
 
-  Point mCenter;       //!< Center of the gradient
+  Point mCenter;             //!< Center of the gradient
-  Float mAngle;        //!< Start angle of gradient
+  Float mAngle;              //!< Start angle of gradient
-  Float mStartOffset;  // Offset of first stop
+  Float mStartOffset;        // Offset of first stop
-  Float mEndOffset;    // Offset of last stop
+  Float mEndOffset;          // Offset of last stop
-  RefPtr<GradientStops>
+  REF<GradientStops> mStops; /**< GradientStops object for this gradient, this
-      mStops;      /**< GradientStops object for this gradient, this
+                                  should match the backend type of the draw
-                        should match the backend type of the draw target
+                                target this pattern will be used with. */
-                        this pattern will be used with. */
   Matrix mMatrix;  //!< A matrix that transforms the pattern into user space
 };
 
+typedef ConicGradientPatternT<> ConicGradientPattern;
+
 /**
  * This class is used for Surface Patterns, they wrap a surface and a
  * repetition mode for the surface. This may be used on the stack.
  */
-class SurfacePattern : public Pattern {
+template <template <typename> typename REF = RefPtr>
+class SurfacePatternT : public Pattern {
+  typedef SurfacePatternT<ThreadSafeWeakPtr> Weak;
+
  public:
   /// For constructor parameter description, see member data documentation.
-  SurfacePattern(SourceSurface* aSourceSurface, ExtendMode aExtendMode,
+  SurfacePatternT(RefPtr<SourceSurface> aSourceSurface, ExtendMode aExtendMode,
-                 const Matrix& aMatrix = Matrix(),
+                  const Matrix& aMatrix = Matrix(),
-                 SamplingFilter aSamplingFilter = SamplingFilter::GOOD,
+                  SamplingFilter aSamplingFilter = SamplingFilter::GOOD,
-                 const IntRect& aSamplingRect = IntRect())
+                  const IntRect& aSamplingRect = IntRect())
-      : mSurface(aSourceSurface),
+      : mSurface(std::move(aSourceSurface)),
         mExtendMode(aExtendMode),
         mSamplingFilter(aSamplingFilter),
         mMatrix(aMatrix),
@@ -464,25 +535,37 @@ class SurfacePattern : public Pattern {
 
   PatternType GetType() const override { return PatternType::SURFACE; }
 
-  Pattern* Clone() const override {
+  Pattern* CloneWeak() const override {
-    return new SurfacePattern(mSurface, mExtendMode, mMatrix, mSamplingFilter,
+    return new Weak(do_AddRef(mSurface), mExtendMode, mMatrix, mSamplingFilter,
-                              mSamplingRect);
+                    mSamplingRect);
+  }
+
+  bool IsWeak() const override {
+    return std::is_same<decltype(*this), Weak>::value;
+  }
+
+  bool IsValid() const override { return IsRefValid(mSurface); }
+
+  template <template <typename> typename T>
+  bool operator==(const SurfacePatternT<T>& aOther) const {
+    return mSurface == aOther.mSurface && mExtendMode == aOther.mExtendMode &&
+           mSamplingFilter == aOther.mSamplingFilter &&
+           mMatrix.ExactlyEquals(aOther.mMatrix) &&
+           mSamplingRect.IsEqualEdges(aOther.mSamplingRect);
   }
 
   bool operator==(const Pattern& aOther) const override {
     if (aOther.GetType() != PatternType::SURFACE) {
       return false;
     }
-    const SurfacePattern& other = static_cast<const SurfacePattern&>(aOther);
+    return aOther.IsWeak()
-    return mSurface == other.mSurface && mExtendMode == other.mExtendMode &&
+               ? *this == static_cast<const Weak&>(aOther)
-           mSamplingFilter == other.mSamplingFilter &&
+               : *this == static_cast<const SurfacePatternT<>&>(aOther);
-           mMatrix.ExactlyEquals(other.mMatrix) &&
-           mSamplingRect.IsEqualEdges(other.mSamplingRect);
   }
 
-  RefPtr<SourceSurface> mSurface;  //!< Surface to use for drawing
+  REF<SourceSurface> mSurface;  //!< Surface to use for drawing
-  ExtendMode mExtendMode; /**< This determines how the image is extended
+  ExtendMode mExtendMode;       /**< This determines how the image is extended
-                               outside the bounds of the image */
+                                     outside the bounds of the image */
   SamplingFilter
       mSamplingFilter;  //!< Resampling filter for resampling the image.
   Matrix mMatrix;       //!< Transforms the pattern into user space
@@ -492,6 +575,8 @@ class SurfacePattern : public Pattern {
                               specified. */
 };
 
+typedef SurfacePatternT<> SurfacePattern;
+
 class StoredPattern;
 
 static const int32_t kReasonableSurfaceSize = 8192;
@@ -506,7 +591,7 @@ static const int32_t kReasonableSurfaceSize = 8192;
  * used on random threads now. This will be fixed in the future. Eventually
  * all SourceSurface should be thread-safe.
  */
-class SourceSurface : public external::AtomicRefCounted<SourceSurface> {
+class SourceSurface : public SupportsThreadSafeWeakPtr<SourceSurface> {
  public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(SourceSurface)
   virtual ~SourceSurface() = default;

mfbt/ThreadSafeWeakPtr.h

@@ -182,6 +182,7 @@ class SupportsThreadSafeWeakPtr : public detail::SupportsThreadSafeWeakPtrBase {
   void ref() { AddRef(); }
   void deref() { Release(); }
   MozRefCountType refCount() const { return mWeakRef->mStrongCnt; }
+  bool hasOneRef() const { return refCount() == 1; }
 
  private:
   template <typename U>
@@ -247,6 +248,11 @@ class ThreadSafeWeakPtr {
     return *this == aOther.get();
   }
 
+  friend bool operator==(const RefPtr<T>& aStrong,
+                         const ThreadSafeWeakPtr& aWeak) {
+    return aWeak == aStrong.get();
+  }
+
   bool operator==(const T* aOther) const {
     if (!mRef) {
       return !aOther;