Bug 1495215. Separate out the SVGObserverUtils mask handling from EffectProperties. r=longsonr
authorJonathan Watt <jwatt@jwatt.org>
Wed, 19 Sep 2018 15:54:27 +0100
changeset 494656 5c8b0d630ceb6bb8601dd0ebcd53a53ee87c4318
parent 494655 0c399277cee5d443830c19f7d28e45060a3b714f
child 494657 c97cf45fc0e4feea7a33bcf4a49dba452a506efa
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1495215
milestone64.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 1495215. Separate out the SVGObserverUtils mask handling from EffectProperties. r=longsonr Differential Revision: https://phabricator.services.mozilla.com/D7242
layout/generic/nsFrame.cpp
layout/painting/nsDisplayList.cpp
layout/svg/SVGObserverUtils.cpp
layout/svg/SVGObserverUtils.h
layout/svg/nsSVGIntegrationUtils.cpp
layout/svg/nsSVGUtils.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2712,20 +2712,21 @@ ComputeClipForMaskItem(nsDisplayListBuil
 
     // Use an infinite dirty rect to pass into nsCSSRendering::
     // GetImageLayerClip() because we don't have an actual dirty rect to
     // pass in. This is fine because the only time GetImageLayerClip() will
     // not intersect the incoming dirty rect with something is in the "NoClip"
     // case, and we handle that specially.
     nsRect dirtyRect(nscoord_MIN/2, nscoord_MIN/2, nscoord_MAX, nscoord_MAX);
 
-    nsIFrame* firstFrame = nsLayoutUtils::FirstContinuationOrIBSplitSibling(aMaskedFrame);
-    SVGObserverUtils::EffectProperties effectProperties =
-        SVGObserverUtils::GetEffectProperties(firstFrame);
-    nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
+    nsIFrame* firstFrame =
+      nsLayoutUtils::FirstContinuationOrIBSplitSibling(aMaskedFrame);
+    nsTArray<nsSVGMaskFrame*> maskFrames;
+    // XXX check return value?
+    SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
 
     for (uint32_t i = 0; i < maskFrames.Length(); ++i) {
       gfxRect clipArea;
       if (maskFrames[i]) {
         clipArea = maskFrames[i]->GetMaskArea(aMaskedFrame);
         clipArea = cssToDevMatrix.TransformBounds(clipArea);
       } else {
         const auto& layer = svgReset->mMask.mLayers[i];
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -9567,19 +9567,19 @@ ComputeMaskGeometry(PaintFramesParams& a
 {
   // Properties are added lazily and may have been removed by a restyle, so
   // make sure all applicable ones are set again.
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(aParams.frame);
 
   const nsStyleSVGReset* svgReset = firstFrame->StyleSVGReset();
 
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
-  nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
+  nsTArray<nsSVGMaskFrame*> maskFrames;
+  // XXX check return value?
+  SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
 
   if (maskFrames.Length() == 0) {
     return;
   }
 
   gfxContext& ctx = aParams.ctx;
   nsIFrame* frame = aParams.frame;
 
@@ -9710,22 +9710,21 @@ nsDisplayMasksAndClipPaths::IsValidMask(
   }
 
   if (mFrame->StyleEffects()->mOpacity == 0.0f && mHandleOpacity) {
     return false;
   }
 
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(mFrame);
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
 
   if (SVGObserverUtils::GetAndObserveClipPath(firstFrame, nullptr) ==
         SVGObserverUtils::eHasRefsSomeInvalid ||
-      effectProperties.HasInvalidMask()) {
+      SVGObserverUtils::GetAndObserveMasks(firstFrame, nullptr) ==
+        SVGObserverUtils::eHasRefsSomeInvalid) {
     return false;
   }
 
   return true;
 }
 
 
 
@@ -10030,18 +10029,16 @@ nsDisplayMasksAndClipPaths::GetClipWithR
 }
 
 #ifdef MOZ_DUMP_PAINTING
 void
 nsDisplayMasksAndClipPaths::PrintEffects(nsACString& aTo)
 {
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(mFrame);
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
   bool first = true;
   aTo += " effects=(";
   if (mFrame->StyleEffects()->mOpacity != 1.0f && mHandleOpacity) {
     first = false;
     aTo += nsPrintfCString("opacity(%f)", mFrame->StyleEffects()->mOpacity);
   }
   nsSVGClipPathFrame* clipPathFrame;
   // XXX Check return value?
@@ -10056,17 +10053,19 @@ nsDisplayMasksAndClipPaths::PrintEffects
   } else if (mFrame->StyleSVGReset()->HasClipPath()) {
     if (!first) {
       aTo += ", ";
     }
     aTo += "clip(basic-shape)";
     first = false;
   }
 
-  nsTArray<nsSVGMaskFrame*> masks = effectProperties.GetMaskFrames();
+  nsTArray<nsSVGMaskFrame*> masks;
+  // XXX check return value?
+  SVGObserverUtils::GetAndObserveMasks(firstFrame, &masks);
   if (!masks.IsEmpty() && masks[0]) {
     if (!first) {
       aTo += ", ";
     }
     aTo += "mask";
   }
   aTo += ")";
 }
--- a/layout/svg/SVGObserverUtils.cpp
+++ b/layout/svg/SVGObserverUtils.cpp
@@ -561,30 +561,16 @@ nsSVGPaintingProperty::OnRenderingChange
 
   if (frame->GetStateBits() & NS_FRAME_SVG_LAYOUT) {
     frame->InvalidateFrameSubtree();
   } else {
     InvalidateAllContinuations(frame);
   }
 }
 
-static SVGMaskObserverList*
-GetOrCreateMaskProperty(nsIFrame* aFrame)
-{
-  SVGMaskObserverList *prop =
-    aFrame->GetProperty(SVGObserverUtils::MaskProperty());
-  if (prop)
-    return prop;
-
-  prop = new SVGMaskObserverList(aFrame);
-  NS_ADDREF(prop);
-  aFrame->SetProperty(SVGObserverUtils::MaskProperty(), prop);
-  return prop;
-}
-
 static already_AddRefed<URLAndReferrerInfo>
 ResolveURLUsingLocalRef(nsIFrame* aFrame, const css::URLValueData* aURL)
 {
   MOZ_ASSERT(aFrame);
 
   if (!aURL) {
     return nullptr;
   }
@@ -668,16 +654,18 @@ SVGObserverUtils::GetMarkerFrames(nsIFra
   return foundMarker;
 }
 
 // Note that the returned list will be empty in the case of a 'filter' property
 // that only specifies CSS filter functions (no url()'s to SVG filters).
 static SVGFilterObserverListForCSSProp*
 GetOrCreateFilterObserverListForCSS(nsIFrame* aFrame)
 {
+  MOZ_ASSERT(!aFrame->GetPrevContinuation(), "Require first continuation");
+
   const nsStyleEffects* effects = aFrame->StyleEffects();
   if (!effects->HasFilters()) {
     return nullptr;
   }
   SVGFilterObserverListForCSSProp* observers =
     aFrame->GetProperty(SVGObserverUtils::FilterProperty());
   if (observers) {
     return observers;
@@ -752,16 +740,18 @@ SVGObserverUtils::DetachFromCanvasContex
   static_cast<SVGFilterObserverListForCanvasContext*>(aAutoObserver)->
     DetachFromContext();
 }
 
 
 static nsSVGPaintingProperty*
 GetOrCreateClipPathObserver(nsIFrame* aClippedFrame)
 {
+  MOZ_ASSERT(!aClippedFrame->GetPrevContinuation(), "Require first continuation");
+
   const nsStyleSVGReset* svgStyleReset = aClippedFrame->StyleSVGReset();
   if (svgStyleReset->mClipPath.GetType() != StyleShapeSourceType::URL) {
     return nullptr;
   }
   css::URLValue* url = svgStyleReset->mClipPath.GetURL();
   RefPtr<URLAndReferrerInfo> pathURI = ResolveURLUsingLocalRef(aClippedFrame, url);
   return SVGObserverUtils::GetPaintingProperty(pathURI, aClippedFrame,
                                          SVGObserverUtils::ClipPathProperty());
@@ -788,16 +778,81 @@ SVGObserverUtils::GetAndObserveClipPath(
     return eHasRefsSomeInvalid;
   }
   if (aClipPathFrame) {
     *aClipPathFrame = frame;
   }
   return frame ? eHasRefsAllValid : eHasNoRefs;
 }
 
+static SVGMaskObserverList*
+GetOrCreateMaskObserverList(nsIFrame* aMaskedFrame)
+{
+  MOZ_ASSERT(!aMaskedFrame->GetPrevContinuation(), "Require first continuation");
+
+  const nsStyleSVGReset* style = aMaskedFrame->StyleSVGReset();
+  if (!style->HasMask()) {
+    return nullptr;
+  }
+
+  MOZ_ASSERT(style->mMask.mImageCount > 0);
+
+  SVGMaskObserverList* prop =
+    aMaskedFrame->GetProperty(SVGObserverUtils::MaskProperty());
+  if (prop) {
+    return prop;
+  }
+  prop = new SVGMaskObserverList(aMaskedFrame);
+  NS_ADDREF(prop);
+  aMaskedFrame->SetProperty(SVGObserverUtils::MaskProperty(), prop);
+  return prop;
+}
+
+SVGObserverUtils::ReferenceState
+SVGObserverUtils::GetAndObserveMasks(nsIFrame* aMaskedFrame,
+                                     nsTArray<nsSVGMaskFrame*>* aMaskFrames)
+{
+  SVGMaskObserverList* observerList = GetOrCreateMaskObserverList(aMaskedFrame);
+  if (!observerList) {
+    return eHasNoRefs;
+  }
+
+  const nsTArray<RefPtr<nsSVGPaintingProperty>>& observers =
+    observerList->GetObservers();
+  if (observers.IsEmpty()) {
+    return eHasNoRefs;
+  }
+
+  ReferenceState state = eHasRefsAllValid;
+
+  for (size_t i = 0; i < observers.Length(); i++) {
+    bool frameTypeOK = true;
+    nsSVGMaskFrame* maskFrame = static_cast<nsSVGMaskFrame*>(
+      observers[i]->GetAndObserveReferencedFrame(LayoutFrameType::SVGMask,
+                                                 &frameTypeOK));
+    MOZ_ASSERT(!maskFrame || frameTypeOK);
+    // XXXjwatt: this looks fishy
+    if (!frameTypeOK) {
+      // We can not find the specific SVG mask resource in the downloaded SVG
+      // document. There are two possibilities:
+      // 1. The given resource id is invalid.
+      // 2. The given resource id refers to a viewbox.
+      //
+      // Hand it over to the style image.
+      observerList->ResolveImage(i);
+      state = eHasRefsSomeInvalid;
+    }
+    if (aMaskFrames) {
+      aMaskFrames->AppendElement(maskFrame);
+    }
+  }
+
+  return state;
+}
+
 SVGGeometryElement*
 SVGObserverUtils::GetTextPathsReferencedPath(nsIFrame* aTextPathFrame)
 {
   SVGTextPathObserver* property =
     aTextPathFrame->GetProperty(SVGObserverUtils::HrefAsTextPathProperty());
 
   if (!property) {
     nsIContent* content = aTextPathFrame->GetContent();
@@ -833,17 +888,17 @@ SVGObserverUtils::GetTextPathsReferenced
 
 void
 SVGObserverUtils::InitiateResourceDocLoads(nsIFrame* aFrame)
 {
   // We create observer objects and attach them to aFrame, but we do not
   // make aFrame start observing the referenced frames.
   Unused << GetOrCreateFilterObserverListForCSS(aFrame);
   Unused << GetOrCreateClipPathObserver(aFrame);
-  Unused << GetEffectProperties(aFrame);
+  Unused << GetOrCreateMaskObserverList(aFrame);
 }
 
 void
 SVGObserverUtils::RemoveTextPathObserver(nsIFrame* aTextPathFrame)
 {
   aTextPathFrame->DeleteProperty(HrefAsTextPathProperty());
 }
 
@@ -914,31 +969,16 @@ SVGObserverUtils::GetPaintingPropertyFor
   if (!prop) {
     bool watchImage = aProperty == SVGObserverUtils::BackgroundImageProperty();
     prop = new nsSVGPaintingProperty(aURI, aFrame, watchImage);
     hashtable->Put(aURI, prop);
   }
   return prop;
 }
 
-SVGObserverUtils::EffectProperties
-SVGObserverUtils::GetEffectProperties(nsIFrame* aFrame)
-{
-  NS_ASSERTION(!aFrame->GetPrevContinuation(), "aFrame should be first continuation");
-
-  EffectProperties result;
-  const nsStyleSVGReset *style = aFrame->StyleSVGReset();
-
-  MOZ_ASSERT(style->mMask.mImageCount > 0);
-  result.mMaskObservers = style->HasMask()
-                          ? GetOrCreateMaskProperty(aFrame) : nullptr;
-
-  return result;
-}
-
 nsSVGPaintServerFrame *
 SVGObserverUtils::GetPaintServer(nsIFrame* aTargetFrame,
                                  nsStyleSVGPaint nsStyleSVG::* aPaint)
 {
   // If we're looking at a frame within SVG text, then we need to look up
   // to find the right frame to get the painting property off.  We should at
   // least look up past a text frame, and if the text frame's parent is the
   // anonymous block frame, then we look up to its parent (the SVGTextFrame).
@@ -973,70 +1013,16 @@ SVGObserverUtils::GetPaintServer(nsIFram
   if (type != LayoutFrameType::SVGLinearGradient &&
       type != LayoutFrameType::SVGRadialGradient &&
       type != LayoutFrameType::SVGPattern)
     return nullptr;
 
   return static_cast<nsSVGPaintServerFrame*>(result);
 }
 
-nsTArray<nsSVGMaskFrame *>
-SVGObserverUtils::EffectProperties::GetMaskFrames()
-{
-  nsTArray<nsSVGMaskFrame *> result;
-  if (!mMaskObservers) {
-    return result;
-  }
-
-  bool ok = true;
-  const nsTArray<RefPtr<nsSVGPaintingProperty>>& observers =
-    mMaskObservers->GetObservers();
-  for (size_t i = 0; i < observers.Length(); i++) {
-    nsSVGMaskFrame* maskFrame = static_cast<nsSVGMaskFrame*>(
-      observers[i]->GetAndObserveReferencedFrame(LayoutFrameType::SVGMask, &ok));
-    MOZ_ASSERT(!maskFrame || ok);
-    if (!ok) {
-      // We can not find the specific SVG mask resource in the downloaded SVG
-      // document. There are two possibilities:
-      // 1. The given resource id is invalid.
-      // 2. The given resource id refers to a viewbox.
-      //
-      // Hand it over to the style image.
-      mMaskObservers->ResolveImage(i);
-    }
-    result.AppendElement(maskFrame);
-  }
-
-  return result;
-}
-
-bool
-SVGObserverUtils::EffectProperties::HasNoOrValidEffects()
-{
-  return HasNoOrValidMask();
-}
-
-bool
-SVGObserverUtils::EffectProperties::HasNoOrValidMask()
-{
-  if (mMaskObservers) {
-    bool ok = true;
-    const nsTArray<RefPtr<nsSVGPaintingProperty>>& observers =
-      mMaskObservers->GetObservers();
-    for (size_t i = 0; i < observers.Length(); i++) {
-      observers[i]->GetAndObserveReferencedFrame(LayoutFrameType::SVGMask, &ok);
-      if (!ok) {
-        return false;
-      }
-    }
-  }
-
-  return true;
-}
-
 void
 SVGObserverUtils::UpdateEffects(nsIFrame* aFrame)
 {
   NS_ASSERTION(aFrame->GetContent()->IsElement(),
                "aFrame's content should be an element");
 
   aFrame->DeleteProperty(FilterProperty());
   aFrame->DeleteProperty(MaskProperty());
--- a/layout/svg/SVGObserverUtils.h
+++ b/layout/svg/SVGObserverUtils.h
@@ -576,56 +576,16 @@ public:
   NS_DECLARE_FRAME_PROPERTY_RELEASABLE(MarkerEndProperty, SVGMarkerObserver)
   NS_DECLARE_FRAME_PROPERTY_RELEASABLE(FillProperty, nsSVGPaintingProperty)
   NS_DECLARE_FRAME_PROPERTY_RELEASABLE(StrokeProperty, nsSVGPaintingProperty)
   NS_DECLARE_FRAME_PROPERTY_RELEASABLE(HrefAsTextPathProperty,
                                        SVGTextPathObserver)
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(BackgroundImageProperty,
                                       URIObserverHashtable)
 
-  struct EffectProperties {
-    SVGMaskObserverList* mMaskObservers;
-
-    /**
-     * @return an array which contains all SVG mask frames.
-     */
-    nsTArray<nsSVGMaskFrame*> GetMaskFrames();
-
-    /*
-     * @return true if all effects we have are valid or we have no effect
-     * at all.
-     */
-    bool HasNoOrValidEffects();
-
-    /*
-     * @return true if we have any invalid effect.
-     */
-    bool HasInvalidEffects() {
-      return !HasNoOrValidEffects();
-    }
-
-    /*
-     * @return true if we either do not have mask or all masks we have
-     * are valid.
-     */
-    bool HasNoOrValidMask();
-
-    /*
-     * @return true if we have an invalid mask.
-     */
-    bool HasInvalidMask() {
-      return !HasNoOrValidMask();
-    }
-  };
-
-  /**
-   * @param aFrame should be the first continuation
-   */
-  static EffectProperties GetEffectProperties(nsIFrame* aFrame);
-
   /**
    * Ensures that that if the given frame requires any resources that are in
    * SVG resource documents that the loading of those documents is initiated.
    * This does not make aFrame start to observe any elements that it
    * references.
    */
   static void InitiateResourceDocLoads(nsIFrame* aFrame);
 
@@ -791,16 +751,32 @@ public:
    * is not invalid for clip-path or mask.  We will return eHasNoRefs in that
    * case.
    */
   static ReferenceState
   GetAndObserveClipPath(nsIFrame* aClippedFrame,
                         nsSVGClipPathFrame** aClipPathFrame);
 
   /**
+   * If masking is applied to aMaskedFrame, gets an array of any SVG masks
+   * that are referenced, setting up aMaskFrames as a rendering observer of
+   * those masks (if any).
+   *
+   * NOTE! A return value of eHasNoRefs does NOT mean that there are no masks
+   * to be applied, only that there are no references to SVG mask elements.
+   *
+   * Note that, unlike for filters, a reference to an ID that doesn't exist
+   * is not invalid for clip-path or mask.  We will return eHasNoRefs in that
+   * case.
+   */
+  static ReferenceState
+  GetAndObserveMasks(nsIFrame* aMaskedFrame,
+                     nsTArray<nsSVGMaskFrame*>* aMaskFrames);
+
+  /**
    * Get the SVGGeometryElement that is referenced by aTextPathFrame, and make
    * aTextPathFrame start observing rendering changes to that element.
    */
   static SVGGeometryElement*
   GetTextPathsReferencedPath(nsIFrame* aTextPathFrame);
 
   /**
    * Make aTextPathFrame stop observing rendering changes to the
--- a/layout/svg/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/nsSVGIntegrationUtils.cpp
@@ -742,19 +742,20 @@ MoveContextOriginToUserSpace(nsIFrame* a
   return offset;
 }
 
 bool
 nsSVGIntegrationUtils::IsMaskResourceReady(nsIFrame* aFrame)
 {
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
-  nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
+  nsTArray<nsSVGMaskFrame*> maskFrames;
+  // XXX check return value?
+  SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
+
   const nsStyleSVGReset* svgReset = firstFrame->StyleSVGReset();
 
   for (uint32_t i = 0; i < maskFrames.Length(); i++) {
     // Refers to a valid SVG mask.
     if (maskFrames[i]) {
       continue;
     }
 
@@ -798,37 +799,37 @@ nsSVGIntegrationUtils::PaintMask(const P
   }
 
   nsIFrame* frame = aParams.frame;
   if (!ValidateSVGFrame(frame)) {
     return false;
   }
 
   gfxContext& ctx = aParams.ctx;
-  nsIFrame* firstFrame =
-    nsLayoutUtils::FirstContinuationOrIBSplitSibling(frame);
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
-
   RefPtr<DrawTarget> maskTarget = ctx.GetDrawTarget();
 
   if (maskUsage.shouldGenerateMaskLayer &&
       (maskUsage.shouldGenerateClipMaskLayer ||
        maskUsage.shouldApplyClipPath)) {
     // We will paint both mask of positioned mask and clip-path into
     // maskTarget.
     //
     // Create one extra draw target for drawing positioned mask, so that we do
     // not have to copy the content of maskTarget before painting
     // clip-path into it.
     maskTarget = maskTarget->CreateSimilarDrawTarget(maskTarget->GetSize(),
                                                      SurfaceFormat::A8);
   }
 
-  nsTArray<nsSVGMaskFrame *> maskFrames = effectProperties.GetMaskFrames();
+  nsIFrame* firstFrame =
+    nsLayoutUtils::FirstContinuationOrIBSplitSibling(frame);
+  nsTArray<nsSVGMaskFrame*> maskFrames;
+  // XXX check return value?
+  SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
+
   AutoPopGroup autoPop;
   bool shouldPushOpacity = (maskUsage.opacity != 1.0) &&
                            (maskFrames.Length() != 1);
   if (shouldPushOpacity) {
     ctx.PushGroupForBlendBack(gfxContentType::COLOR_ALPHA, maskUsage.opacity);
     autoPop.SetContext(&ctx);
   }
 
@@ -919,29 +920,28 @@ void PaintMaskAndClipPathInternal(const 
 
   if (maskUsage.opacity == 0.0f) {
     return;
   }
 
   gfxContext& context = aParams.ctx;
   gfxContextMatrixAutoSaveRestore matrixAutoSaveRestore(&context);
 
-  /* Properties are added lazily and may have been removed by a restyle,
-     so make sure all applicable ones are set again. */
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(frame);
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
 
   nsSVGClipPathFrame* clipPathFrame;
   // XXX check return value?
   SVGObserverUtils::GetAndObserveClipPath(firstFrame, &clipPathFrame);
 
+  nsTArray<nsSVGMaskFrame*> maskFrames;
+  // XXX check return value?
+  SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
+
   gfxMatrix cssPxToDevPxMatrix = nsSVGUtils::GetCSSPxToDevPxMatrix(frame);
-  nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
 
   bool shouldGenerateMask = (maskUsage.opacity != 1.0f ||
                              maskUsage.shouldGenerateClipMaskLayer ||
                              maskUsage.shouldGenerateMaskLayer);
   bool shouldPushMask = false;
 
   /* Check if we need to do additional operations on this child's
    * rendering, which necessitates rendering into another surface. */
--- a/layout/svg/nsSVGUtils.cpp
+++ b/layout/svg/nsSVGUtils.cpp
@@ -482,22 +482,21 @@ void
 nsSVGUtils::DetermineMaskUsage(nsIFrame* aFrame, bool aHandleOpacity,
                                MaskUsage& aUsage)
 {
   aUsage.opacity = ComputeOpacity(aFrame, aHandleOpacity);
 
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
 
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(firstFrame);
   const nsStyleSVGReset *svgReset = firstFrame->StyleSVGReset();
 
-  nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
-
+  nsTArray<nsSVGMaskFrame*> maskFrames;
+  // XXX check return value?
+  SVGObserverUtils::GetAndObserveMasks(firstFrame, &maskFrames);
   aUsage.shouldGenerateMaskLayer = (maskFrames.Length() > 0);
 
   nsSVGClipPathFrame* clipPathFrame;
   // XXX check return value?
   SVGObserverUtils::GetAndObserveClipPath(firstFrame, &clipPathFrame);
   MOZ_ASSERT(!clipPathFrame ||
              svgReset->mClipPath.GetType() == StyleShapeSourceType::URL);
 
@@ -709,32 +708,31 @@ nsSVGUtils::PaintFrameWithEffects(nsIFra
    *   clip region).
    *f
    * + Merge opacity and masking if both used together.
    */
 
   /* Properties are added lazily and may have been removed by a restyle,
      so make sure all applicable ones are set again. */
   nsSVGClipPathFrame* clipPathFrame;
-  SVGObserverUtils::EffectProperties effectProperties =
-    SVGObserverUtils::GetEffectProperties(aFrame);
+  nsTArray<nsSVGMaskFrame*> maskFrames;
   // TODO: We currently pass nullptr instead of an nsTArray* here, but we
   // actually should get the filter frames and then pass them into
   // PaintFilteredFrame below!  See bug 1494263.
-  if (effectProperties.HasInvalidEffects() ||
-      SVGObserverUtils::GetAndObserveFilters(aFrame, nullptr) ==
+  if (SVGObserverUtils::GetAndObserveFilters(aFrame, nullptr) ==
         SVGObserverUtils::eHasRefsSomeInvalid ||
       SVGObserverUtils::GetAndObserveClipPath(aFrame, &clipPathFrame) ==
+        SVGObserverUtils::eHasRefsSomeInvalid ||
+      SVGObserverUtils::GetAndObserveMasks(aFrame, &maskFrames) ==
         SVGObserverUtils::eHasRefsSomeInvalid) {
     // Some resource is invalid. We shouldn't paint anything.
     return;
   }
 
-  nsTArray<nsSVGMaskFrame*> masks = effectProperties.GetMaskFrames();
-  nsSVGMaskFrame *maskFrame = masks.IsEmpty() ? nullptr : masks[0];
+  nsSVGMaskFrame* maskFrame = maskFrames.IsEmpty() ? nullptr : maskFrames[0];
 
   MixModeBlender blender(aFrame, &aContext);
   gfxContext* target = blender.ShouldCreateDrawTargetForBlend()
                        ? blender.CreateBlendTarget(aTransform) : &aContext;
 
   if (!target) {
     return;
   }