Bug 418206. Some SVG files that appear to hang Mozilla. r=longsonr@gmail.com, sr=roc@ocallahan.org, a1.9=beltzner@mozilla.com

content/svg/content/src/nsSVGMarkerElement.cpp

@@ -434,7 +434,7 @@ nsSVGMarkerElement::GetMarkerTransform(float aStrokeWidth,
 nsresult
 nsSVGMarkerElement::GetViewboxToViewportTransform(nsIDOMSVGMatrix **_retval)
 {
-  nsresult rv = NS_OK;
+  *_retval = nsnull;
 
   if (!mViewBoxToViewportTransform) {
     float viewportWidth =
@@ -452,10 +452,8 @@ nsSVGMarkerElement::GetViewboxToViewportTransform(nsIDOMSVGMatrix **_retval)
       vb->GetWidth(&viewboxWidth);
       vb->GetHeight(&viewboxHeight);
     }
-    if (viewboxWidth==0.0f || viewboxHeight==0.0f) {
-      NS_ERROR("XXX. We shouldn't get here. Viewbox width/height is set to 0. Need to disable display of element as per specs.");
-      viewboxWidth = 1.0f;
-      viewboxHeight = 1.0f;
+    if (viewboxWidth <= 0.0f || viewboxHeight <= 0.0f) {
+      return NS_ERROR_FAILURE; // invalid - don't paint element
     }
 
     float refX =
@@ -481,7 +479,7 @@ nsSVGMarkerElement::GetViewboxToViewportTransform(nsIDOMSVGMatrix **_retval)
 
   *_retval = mViewBoxToViewportTransform;
   NS_IF_ADDREF(*_retval);
-  return rv;
+  return NS_OK;
 }
 
 

content/svg/content/src/nsSVGSVGElement.cpp

@@ -1224,7 +1224,7 @@ nsSVGSVGElement::IsEventName(nsIAtom* aName)
 nsresult
 nsSVGSVGElement::GetViewboxToViewportTransform(nsIDOMSVGMatrix **_retval)
 {
-  nsresult rv = NS_OK;
+  *_retval = nsnull;
 
   float viewportWidth, viewportHeight;
   nsSVGSVGElement *ctx = GetCtx();
@@ -1252,10 +1252,8 @@ nsSVGSVGElement::GetViewboxToViewportTransform(nsIDOMSVGMatrix **_retval)
     viewboxHeight = viewportHeight;
   }
 
-  if (viewboxWidth==0.0f || viewboxHeight==0.0f) {
-    NS_ERROR("XXX. We shouldn't get here. Viewbox width/height is set to 0. Need to disable display of element as per specs.");
-    viewboxWidth = 1.0f;
-    viewboxHeight = 1.0f;
+  if (viewboxWidth <= 0.0f || viewboxHeight <= 0.0f) {
+    return NS_ERROR_FAILURE; // invalid - don't paint element
   }
 
   nsCOMPtr<nsIDOMSVGMatrix> xform =
@@ -1265,7 +1263,7 @@ nsSVGSVGElement::GetViewboxToViewportTransform(nsIDOMSVGMatrix **_retval)
                                     mPreserveAspectRatio);
   xform.swap(*_retval);
 
-  return rv;
+  return NS_OK;
 }
 
 //----------------------------------------------------------------------
@@ -1340,12 +1338,14 @@ nsSVGSVGElement::InvalidateTransformNotifyFrame()
 already_AddRefed<nsIDOMSVGRect>
 nsSVGSVGElement::GetCtxRect()
 {
+  float w, h;
   nsCOMPtr<nsIDOMSVGRect> vb;
   if (HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox)) {
     mViewBox->GetAnimVal(getter_AddRefs(vb));
+    vb->GetWidth(&w);
+    vb->GetHeight(&h);
   } else {
     nsSVGSVGElement *ctx = GetCtx();
-    float w, h;
     if (ctx) {
       w = mLengthAttributes[WIDTH].GetAnimValue(ctx);
       h = mLengthAttributes[HEIGHT].GetAnimValue(ctx);
@@ -1353,7 +1353,10 @@ nsSVGSVGElement::GetCtxRect()
       w = mViewportWidth;
       h = mViewportHeight;
     }
-    NS_NewSVGRect(getter_AddRefs(vb), 0, 0, w, h);
+  }
+
+  if (!vb || w < 0.0f || h < 0.0f) {
+    NS_NewSVGRect(getter_AddRefs(vb), 0, 0, PR_MAX(w, 0.0f), PR_MAX(h, 0.0f));
   }
 
   nsIDOMSVGRect *retval = nsnull;
@@ -1382,6 +1385,13 @@ nsSVGSVGElement::GetLength(PRUint8 aCtxType)
     }
   }
 
