Bug 1322330 - Part 2. Implement EffectProperties::HasInvalidClipPath. r=longsonr+218550
authorcku <cku@mozilla.com>
Tue, 06 Dec 2016 21:28:47 -1000
changeset 325722 32b11d935bba6d03d80080d8cb3fe92ec07bf383
parent 325721 377972df07e07920a0fb59013094e105a9ddd2a0
child 325723 848bba1099dc915adb98e7dcaf24f8645c1dfd4d
push id31072
push userphilringnalda@gmail.com
push dateWed, 14 Dec 2016 03:19:56 +0000
treeherdermozilla-central@0c7a106074f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1322330, 218550
milestone53.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 1322330 - Part 2. Implement EffectProperties::HasInvalidClipPath. r=longsonr+218550 MozReview-Commit-ID: F3m4UZ0ET9x
layout/painting/nsDisplayList.cpp
layout/svg/nsSVGClipPathFrame.cpp
layout/svg/nsSVGEffects.cpp
layout/svg/nsSVGEffects.h
layout/svg/nsSVGIntegrationUtils.cpp
layout/svg/nsSVGUtils.cpp
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -7270,20 +7270,17 @@ nsDisplayMask::BuildLayer(nsDisplayListB
     return nullptr;
   }
 
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(mFrame);
   nsSVGEffects::EffectProperties effectProperties =
     nsSVGEffects::GetEffectProperties(firstFrame);
 
