Bug 413960, part 1 - Clean up and document various parts of misleading viewBox code. r=longsonr.

--HG--
extra : rebase_source : f9dc173b36d4afbce3a8e256f7aa90d9d231615a

content/svg/content/src/nsSVGMarkerElement.cpp

@@ -370,7 +370,7 @@ nsSVGMarkerElement::GetMarkerTransform(float aStrokeWidth,
 nsSVGViewBoxRect
 nsSVGMarkerElement::GetViewBoxRect()
 {
-  if (mViewBox.IsValid()) {
+  if (mViewBox.IsExplicitlySet()) {
     return mViewBox.GetAnimValue();
   }
   return nsSVGViewBoxRect(

content/svg/content/src/nsSVGSVGElement.cpp

@@ -976,7 +976,7 @@ nsSVGSVGElement::GetViewBoxTransform() const
   }
 
   nsSVGViewBoxRect viewBox;
-  if (mViewBox.IsValid()) {
+  if (HasViewBox()) {
     viewBox = mViewBox.GetAnimValue();
   } else {
     viewBox.x = viewBox.y = 0.0f;
@@ -1127,7 +1127,7 @@ nsSVGSVGElement::GetLength(PRUint8 aCtxType)
 {
   float h, w;
 
-  if (mViewBox.IsValid()) {
+  if (HasViewBox()) {
     const nsSVGViewBoxRect& viewbox = mViewBox.GetAnimValue();
     w = viewbox.width;
     h = viewbox.height;
@@ -1248,7 +1248,7 @@ nsSVGSVGElement::GetPreserveAspectRatio()
 bool
 nsSVGSVGElement::ShouldSynthesizeViewBox() const
 {
-  NS_ABORT_IF_FALSE(!HasValidViewbox(),
+  NS_ABORT_IF_FALSE(!HasViewBox(),
                     "Should only be called if we lack a viewBox");
 
   nsIDocument* doc = GetCurrentDoc();
@@ -1280,7 +1280,7 @@ nsSVGSVGElement::
                     "should only override preserveAspectRatio in images");
 #endif
 
-  if (!HasValidViewbox() && ShouldSynthesizeViewBox()) {
+  if (!HasViewBox() && ShouldSynthesizeViewBox()) {
     // My non-<svg:image> clients will have been painting me with a synthesized
     // viewBox, but my <svg:image> client that's about to paint me now does NOT
     // want that.  Need to tell ourselves to flush our transform.
@@ -1288,7 +1288,7 @@ nsSVGSVGElement::
   }
   mIsPaintingSVGImageElement = true;
 
-  if (!mViewBox.IsValid()) {
+  if (!HasViewBox()) {
     return; // preserveAspectRatio irrelevant (only matters if we have viewBox)
   }
 
@@ -1320,7 +1320,7 @@ nsSVGSVGElement::ClearImageOverridePreserveAspectRatio()
 #endif
 
   mIsPaintingSVGImageElement = false;
-  if (!HasValidViewbox() && ShouldSynthesizeViewBox()) {
+  if (!HasViewBox() && ShouldSynthesizeViewBox()) {
     // My non-<svg:image> clients will want to paint me with a synthesized
     // viewBox, but my <svg:image> client that just painted me did NOT
     // use that.  Need to tell ourselves to flush our transform.

content/svg/content/src/nsSVGSVGElement.h

@@ -196,8 +196,31 @@ public:
   void SyncWidthOrHeight(nsIAtom* aName, nsSVGElement *aTarget) const;
 
   // public helpers:
+
+  /**
+   * Returns true if this element has a base/anim value for its "viewBox"
+   * attribute that defines a viewBox rectangle with finite values.
+   *
+   * Note that this does not check whether we need to synthesize a viewBox,
+   * so you must call ShouldSynthesizeViewBox() if you need to check that too.
+   *
+   * Note also that this method does not pay attention to whether the width or
+   * height values of the viewBox rect are positive!
+   */
+  bool HasViewBox() const {
+    return mViewBox.IsExplicitlySet();
+  }
+
+  /**
+   * Returns true if we should synthesize a viewBox for ourselves (that is, if
+   * we're the root element in an image document, and we're not currently being
+   * painted for an <svg:image> element).
+   *
+   * Only call this method if HasViewBox() returns false.
+   */
+  bool ShouldSynthesizeViewBox() const;
+
   gfxMatrix GetViewBoxTransform() const;
-  bool      HasValidViewbox() const { return mViewBox.IsValid(); }
 
   // This services any pending notifications for the transform on on this root
   // <svg> node needing to be recalculated.  (Only applicable in
@@ -227,12 +250,6 @@ private:
   void ClearImageOverridePreserveAspectRatio();
   const SVGPreserveAspectRatio* GetImageOverridePreserveAspectRatio() const;
 
-  // Returns true if we should synthesize a viewBox for ourselves (that is,
-  // if we're the outermost <svg> in an image document, and we're not currently
-  // being painted by an <svg:image> element). This method also assumes that we
-  // lack a valid viewBox attribute.
-  bool ShouldSynthesizeViewBox() const;
-
 protected:
   // nsSVGElement overrides
   bool IsEventName(nsIAtom* aName);

content/svg/content/src/nsSVGViewBox.h

@@ -71,8 +71,17 @@ public:
 
   void Init();
 
-  // Used by element to tell if viewBox is defined
-  bool IsValid() const
+  /**
+   * Returns true if the corresponding "viewBox" attribute defined a rectangle
+   * with finite values. Returns false if the viewBox was set to an invalid
+   * string, or if any of the four rect values were too big to store in a
+   * float.
+   *
+   * This method does not check whether the width or height values are
+   * positive, so callers must check whether the viewBox rect is valid where
+   * necessary!
+   */
+  bool IsExplicitlySet() const
     { return (mHasBaseVal || mAnimVal); }
 
   const nsSVGViewBoxRect& GetBaseValue() const

layout/svg/base/src/nsSVGInnerSVGFrame.cpp

@@ -134,7 +134,7 @@ nsSVGInnerSVGFrame::NotifySVGChanged(PRUint32 aFlags)
     if (!(aFlags & TRANSFORM_CHANGED) &&
         (svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage() ||
          svg->mLengthAttributes[nsSVGSVGElement::Y].IsPercentage() ||
-         (svg->mViewBox.IsValid() &&
+         (svg->HasViewBox() &&
           (svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() ||
            svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage())))) {
     
@@ -175,7 +175,7 @@ nsSVGInnerSVGFrame::AttributeChanged(PRInt32  aNameSpaceID,
         aAttribute == nsGkAtoms::height) {
 
       nsSVGSVGElement* svg = static_cast<nsSVGSVGElement*>(mContent);
-      if (svg->mViewBox.IsValid()) {
+      if (svg->HasViewBox()) {
 
         // make sure our cached transform matrix gets (lazily) updated
         mCanvasTM = nsnull;

layout/svg/base/src/nsSVGOuterSVGFrame.cpp

@@ -289,7 +289,7 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio()
     return ratio;
   }
 
-  if (content->mViewBox.IsValid()) {
+  if (content->HasViewBox()) {
     const nsSVGViewBoxRect viewbox = content->mViewBox.GetAnimValue();
     float viewBoxWidth = viewbox.width;
     float viewBoxHeight = viewbox.height;

layout/svg/base/src/nsSVGPatternFrame.cpp

@@ -420,7 +420,7 @@ nsSVGPatternFrame::GetViewBox(nsIContent* aDefault)
   const nsSVGViewBox &thisViewBox =
     static_cast<nsSVGPatternElement *>(mContent)->mViewBox;
 
-  if (thisViewBox.IsValid())
+  if (thisViewBox.IsExplicitlySet())
     return thisViewBox;
 
   AutoPatternReferencer patternRef(this);
@@ -581,7 +581,7 @@ nsSVGPatternFrame::ConstructCTM(const gfxRect &callerBBox,
   }
 
   const nsSVGViewBox& viewBox = GetViewBox();
-  if (!viewBox.IsValid()) {
+  if (!viewBox.IsExplicitlySet()) {
     return tCTM;
   }
   const nsSVGViewBoxRect viewBoxRect = GetViewBox().GetAnimValue();

layout/svg/base/src/nsSVGUtils.cpp

@@ -1711,20 +1711,6 @@ nsSVGUtils::PathExtentsToMaxStrokeExtents(const gfxRect& aPathExtents,
 
 // ----------------------------------------------------------------------
 
-/* static */ bool
-nsSVGUtils::RootSVGElementHasViewbox(const nsIContent *aRootSVGElem)
-{
-  if (!aRootSVGElem->IsSVG(nsGkAtoms::svg)) {
-    NS_ABORT_IF_FALSE(false, "Expecting an SVG <svg> node");
-    return false;
-  }
-
-  const nsSVGSVGElement *svgSvgElem =
-    static_cast<const nsSVGSVGElement*>(aRootSVGElem);
-
-  return svgSvgElem->HasValidViewbox();
-}
-
 /* static */ void
 nsSVGUtils::GetFallbackOrPaintColor(gfxContext *aContext, nsStyleContext *aStyleContext,
                                     nsStyleSVGPaint nsStyleSVG::*aFillOrStroke,

layout/svg/base/src/nsSVGUtils.h

@@ -637,16 +637,6 @@ public:
                             NS_MIN(double(PR_INT32_MAX), aVal)));
   }
 
-  /**
-   * Given a nsIContent* that is actually an nsSVGSVGElement*, this method
-   * checks whether it currently has a valid viewBox, and returns true if so.
-   *
-   * No other type of element should be passed to this method.
-   * (In debug builds, anything non-<svg> will trigger an abort; in non-debug
-   * builds, it will trigger a false return-value as a safe fallback.)
-   */
-  static bool RootSVGElementHasViewbox(const nsIContent *aRootSVGElem);
-
   static void GetFallbackOrPaintColor(gfxContext *aContext,
                                       nsStyleContext *aStyleContext,
                                       nsStyleSVGPaint nsStyleSVG::*aFillOrStroke,