+  if (w < 0.0f) {
+    w = 0.0f;
+  }
+  if (h < 0.0f) {
+    h = 0.0f;
+  }
+
   switch (aCtxType) {
   case nsSVGUtils::X:
     return w;

layout/svg/base/src/nsSVGInnerSVGFrame.cpp

@@ -351,7 +351,7 @@ nsSVGInnerSVGFrame::GetCanvasTM()
     return retval;
   }
 
-  // parentTM * Translate(x,y) * viewboxToViewportTM
+  // parentTM * Translate(x,y) * viewBoxTM
 
   if (!mCanvasTM) {
     // get the transform from our parent's coordinate system to ours:
@@ -370,10 +370,16 @@ nsSVGInnerSVGFrame::GetCanvasTM()
     parentTM->Translate(x, y, getter_AddRefs(xyTM));
 
     // append the viewbox to viewport transform:
-    nsCOMPtr<nsIDOMSVGMatrix> viewBoxToViewportTM;
+    nsCOMPtr<nsIDOMSVGMatrix> viewBoxTM;
     nsSVGSVGElement *svgElement = static_cast<nsSVGSVGElement*>(mContent);
-    svgElement->GetViewboxToViewportTransform(getter_AddRefs(viewBoxToViewportTM));
-    xyTM->Multiply(viewBoxToViewportTM, getter_AddRefs(mCanvasTM));
+    nsresult res =
+      svgElement->GetViewboxToViewportTransform(getter_AddRefs(viewBoxTM));
+    if (NS_SUCCEEDED(res) && viewBoxTM) {
+      xyTM->Multiply(viewBoxTM, getter_AddRefs(mCanvasTM));
+    } else {
+      NS_WARNING("We should propagate the fact that the viewBox is invalid.");
+      mCanvasTM = xyTM;
+    }
   }    
 
   nsIDOMSVGMatrix* retval = mCanvasTM.get();

layout/svg/base/src/nsSVGMarkerFrame.cpp

@@ -104,14 +104,21 @@ nsSVGMarkerFrame::GetCanvasTM()
   element->GetMarkerTransform(mStrokeWidth, mX, mY, mAngle, getter_AddRefs(markerTM));
 
   // viewport marker
-  nsCOMPtr<nsIDOMSVGMatrix> viewTM;
-  element->GetViewboxToViewportTransform(getter_AddRefs(viewTM));
+  nsCOMPtr<nsIDOMSVGMatrix> viewBoxTM;
+  nsresult res =
+    element->GetViewboxToViewportTransform(getter_AddRefs(viewBoxTM));
 
   nsCOMPtr<nsIDOMSVGMatrix> tmpTM;
   nsCOMPtr<nsIDOMSVGMatrix> resultTM;
 
   markedTM->Multiply(markerTM, getter_AddRefs(tmpTM));
-  tmpTM->Multiply(viewTM, getter_AddRefs(resultTM));
+
+  if (NS_SUCCEEDED(res) && viewBoxTM) {
+    tmpTM->Multiply(viewBoxTM, getter_AddRefs(resultTM));
+  } else {
+    NS_WARNING("We should propagate the fact that the viewBox is invalid.");
+    resultTM = tmpTM;
+  }
 
   nsIDOMSVGMatrix *retval = resultTM.get();
   NS_IF_ADDREF(retval);