-  bool isOK = effectProperties.HasNoFilterOrHasValidFilter();
-  effectProperties.GetClipPathFrame(&isOK);
-
-  if (!isOK) {
+  if (effectProperties.HasInvalidClipPath()) {
     return nullptr;
   }
 
   RefPtr<ContainerLayer> container = aManager->GetLayerBuilder()->
     BuildContainerLayerFor(aBuilder, aManager, mFrame, this, &mList,
                            aContainerParameters, nullptr);
 
   return container.forget();
@@ -7433,18 +7430,17 @@ nsDisplayMask::PaintAsLayer(nsDisplayLis
 #ifdef MOZ_DUMP_PAINTING
 void
 nsDisplayMask::PrintEffects(nsACString& aTo)
 {
   nsIFrame* firstFrame =
     nsLayoutUtils::FirstContinuationOrIBSplitSibling(mFrame);
   nsSVGEffects::EffectProperties effectProperties =
     nsSVGEffects::GetEffectProperties(firstFrame);
-  bool isOK = true;
-  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame(&isOK);
+  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame();
   bool first = true;
   aTo += " effects=(";
   if (mFrame->StyleEffects()->mOpacity != 1.0f && mHandleOpacity) {
     first = false;
     aTo += nsPrintfCString("opacity(%f)", mFrame->StyleEffects()->mOpacity);
   }
   if (clipPathFrame) {
     if (!first) {
--- a/layout/svg/nsSVGClipPathFrame.cpp
+++ b/layout/svg/nsSVGClipPathFrame.cpp
@@ -153,17 +153,17 @@ nsSVGClipPathFrame::PaintClipMask(gfxCon
   // Paint this clipPath's contents into aMaskDT:
   // We need to set mMatrixForChildren here so that under the PaintSVG calls
   // on our children (below) our GetCanvasTM() method will return the correct
   // transform.
   mMatrixForChildren = GetClipPathTransform(aClippedFrame) * aMatrix;
 
   // Check if this clipPath is itself clipped by another clipPath:
   nsSVGClipPathFrame* clipPathThatClipsClipPath =
-    nsSVGEffects::GetEffectProperties(this).GetClipPathFrame(nullptr);
+    nsSVGEffects::GetEffectProperties(this).GetClipPathFrame();
   nsSVGUtils::MaskUsage maskUsage;
   nsSVGUtils::DetermineMaskUsage(this, true, maskUsage);
 
   if (maskUsage.shouldApplyClipPath) {
     clipPathThatClipsClipPath->ApplyClipPath(aMaskContext, aClippedFrame,
                                              aMatrix);
   } else if (maskUsage.shouldGenerateClipMaskLayer) {
     Matrix maskTransform;
@@ -210,23 +210,24 @@ nsSVGClipPathFrame::PaintFrameIntoMask(n
   nsISVGChildFrame* frame = do_QueryFrame(aFrame);
   if (!frame) {
     return DrawResult::SUCCESS;
   }
 
   // The CTM of each frame referencing us can be different.
   frame->NotifySVGChanged(nsISVGChildFrame::TRANSFORM_CHANGED);
 
-  bool isOK = true;
   // Children of this clipPath may themselves be clipped.
-  nsSVGClipPathFrame *clipPathThatClipsChild =
-    nsSVGEffects::GetEffectProperties(aFrame).GetClipPathFrame(&isOK);
-  if (!isOK) {
+  nsSVGEffects::EffectProperties effectProperties =
+    nsSVGEffects::GetEffectProperties(aFrame);
+  if (effectProperties.HasInvalidClipPath()) {
     return DrawResult::SUCCESS;
   }
+  nsSVGClipPathFrame *clipPathThatClipsChild =
+    effectProperties.GetClipPathFrame();
 
   nsSVGUtils::MaskUsage maskUsage;
   nsSVGUtils::DetermineMaskUsage(aFrame, true, maskUsage);
   DrawResult result = DrawResult::SUCCESS;
   if (maskUsage.shouldApplyClipPath) {
     clipPathThatClipsChild->ApplyClipPath(aTarget, aClippedFrame, aMatrix);
   } else if (maskUsage.shouldGenerateClipMaskLayer) {
     Matrix maskTransform;
@@ -323,17 +324,17 @@ nsSVGClipPathFrame::PointIsInsideClipPat
   gfxPoint point = matrix.Transform(aPoint);
 
   // clipPath elements can themselves be clipped by a different clip path. In
   // that case the other clip path further clips away the element that is being
   // clipped by the original clipPath. If this clipPath is being clipped by a
   // different clip path we need to check if it prevents the original element
   // from recieving events at aPoint:
   nsSVGClipPathFrame *clipPathFrame =
-    nsSVGEffects::GetEffectProperties(this).GetClipPathFrame(nullptr);
+    nsSVGEffects::GetEffectProperties(this).GetClipPathFrame();
   if (clipPathFrame &&
       !clipPathFrame->PointIsInsideClipPath(aClippedFrame, aPoint)) {
     return false;
   }
 
   for (nsIFrame* kid = mFrames.FirstChild(); kid;
        kid = kid->GetNextSibling()) {
     nsISVGChildFrame* SVGFrame = do_QueryFrame(kid);
@@ -355,17 +356,17 @@ nsSVGClipPathFrame::PointIsInsideClipPat
 
   return false;
 }
 
 bool
 nsSVGClipPathFrame::IsTrivial(nsISVGChildFrame **aSingleChild)
 {
   // If the clip path is clipped then it's non-trivial
-  if (nsSVGEffects::GetEffectProperties(this).GetClipPathFrame(nullptr))
+  if (nsSVGEffects::GetEffectProperties(this).GetClipPathFrame())
     return false;
 
   if (aSingleChild) {
     *aSingleChild = nullptr;
   }
 
   nsISVGChildFrame *foundChild = nullptr;
 
@@ -374,17 +375,17 @@ nsSVGClipPathFrame::IsTrivial(nsISVGChil
     nsISVGChildFrame *svgChild = do_QueryFrame(kid);
     if (svgChild) {
       // We consider a non-trivial clipPath to be one containing
       // either more than one svg child and/or a svg container
       if (foundChild || svgChild->IsDisplayContainer())
         return false;
 
       // or where the child is itself clipped
-      if (nsSVGEffects::GetEffectProperties(kid).GetClipPathFrame(nullptr))
+      if (nsSVGEffects::GetEffectProperties(kid).GetClipPathFrame())
         return false;
 
       foundChild = svgChild;
     }
   }
   if (aSingleChild) {
     *aSingleChild = foundChild;
   }
@@ -406,19 +407,17 @@ nsSVGClipPathFrame::IsValid()
 
   // And to prevent reference loops we check that this clipPath only appears
   // once in the reference chain (if any) that we're currently processing:
   AutoReferenceLimiter refLoopDetector(&mReferencing, 1);
   if (!refLoopDetector.Reference()) {
     return false; // Reference loop!
   }
 
-  bool isOK = true;
-  nsSVGEffects::GetEffectProperties(this).GetClipPathFrame(&isOK);
-  if (!isOK) {
+  if (nsSVGEffects::GetEffectProperties(this).HasInvalidClipPath()) {
     return false;
   }
 
   for (nsIFrame* kid = mFrames.FirstChild(); kid;
        kid = kid->GetNextSibling()) {
 
     nsIAtom* kidType = kid->GetType();
 
@@ -497,48 +496,51 @@ nsSVGClipPathFrame::GetClipPathTransform
 
   nsSVGEnum* clipPathUnits =
     &content->mEnumAttributes[SVGClipPathElement::CLIPPATHUNITS];
 
   return nsSVGUtils::AdjustMatrixForUnits(tm, clipPathUnits, aClippedFrame);
 }
 
 SVGBBox
-nsSVGClipPathFrame::GetBBoxForClipPathFrame(const SVGBBox &aBBox, 
+nsSVGClipPathFrame::GetBBoxForClipPathFrame(const SVGBBox &aBBox,
                                             const gfxMatrix &aMatrix)
 {
   nsIContent* node = GetContent()->GetFirstChild();
   SVGBBox unionBBox, tmpBBox;
   for (; node; node = node->GetNextSibling()) {
-    nsIFrame *frame = 
+    nsIFrame *frame =
       static_cast<nsSVGElement*>(node)->GetPrimaryFrame();
     if (frame) {
       nsISVGChildFrame *svg = do_QueryFrame(frame);
       if (svg) {
-        tmpBBox = svg->GetBBoxContribution(mozilla::gfx::ToMatrix(aMatrix), 
-                                         nsSVGUtils::eBBoxIncludeFill);
+        tmpBBox = svg->GetBBoxContribution(mozilla::gfx::ToMatrix(aMatrix),
+                                           nsSVGUtils::eBBoxIncludeFill);
         nsSVGEffects::EffectProperties effectProperties =
                               nsSVGEffects::GetEffectProperties(frame);
-        bool isOK = true;
-        nsSVGClipPathFrame *clipPathFrame = 
-                              effectProperties.GetClipPathFrame(&isOK);
-        if (clipPathFrame && isOK) {
-          tmpBBox = clipPathFrame->GetBBoxForClipPathFrame(tmpBBox, aMatrix);
-        } 
+        if (effectProperties.HasNoOrValidClipPath()) {
+          nsSVGClipPathFrame *clipPathFrame =
+            effectProperties.GetClipPathFrame();
+          if (clipPathFrame) {
+            tmpBBox = clipPathFrame->GetBBoxForClipPathFrame(tmpBBox, aMatrix);
+          }
+        }
         tmpBBox.Intersect(aBBox);
         unionBBox.UnionEdges(tmpBBox);
       }
     }
   }
-  nsSVGEffects::EffectProperties props = 
-    nsSVGEffects::GetEffectProperties(this);    
+
+  nsSVGEffects::EffectProperties props =
+    nsSVGEffects::GetEffectProperties(this);
   if (props.mClipPath) {
-    bool isOK = true;
-    nsSVGClipPathFrame *clipPathFrame = props.GetClipPathFrame(&isOK);
-    if (clipPathFrame && isOK) {
-      tmpBBox = clipPathFrame->GetBBoxForClipPathFrame(aBBox, aMatrix);                                                       
-      unionBBox.Intersect(tmpBBox);
-    } else if (!isOK) {
+    if (props.HasInvalidClipPath()) {
       unionBBox = SVGBBox();
+    } else  {
+      nsSVGClipPathFrame *clipPathFrame = props.GetClipPathFrame();
+      if (clipPathFrame) {
+        tmpBBox = clipPathFrame->GetBBoxForClipPathFrame(aBBox, aMatrix);
+        unionBBox.Intersect(tmpBBox);
+      }
     }
   }
   return unionBBox;
 }
--- a/layout/svg/nsSVGEffects.cpp
+++ b/layout/svg/nsSVGEffects.cpp
@@ -630,25 +630,24 @@ nsSVGEffects::GetPaintServer(nsIFrame* a
       type != nsGkAtoms::svgRadialGradientFrame &&
       type != nsGkAtoms::svgPatternFrame)
     return nullptr;
 
   return static_cast<nsSVGPaintServerFrame*>(result);
 }
 
 nsSVGClipPathFrame *
-nsSVGEffects::EffectProperties::GetClipPathFrame(bool* aOK)
+nsSVGEffects::EffectProperties::GetClipPathFrame()
 {
   if (!mClipPath)
     return nullptr;
-  nsSVGClipPathFrame *frame = static_cast<nsSVGClipPathFrame*>
-    (mClipPath->GetReferencedFrame(nsGkAtoms::svgClipPathFrame, aOK));
-  if (frame && aOK && *aOK) {
-    *aOK = frame->IsValid();
-  }
+
+  nsSVGClipPathFrame *frame = static_cast<nsSVGClipPathFrame *>
+    (mClipPath->GetReferencedFrame(nsGkAtoms::svgClipPathFrame, nullptr));
+
   return frame;
 }
 
 nsTArray<nsSVGMaskFrame *>
 nsSVGEffects::EffectProperties::GetMaskFrames()
 {
   nsTArray<nsSVGMaskFrame *> result;
   if (!mMask)
@@ -665,23 +664,18 @@ nsSVGEffects::EffectProperties::GetMaskF
   }
 
   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 (HasInvalidClipPath()) {
+    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) {
@@ -706,16 +700,31 @@ nsSVGEffects::EffectProperties::MightHav
         !props[i]->GetReferencedFrame(nsGkAtoms::svgMaskFrame, nullptr)) {
       return true;
     }
   }
 
   return false;
 }
 
+bool
+nsSVGEffects::EffectProperties::HasNoOrValidClipPath()
+{
+  if (mClipPath) {
+    bool ok = true;
+    nsSVGClipPathFrame *frame = static_cast<nsSVGClipPathFrame *>
+      (mClipPath->GetReferencedFrame(nsGkAtoms::svgClipPathFrame, &ok));
+    if (!ok || (frame && !frame->IsValid())) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 void
 nsSVGEffects::UpdateEffects(nsIFrame* aFrame)
 {
   NS_ASSERTION(aFrame->GetContent()->IsElement(),
                "aFrame's content should be an element");
 
   FrameProperties props = aFrame->Properties();
   props.Delete(FilterProperty());
--- a/layout/svg/nsSVGEffects.h
+++ b/layout/svg/nsSVGEffects.h
@@ -470,21 +470,18 @@ public:
 
   struct EffectProperties {
     nsSVGFilterProperty*   mFilter;
     nsSVGMaskProperty*     mMask;
     nsSVGPaintingProperty* mClipPath;
 
     /**
      * @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);
+    nsSVGClipPathFrame* GetClipPathFrame();
 
     /**
      * @return an array which contains all SVG mask frames.
      */
     nsTArray<nsSVGMaskFrame*> GetMaskFrames();
 
     bool MightHaveNoneSVGMask() const;
 
@@ -496,16 +493,29 @@ public:
 
     /*
      * @return true if we have any invalid effect.
      */
     bool HasInvalidEffects() {
       return !HasNoOrValidEffects();
     }
 
+    /*
+     * @return true if we either do not have clip-path or have a valid
+     * clip-path.
+     */
+    bool HasNoOrValidClipPath();
+
+    /*
+     * @return true if we have an invalid clip-path.
+     */
+    bool HasInvalidClipPath() {
+      return !HasNoOrValidClipPath();
+    }
+
     bool HasValidFilter() {
       return mFilter && mFilter->ReferencesValidResources();
     }
 
     bool HasNoFilterOrHasValidFilter() {
       return !mFilter || mFilter->ReferencesValidResources();
     }
   };
--- a/layout/svg/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/nsSVGIntegrationUtils.cpp
@@ -801,19 +801,17 @@ nsSVGIntegrationUtils::PaintMask(const P
     matSR.Restore();
     matSR.SetContext(&ctx);
 
     SetupContextMatrix(firstFrame, aParams, offsetToBoundingBox,
                        offsetToUserSpace, false);
     Matrix clipMaskTransform;
     gfxMatrix cssPxToDevPxMatrix = GetCSSPxToDevPxMatrix(frame);
 
-    bool isOK = true;
-    nsSVGClipPathFrame *clipPathFrame =
-      effectProperties.GetClipPathFrame(&isOK);
+    nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame();
     RefPtr<SourceSurface> maskSurface =
       maskUsage.shouldGenerateMaskLayer ? target->Snapshot() : nullptr;
     result =
       clipPathFrame->PaintClipMask(ctx, frame, cssPxToDevPxMatrix,
                                    &clipMaskTransform, maskSurface,
                                    ToMatrix(ctx.CurrentMatrix()));
   }
 
@@ -859,18 +857,17 @@ nsSVGIntegrationUtils::PaintMaskAndClipP
 
   /* 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);
   nsSVGEffects::EffectProperties effectProperties =
     nsSVGEffects::GetEffectProperties(firstFrame);
 
-  bool isOK = effectProperties.HasNoFilterOrHasValidFilter();
-  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame(&isOK);
+  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame();
 
   gfxMatrix cssPxToDevPxMatrix = GetCSSPxToDevPxMatrix(frame);
   nsTArray<nsSVGMaskFrame*> maskFrames = effectProperties.GetMaskFrames();
 
   nsPoint offsetToBoundingBox;
   nsPoint offsetToUserSpace;
 
   bool shouldGenerateMask = (maskUsage.opacity != 1.0f ||
--- a/layout/svg/nsSVGUtils.cpp
+++ b/layout/svg/nsSVGUtils.cpp
@@ -519,18 +519,17 @@ nsSVGUtils::DetermineMaskUsage(nsIFrame*
 #else
   // Since we do not support image mask so far, we should treat any
   // unresolvable mask as no mask. Otherwise, any object with a valid image
   // mask, e.g. url("xxx.png"), will become invisible just because we can not
   // handle image mask correctly. (See bug 1294171)
   aUsage.shouldGenerateMaskLayer = maskFrames.Length() == 1 && maskFrames[0];
 #endif
 
-  bool isOK = effectProperties.HasNoFilterOrHasValidFilter();
-  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame(&isOK);
+  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame();
   MOZ_ASSERT_IF(clipPathFrame,
                 svgReset->mClipPath.GetType() == StyleShapeSourceType::URL);
 
   switch (svgReset->mClipPath.GetType()) {
     case StyleShapeSourceType::URL:
       if (clipPathFrame) {
         if (clipPathFrame->IsTrivial()) {
           aUsage.shouldApplyClipPath = true;
@@ -740,18 +739,17 @@ nsSVGUtils::PaintFrameWithEffects(nsIFra
      so make sure all applicable ones are set again. */
   nsSVGEffects::EffectProperties effectProperties =
     nsSVGEffects::GetEffectProperties(aFrame);
   if (effectProperties.HasInvalidEffects()) {
     // Some resource is invalid. We shouldn't paint anything.
     return DrawResult::SUCCESS;
   }
 
-  nsSVGClipPathFrame *clipPathFrame =
-    effectProperties.GetClipPathFrame(nullptr);
+  nsSVGClipPathFrame *clipPathFrame = effectProperties.GetClipPathFrame();
   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) {
@@ -876,23 +874,23 @@ nsSVGUtils::HitTestClip(nsIFrame *aFrame
   if (!props.mClipPath) {
     const nsStyleSVGReset *style = aFrame->StyleSVGReset();
     if (style->HasClipPath()) {
       return nsCSSClipPathInstance::HitTestBasicShapeClip(aFrame, aPoint);
     }
     return true;
   }
 
-  bool isOK = true;
-  nsSVGClipPathFrame *clipPathFrame = props.GetClipPathFrame(&isOK);
-  if (!isOK) {
+  if (props.HasInvalidClipPath()) {
     // clipPath is not a valid resource, so nothing gets painted, so
     // hit-testing must fail.
     return false;
   }
+  nsSVGClipPathFrame *clipPathFrame = props.GetClipPathFrame();
+
   if (!clipPathFrame) {
     // clipPath doesn't exist, ignore it.
     return true;
   }
 
   return clipPathFrame->PointIsInsideClipPath(aFrame, aPoint);
 }
 
@@ -1124,64 +1122,61 @@ nsSVGUtils::GetBBox(nsIFrame *aFrame, ui
       matrix = element->PrependLocalTransformsTo(matrix, eChildToUserSpace);
     }
     bbox = svg->GetBBoxContribution(ToMatrix(matrix), aFlags).ToThebesRect();
     // Account for 'clipped'.
     if (aFlags & nsSVGUtils::eBBoxIncludeClipped) {
       gfxRect clipRect(0, 0, 0, 0);
       float x, y, width, height;
       gfxMatrix tm;
-      gfxRect fillBBox = 
-        svg->GetBBoxContribution(ToMatrix(tm), 
+      gfxRect fillBBox =
+        svg->GetBBoxContribution(ToMatrix(tm),
                                  nsSVGUtils::eBBoxIncludeFill).ToThebesRect();
       x = fillBBox.x;
       y = fillBBox.y;
       width = fillBBox.width;
       height = fillBBox.height;
       bool hasClip = aFrame->StyleDisplay()->IsScrollableOverflow();
       if (hasClip) {
-        clipRect = 
+        clipRect =
           nsSVGUtils::GetClipRectForFrame(aFrame, x, y, width, height);
           if (aFrame->GetType() == nsGkAtoms::svgForeignObjectFrame ||
               aFrame->GetType() == nsGkAtoms::svgUseFrame) {
             clipRect = matrix.TransformBounds(clipRect);
           }
       }
       nsSVGEffects::EffectProperties effectProperties =
         nsSVGEffects::GetEffectProperties(aFrame);
-      bool isOK = true;
-      nsSVGClipPathFrame *clipPathFrame = 
-        effectProperties.GetClipPathFrame(&isOK);
-      if (clipPathFrame && isOK) {
-        SVGClipPathElement *clipContent = 
-          static_cast<SVGClipPathElement*>(clipPathFrame->GetContent());
-        RefPtr<SVGAnimatedEnumeration> units = clipContent->ClipPathUnits();
-        if (units->AnimVal() == SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
-          matrix.Translate(gfxPoint(x, y));
-          matrix.Scale(width, height);
-        } else if (aFrame->GetType() == nsGkAtoms::svgForeignObjectFrame) {
-          matrix.Reset();
+      if (effectProperties.HasInvalidClipPath()) {
+        bbox = gfxRect(0, 0, 0, 0);
+      } else {
+        nsSVGClipPathFrame *clipPathFrame =
+          effectProperties.GetClipPathFrame();
+        if (clipPathFrame) {
+          SVGClipPathElement *clipContent =
+            static_cast<SVGClipPathElement*>(clipPathFrame->GetContent());
+          RefPtr<SVGAnimatedEnumeration> units = clipContent->ClipPathUnits();
+          if (units->AnimVal() == SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
+            matrix.Translate(gfxPoint(x, y));
+            matrix.Scale(width, height);
+          } else if (aFrame->GetType() == nsGkAtoms::svgForeignObjectFrame) {
+            matrix.Reset();
+          }
+          bbox =
+            clipPathFrame->GetBBoxForClipPathFrame(bbox, matrix).ToThebesRect();
         }
-        bbox = 
-          clipPathFrame->GetBBoxForClipPathFrame(bbox, matrix).ToThebesRect();
+
         if (hasClip) {
           bbox = bbox.Intersect(clipRect);
         }
-      } else {
-        if (!isOK) {
+
+        if (bbox.IsEmpty()) {
           bbox = gfxRect(0, 0, 0, 0);
-        } else {
-          if (hasClip) {
-            bbox = bbox.Intersect(clipRect);
-          }
         }
       }
-      if (bbox.IsEmpty()) {
-        bbox = gfxRect(0, 0, 0, 0);
-      }
     }
 
     if (aFlags == eBBoxIncludeFillGeometry) {
       // Obtaining the bbox for objectBoundingBox calculations is common so we
       // cache the result for future calls, since calculation can be expensive:
       props.Set(ObjectBoundingBoxProperty(), new gfxRect(bbox));
     }