Bug 1856319 - create new ISVGFilterObserverList class instead of using nsISupports r=emilio

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

dom/canvas/CanvasRenderingContext2D.cpp

@@ -892,12 +892,17 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(CanvasRenderingContext2D)
     ImplCycleCollectionUnlink(
         tmp->mStyleStack[i].gradientStyles[Style::STROKE]);
     ImplCycleCollectionUnlink(tmp->mStyleStack[i].gradientStyles[Style::FILL]);
-    auto autoSVGFiltersObserver =
-        tmp->mStyleStack[i].autoSVGFiltersObserver.get();
-    if (autoSVGFiltersObserver) {
-      // XXXjwatt: I don't think this call achieves anything.  See the comment
-      // that documents this function.
-      SVGObserverUtils::DetachFromCanvasContext(autoSVGFiltersObserver);
+    if (auto* autoSVGFiltersObserver =
+            tmp->mStyleStack[i].autoSVGFiltersObserver.get()) {
+      /*
+       * XXXjwatt: I don't think this is doing anything useful.  All we do under
+       * this function is clear a raw C-style (i.e. not strong) pointer.  That's
+       * clearly not helping in breaking any cycles.  The fact that we MOZ_CRASH
+       * in OnRenderingChange if that pointer is null indicates that this isn't
+       * even doing anything useful in terms of preventing further invalidation
+       * from any observed filters.
+       */
+      autoSVGFiltersObserver->Detach();
     }
     ImplCycleCollectionUnlink(tmp->mStyleStack[i].autoSVGFiltersObserver);
   }

dom/canvas/CanvasRenderingContext2D.h