@@ -133,6 +140,27 @@ nsSVGMarkerFrame::PaintMark(nsSVGRenderState *aContext,
   if (mInUse)
     return NS_OK;
 
+  nsSVGMarkerElement *marker = static_cast<nsSVGMarkerElement*>(mContent);
+
+  nsCOMPtr<nsIDOMSVGAnimatedRect> arect;
+  nsresult rv = marker->GetViewBox(getter_AddRefs(arect));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCOMPtr<nsIDOMSVGRect> rect;
+  rv = arect->GetAnimVal(getter_AddRefs(rect));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  float x, y, width, height;
+  rect->GetX(&x);
+  rect->GetY(&y);
+  rect->GetWidth(&width);
+  rect->GetHeight(&height);
+
+  if (width <= 0.0f || height <= 0.0f) {
+    // We must disable rendering if the viewBox width or height are zero.
+    return NS_OK;
+  }
+
   AutoMarkerReferencer markerRef(this, aMarkedFrame);
 
   mStrokeWidth = aStrokeWidth;
@@ -143,22 +171,6 @@ nsSVGMarkerFrame::PaintMark(nsSVGRenderState *aContext,
   gfxContext *gfx = aContext->GetGfxContext();
 
   if (GetStyleDisplay()->IsScrollableOverflow()) {
-    nsSVGMarkerElement *marker = static_cast<nsSVGMarkerElement*>(mContent);
-
-    nsCOMPtr<nsIDOMSVGAnimatedRect> arect;
-    nsresult rv = marker->GetViewBox(getter_AddRefs(arect));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    nsCOMPtr<nsIDOMSVGRect> rect;
-    rv = arect->GetAnimVal(getter_AddRefs(rect));
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    float x, y, width, height;
-    rect->GetX(&x);
-    rect->GetY(&y);
-    rect->GetWidth(&width);
-    rect->GetHeight(&height);
-
     nsCOMPtr<nsIDOMSVGMatrix> matrix = GetCanvasTM();
     NS_ENSURE_TRUE(matrix, NS_ERROR_OUT_OF_MEMORY);
 

layout/svg/base/src/nsSVGOuterSVGFrame.cpp

@@ -307,6 +307,12 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio()
     content->mViewBox->GetAnimVal(getter_AddRefs(viewBox));
     viewBox->GetWidth(&viewBoxWidth);
     viewBox->GetHeight(&viewBoxHeight);
+    if (viewBoxWidth < 0.0f) {
+      viewBoxWidth = 0.0f;
+    }
+    if (viewBoxHeight < 0.0f) {
+      viewBoxHeight = 0.0f;
+    }
     return nsSize(viewBoxWidth, viewBoxHeight);
   }
 
@@ -763,11 +769,16 @@ nsSVGOuterSVGFrame::GetCanvasTM()
                     devPxPerCSSPx, 0.0f,
                     0.0f, devPxPerCSSPx);
 
-    nsCOMPtr<nsIDOMSVGMatrix> viewBoxMatrix;
-    svgElement->GetViewboxToViewportTransform(getter_AddRefs(viewBoxMatrix));
-
-    // PRE-multiply px conversion!
-    devPxToCSSPxMatrix->Multiply(viewBoxMatrix, getter_AddRefs(mCanvasTM));
+    nsCOMPtr<nsIDOMSVGMatrix> viewBoxTM;
+    nsresult res =
+      svgElement->GetViewboxToViewportTransform(getter_AddRefs(viewBoxTM));
+    if (NS_SUCCEEDED(res) && viewBoxTM) {
+      // PRE-multiply px conversion!
+      devPxToCSSPxMatrix->Multiply(viewBoxTM, getter_AddRefs(mCanvasTM));
+    } else {
+      NS_WARNING("We should propagate the fact that the viewBox is invalid.");
+      mCanvasTM = devPxToCSSPxMatrix;
+    }
 
     // our content is the document element so we must premultiply the values
     // of its currentScale and currentTranslate properties

layout/svg/base/src/nsSVGPatternFrame.cpp

@@ -694,7 +694,7 @@ nsSVGPatternFrame::ConstructCTM(nsIDOMSVGMatrix **aCTM,
   viewRect->GetY(&viewBoxY);
   viewRect->GetHeight(&viewBoxHeight);
   viewRect->GetWidth(&viewBoxWidth);
-  if (viewBoxHeight != 0.0f && viewBoxWidth != 0.0f) {
+  if (viewBoxHeight > 0.0f && viewBoxWidth > 0.0f) {
 
     float viewportWidth = GetLengthValue(GetWidth());
     float viewportHeight = GetLengthValue(GetHeight());

layout/svg/base/src/nsSVGUtils.cpp

@@ -998,6 +998,9 @@ nsSVGUtils::GetViewBoxTransform(float aViewportWidth, float aViewportHeight,
                                 nsIDOMSVGAnimatedPreserveAspectRatio *aPreserveAspectRatio,
                                 PRBool aIgnoreAlign)
 {
+  NS_ASSERTION(aViewboxWidth > 0, "viewBox width must be greater than zero!");
+  NS_ASSERTION(aViewboxHeight > 0, "viewBox height must be greater than zero!");
+
   PRUint16 align, meetOrSlice;
   {
     nsCOMPtr<nsIDOMSVGPreserveAspectRatio> par;