Bug 1322330 - Part 1. Remove EffectProperties::GetFirstMaskFrame and implement EffectProperties::HasInvalidMaskOrClipPathResource. draft
authorcku <cku@mozilla.com>
Tue, 06 Dec 2016 16:12:36 -1000
changeset 448943 310f991f6c0018c39c3602b4916007a0c8237613
parent 448717 f46f85dcfbc2b3098ea758825d18be6fab33cbc6
child 448944 5ad31d8db3e57fc92efa44177b428f55a9756548
push id38492
push userbmo:cku@mozilla.com
push dateTue, 13 Dec 2016 09:30:56 +0000
bugs1322330
milestone53.0a1
Bug 1322330 - Part 1. Remove EffectProperties::GetFirstMaskFrame and implement EffectProperties::HasInvalidMaskOrClipPathResource. MozReview-Commit-ID: 4nfhL7btNFT
layout/painting/nsDisplayList.cpp
layout/svg/nsSVGEffects.cpp
layout/svg/nsSVGEffects.h
layout/svg/nsSVGUtils.cpp
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7457,17 +7457,18 @@ nsDisplayMask::PrintEffects(nsACString& 
   if (style->HasClipPath() && !clipPathFrame) {
     if (!first) {
       aTo += ", ";
     }
     aTo += "clip(basic-shape)";
     first = false;
   }
 
-  if (effectProperties.GetFirstMaskFrame()) {
+  nsTArray<nsSVGMaskFrame*> masks = effectProperties.GetMaskFrames();
+  if (!masks.IsEmpty() && masks[0]) {
     if (!first) {
       aTo += ", ";
     }
     aTo += "mask";
   }
   aTo += ")";
 }
 #endif
--- a/layout/svg/nsSVGEffects.cpp
+++ b/layout/svg/nsSVGEffects.cpp
@@ -634,61 +634,71 @@ nsSVGEffects::GetPaintServer(nsIFrame* a
   return static_cast<nsSVGPaintServerFrame*>(result);
 }
 
 nsSVGClipPathFrame *
 nsSVGEffects::EffectProperties::GetClipPathFrame(bool* aOK)
 {
   if (!mClipPath)
     return nullptr;
-  nsSVGClipPathFrame *frame = static_cast<nsSVGClipPathFrame *>
+  nsSVGClipPathFrame *frame = static_cast<nsSVGClipPathFrame*>
     (mClipPath->GetReferencedFrame(nsGkAtoms::svgClipPathFrame, aOK));
   if (frame && aOK && *aOK) {
     *aOK = frame->IsValid();
   }
   return frame;
 }
 
-nsSVGMaskFrame *
-nsSVGEffects::EffectProperties::GetFirstMaskFrame(bool* aOK)
-{
-  if (!mMask) {
-    return nullptr;
-  }
-
-  const nsTArray<RefPtr<nsSVGPaintingProperty>>& props = mMask->GetProps();
-
-  if (props.IsEmpty()) {
-    return nullptr;
-  }
-
-  return static_cast<nsSVGMaskFrame *>
-    (props[0]->GetReferencedFrame(nsGkAtoms::svgMaskFrame, aOK));
-}
-
 nsTArray<nsSVGMaskFrame *>
 nsSVGEffects::EffectProperties::GetMaskFrames()
 {
   nsTArray<nsSVGMaskFrame *> result;
   if (!mMask)
     return result;
 
-  bool ok = false;
+  bool ok = true;
   const nsTArray<RefPtr<nsSVGPaintingProperty>>& props = mMask->GetProps();
   for (size_t i = 0; i < props.Length(); i++) {
     nsSVGMaskFrame* maskFrame =
       static_cast<nsSVGMaskFrame *>(props[i]->GetReferencedFrame(
                                                 nsGkAtoms::svgMaskFrame, &ok));
+    MOZ_ASSERT_IF(maskFrame, ok);
     result.AppendElement(maskFrame);
   }
 
   return result;
 }
 
 bool
+nsSVGEffects::EffectProperties::HasNoOrValidEffects()
+{
+  if (mClipPath) {
+    bool ok = true;
+    nsSVGClipPathFrame *frame = static_cast<nsSVGClipPathFrame *>
+      (mClipPath->GetReferencedFrame(nsGkAtoms::svgClipPathFrame, &ok));
+    if (!ok || (frame && !frame->IsValid())) {
+      return false;
+    }
+  }
+
+  if (mMask) {
+    bool ok = true;
+    const nsTArray<RefPtr<nsSVGPaintingProperty>>& props = mMask->GetProps();
+    for (size_t i = 0; i < props.Length(); i++) {
+      props[i]->GetReferencedFrame(nsGkAtoms::svgMaskFrame, &ok);
+      if (!ok) {
+        return false;
+      }
+    }
+  }
+
+  return HasNoFilterOrHasValidFilter();
+}
+
+bool
 nsSVGEffects::EffectProperties::MightHaveNoneSVGMask() const
 {
   if (!mMask) {
     return false;
   }
 
   const nsTArray<RefPtr<nsSVGPaintingProperty>>& props = mMask->GetProps();
   for (size_t i = 0; i < props.Length(); i++) {
--- a/layout/svg/nsSVGEffects.h
+++ b/layout/svg/nsSVGEffects.h
@@ -475,30 +475,37 @@ public:
 
     /**
      * @return the clip-path frame, or null if there is no clip-path frame
      * @param aOK if a clip-path was specified and the designated element
      * exists but is an element of the wrong type, *aOK is set to false.
      * Otherwise *aOK is untouched.
      */
     nsSVGClipPathFrame *GetClipPathFrame(bool* aOK);
-    /**
-     * @return the first mask frame, or null if there is no mask frame
-     * @param aOK if a mask was specified and the designated element
-     * exists but is an element of the wrong type, *aOK is set to false.
-     * Otherwise *aOK is untouched.
-     */
-    nsSVGMaskFrame *GetFirstMaskFrame(bool* aOK = nullptr);
 
     /**
      * @return an array which contains all SVG mask frames.
      */
     nsTArray<nsSVGMaskFrame*> GetMaskFrames();
 
     bool MightHaveNoneSVGMask() const;
+
+    /*
+     * @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();
+    }
+
     bool HasValidFilter() {
       return mFilter && mFilter->ReferencesValidResources();
     }
 
     bool HasNoFilterOrHasValidFilter() {
       return !mFilter || mFilter->ReferencesValidResources();
     }
   };
--- a/layout/svg/nsSVGUtils.cpp
+++ b/layout/svg/nsSVGUtils.cpp
@@ -735,24 +735,26 @@ nsSVGUtils::PaintFrameWithEffects(nsIFra
    *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. */
   nsSVGEffects::EffectProperties effectProperties =
     nsSVGEffects::GetEffectProperties(aFrame);
-  bool isOK = effectProperties.HasNoFilterOrHasValidFilter();
-  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame(&isOK);
-  nsSVGMaskFrame *maskFrame = effectProperties.GetFirstMaskFrame(&isOK);
-  if (!isOK) {
+  if (effectProperties.HasInvalidEffects()) {
     // Some resource is invalid. We shouldn't paint anything.
     return DrawResult::SUCCESS;
   }
 
+  nsSVGClipPathFrame *clipPathFrame =
+    effectProperties.GetClipPathFrame(nullptr);
+  nsTArray<nsSVGMaskFrame*> masks = effectProperties.GetMaskFrames();
+  nsSVGMaskFrame *maskFrame = masks.IsEmpty() ? nullptr : masks[0];
+
   MixModeBlender blender(aFrame, &aContext);
   gfxContext* target = blender.ShouldCreateDrawTargetForBlend()
                        ? blender.CreateBlendTarget(aTransform) : &aContext;
 
   if (!target) {
     return DrawResult::TEMPORARY_ERROR;
   }