@@ -36,6 +36,7 @@ class nsXULElement;
 
 namespace mozilla {
 class ErrorResult;
+class ISVGFilterObserverList;
 class PresShell;
 
 namespace gl {
@@ -1083,7 +1084,7 @@ class CanvasRenderingContext2D : public nsICanvasRenderingContextInternal,
     StyleOwnedSlice<StyleFilter> filterChain;
     // RAII object that we obtain when we start to observer SVG filter elements
     // for rendering changes.  When released we stop observing the SVG elements.
-    nsCOMPtr<nsISupports> autoSVGFiltersObserver;
+    nsCOMPtr<ISVGFilterObserverList> autoSVGFiltersObserver;
     mozilla::gfx::FilterDescription filter;
     nsTArray<RefPtr<mozilla::gfx::SourceSurface>> filterAdditionalImages;
 

layout/svg/FilterInstance.cpp

@@ -42,7 +42,7 @@ namespace mozilla {
 
 FilterDescription FilterInstance::GetFilterDescription(
     nsIContent* aFilteredElement, Span<const StyleFilter> aFilterChain,
-    nsISupports* aFiltersObserverList, bool aFilterInputIsTainted,
+    ISVGFilterObserverList* aFiltersObserverList, bool aFilterInputIsTainted,
     const UserSpaceMetrics& aMetrics, const gfxRect& aBBox,
     nsTArray<RefPtr<SourceSurface>>& aOutAdditionalImages) {
   gfxMatrix identity;

layout/svg/FilterInstance.h

@@ -28,6 +28,7 @@ class nsIFrame;
 struct WrFiltersHolder;
 
 namespace mozilla {
+class ISVGFilterObserverList;
 class SVGFilterFrame;
 
 namespace dom {
@@ -80,7 +81,7 @@ class FilterInstance {
    */
   static FilterDescription GetFilterDescription(
       nsIContent* aFilteredElement, Span<const StyleFilter> aFilterChain,
-      nsISupports* aFiltersObserverList, bool aFilterInputIsTainted,
+      ISVGFilterObserverList* aFiltersObserverList, bool aFilterInputIsTainted,
       const UserSpaceMetrics& aMetrics, const gfxRect& aBBox,
       nsTArray<RefPtr<SourceSurface>>& aOutAdditionalImages);
 

layout/svg/SVGObserverUtils.cpp

@@ -803,6 +803,15 @@ SVGFilterFrame* SVGFilterObserver::GetAndObserveFilterFrame() {
       GetAndObserveReferencedFrame(LayoutFrameType::SVGFilter, nullptr));
 }
 
+NS_IMPL_CYCLE_COLLECTION(ISVGFilterObserverList)
+
+NS_IMPL_CYCLE_COLLECTING_ADDREF(ISVGFilterObserverList)
+NS_IMPL_CYCLE_COLLECTING_RELEASE(ISVGFilterObserverList)
+
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ISVGFilterObserverList)
+  NS_INTERFACE_MAP_ENTRY(nsISupports)
+NS_INTERFACE_MAP_END
+
 /**
  * This class manages a list of SVGFilterObservers, which correspond to
  * reference to SVG filters in a list of filters in a given 'filter' property.
@@ -817,19 +826,20 @@ SVGFilterFrame* SVGFilterObserver::GetAndObserveFilterFrame() {
  * FIXME(emilio): Why do we need this as opposed to the individual observers we
  * create in the constructor?
  */
-class SVGFilterObserverList : public nsISupports {
+class SVGFilterObserverList : public ISVGFilterObserverList {
  public:
   SVGFilterObserverList(Span<const StyleFilter> aFilters,
                         nsIContent* aFilteredElement,
                         nsIFrame* aFilteredFrame = nullptr);
 
-  const nsTArray<RefPtr<SVGFilterObserver>>& GetObservers() const {
+  const nsTArray<RefPtr<SVGFilterObserver>>& GetObservers() const override {
     return mObservers;
   }
 
   // nsISupports
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_CLASS(SVGFilterObserverList)
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(SVGFilterObserverList,
+                                           ISVGFilterObserverList)
 
   virtual void OnRenderingChange(Element* aObservingContent) = 0;
 
@@ -857,21 +867,24 @@ void SVGFilterObserver::OnRenderingChange() {
   }
 }
 
-NS_IMPL_CYCLE_COLLECTING_ADDREF(SVGFilterObserverList)
-NS_IMPL_CYCLE_COLLECTING_RELEASE(SVGFilterObserverList)
+NS_IMPL_ADDREF_INHERITED(SVGFilterObserverList, ISVGFilterObserverList)
+NS_IMPL_RELEASE_INHERITED(SVGFilterObserverList, ISVGFilterObserverList)
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(SVGFilterObserverList)
 
-NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(SVGFilterObserverList)
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(SVGFilterObserverList,
+                                                  ISVGFilterObserverList)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mObservers)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
-NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(SVGFilterObserverList)
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(SVGFilterObserverList,
+                                                ISVGFilterObserverList)
   tmp->DetachObservers();
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mObservers);
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mObservers)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(SVGFilterObserverList)
+  NS_INTERFACE_MAP_ENTRY(ISVGFilterObserverList)
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
@@ -950,7 +963,7 @@ class SVGFilterObserverListForCanvasContext final
       : SVGFilterObserverList(aFilters, aCanvasElement), mContext(aContext) {}
 
   void OnRenderingChange(Element* aObservingContent) override;
-  void DetachFromContext() { mContext = nullptr; }
+  void Detach() override { mContext = nullptr; }
 
  private:
   CanvasRenderingContext2D* mContext;
@@ -961,7 +974,7 @@ void SVGFilterObserverListForCanvasContext::OnRenderingChange(
   if (!mContext) {
     NS_WARNING(
         "GFX: This should never be called without a context, except during "
-        "cycle collection (when DetachFromContext has been called)");
+        "cycle collection (when Detach has been called)");
     return;
   }
   // Refresh the cached FilterDescription in mContext->CurrentState().filter.
@@ -1350,7 +1363,7 @@ static SVGFilterObserverListForCSSProp* GetOrCreateFilterObserverListForCSS(
 }
 
 static SVGObserverUtils::ReferenceState GetAndObserveFilters(
-    SVGFilterObserverList* aObserverList,
+    ISVGFilterObserverList* aObserverList,
     nsTArray<SVGFilterFrame*>* aFilterFrames) {
   if (!aObserverList) {
     return SVGObserverUtils::eHasNoRefs;
@@ -1387,10 +1400,9 @@ SVGObserverUtils::ReferenceState SVGObserverUtils::GetAndObserveFilters(
 }
 
 SVGObserverUtils::ReferenceState SVGObserverUtils::GetAndObserveFilters(
-    nsISupports* aObserverList, nsTArray<SVGFilterFrame*>* aFilterFrames) {
-  return mozilla::GetAndObserveFilters(
-      static_cast<SVGFilterObserverListForCanvasContext*>(aObserverList),
-      aFilterFrames);
+    ISVGFilterObserverList* aObserverList,
+    nsTArray<SVGFilterFrame*>* aFilterFrames) {
+  return mozilla::GetAndObserveFilters(aObserverList, aFilterFrames);
 }
 
 SVGObserverUtils::ReferenceState SVGObserverUtils::GetFiltersIfObserving(
@@ -1400,18 +1412,14 @@ SVGObserverUtils::ReferenceState SVGObserverUtils::GetFiltersIfObserving(
   return mozilla::GetAndObserveFilters(observerList, aFilterFrames);
 }
 
-already_AddRefed<nsISupports> SVGObserverUtils::ObserveFiltersForCanvasContext(
+already_AddRefed<ISVGFilterObserverList>
+SVGObserverUtils::ObserveFiltersForCanvasContext(
     CanvasRenderingContext2D* aContext, Element* aCanvasElement,
     const Span<const StyleFilter> aFilters) {
   return do_AddRef(new SVGFilterObserverListForCanvasContext(
       aContext, aCanvasElement, aFilters));
 }
 
-void SVGObserverUtils::DetachFromCanvasContext(nsISupports* aAutoObserver) {
-  static_cast<SVGFilterObserverListForCanvasContext*>(aAutoObserver)
-      ->DetachFromContext();
-}
-
 static SVGPaintingProperty* GetOrCreateClipPathObserver(
     nsIFrame* aClippedFrame) {
   MOZ_ASSERT(!aClippedFrame->GetPrevContinuation(),

layout/svg/SVGObserverUtils.h

@@ -29,6 +29,7 @@ class nsIURI;
 namespace mozilla {
 class SVGClipPathFrame;
 class SVGFilterFrame;
+class SVGFilterObserver;
 class SVGMarkerFrame;
 class SVGMaskFrame;
 class SVGPaintServerFrame;
@@ -41,8 +42,31 @@ class SVGMPathElement;
 }  // namespace dom
 }  // namespace mozilla
 
+#define MOZILLA_ICANVASFILTEROBSERVER_IID            \
+  {                                                  \
+    0xd1c85f93, 0xd1ed, 0x4ea9, {                    \
+      0xa0, 0x39, 0x71, 0x62, 0xe4, 0x41, 0xf1, 0xa1 \
+    }                                                \
+  }
+
 namespace mozilla {
 
+class ISVGFilterObserverList : public nsISupports {
+ public:
+  NS_DECLARE_STATIC_IID_ACCESSOR(MOZILLA_ICANVASFILTEROBSERVER_IID)
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTION_CLASS(ISVGFilterObserverList)
+
+  virtual const nsTArray<RefPtr<SVGFilterObserver>>& GetObservers() const = 0;
+  virtual void Detach() {}
+
+ protected:
+  virtual ~ISVGFilterObserverList() = default;
+};
+
+NS_DEFINE_STATIC_IID_ACCESSOR(ISVGFilterObserverList,
+                              MOZILLA_ICANVASFILTEROBSERVER_IID)
+
 /*
  * This class contains URL and referrer information (referrer and referrer
  * policy).
@@ -296,7 +320,8 @@ class SVGObserverUtils {
    * parameter.
    */
   static ReferenceState GetAndObserveFilters(
-      nsISupports* aObserverList, nsTArray<SVGFilterFrame*>* aFilterFrames);
+      ISVGFilterObserverList* aObserverList,
+      nsTArray<SVGFilterFrame*>* aFilterFrames);
 
   /**
    * If the given frame is already observing SVG filters, this function gets
@@ -319,22 +344,10 @@ class SVGObserverUtils {
    * objects separately.  It would be better to refactor things so that we only
    * do that work once.
    */
-  static already_AddRefed<nsISupports> ObserveFiltersForCanvasContext(
-      CanvasRenderingContext2D* aContext, Element* aCanvasElement,
-      Span<const StyleFilter> aFilters);
-
-  /**
-   * Called when cycle collecting CanvasRenderingContext2D, and requires the
-   * RAII object returned from ObserveFiltersForCanvasContext to be passed in.
-   *
-   * XXXjwatt: I don't think this is doing anything useful.  All we do under
-   * this function is clear a raw C-style (i.e. not strong) pointer.  That's
-   * clearly not helping in breaking any cycles.  The fact that we MOZ_CRASH
-   * in OnRenderingChange if that pointer is null indicates that this isn't
-   * even doing anything useful in terms of preventing further invalidation
-   * from any observed filters.
-   */
-  static void DetachFromCanvasContext(nsISupports* aAutoObserver);
+  static already_AddRefed<ISVGFilterObserverList>
+  ObserveFiltersForCanvasContext(CanvasRenderingContext2D* aContext,
+                                 Element* aCanvasElement,
+                                 Span<const StyleFilter> aFilters);
 
   /**
    * Get the frame of the SVG clipPath applied to aClippedFrame, if any, and