Bug 411555 - Text inside filter causing invalidation loop. r=jwat,sr=tor,a1.9=blocking1.9+

content/svg/content/src/nsSVGGraphicElement.cpp

@@ -99,10 +99,12 @@ NS_IMETHODIMP nsSVGGraphicElement::GetBBox(nsIDOMSVGRect **_retval)
   NS_ASSERTION(svgframe, "wrong frame type");
   if (svgframe) {
     svgframe->SetMatrixPropagation(PR_FALSE);
-    svgframe->NotifyCanvasTMChanged(PR_TRUE);
+    svgframe->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                               nsISVGChildFrame::TRANSFORM_CHANGED);
     nsresult rv = svgframe->GetBBox(_retval);
     svgframe->SetMatrixPropagation(PR_TRUE);
-    svgframe->NotifyCanvasTMChanged(PR_TRUE);
+    svgframe->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                               nsISVGChildFrame::TRANSFORM_CHANGED);
     return rv;
   }
   return NS_ERROR_FAILURE;

content/svg/content/src/nsSVGLength2.h

@@ -71,6 +71,8 @@ public:
 
   PRUint8 GetCtxType() const { return mCtxType; }
   PRUint8 GetSpecifiedUnitType() const { return mSpecifiedUnitType; }
+  PRBool IsPercentage() const
+    { return mSpecifiedUnitType == nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE; }
   float GetAnimValInSpecifiedUnits() const { return mAnimVal; }
   float GetBaseValInSpecifiedUnits() const { return mBaseVal; }
 

content/svg/content/src/nsSVGSVGElement.cpp

@@ -681,10 +681,12 @@ nsSVGSVGElement::GetBBox(nsIDOMSVGRect **_retval)
   CallQueryInterface(frame, &svgframe);
   if (svgframe) {
     svgframe->SetMatrixPropagation(PR_FALSE);
-    svgframe->NotifyCanvasTMChanged(PR_TRUE);
+    svgframe->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                               nsISVGChildFrame::TRANSFORM_CHANGED);
     nsresult rv = svgframe->GetBBox(_retval);
     svgframe->SetMatrixPropagation(PR_TRUE);
-    svgframe->NotifyCanvasTMChanged(PR_TRUE);
+    svgframe->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                               nsISVGChildFrame::TRANSFORM_CHANGED);
     return rv;
   } else {
     // XXX: outer svg

layout/svg/base/src/nsISVGChildFrame.h

@@ -51,8 +51,8 @@ class nsSVGRenderState;
 struct nsRect;
 
 #define NS_ISVGCHILDFRAME_IID \
-{ 0x93560e72, 0x6818, 0x4218, \
- { 0xa1, 0xe9, 0xf3, 0xb9, 0x63, 0x6a, 0xff, 0xc2 } }
+{ 0x667e8781, 0x72bd, 0x4344, \
+ { 0x95, 0x8c, 0x69, 0xa5, 0x70, 0xc4, 0xcc, 0xb3 } }
 
 class nsISVGChildFrame : public nsISupports {
 public:
@@ -84,7 +84,19 @@ public:
   // into the frame tree (if they're inserted after the initial reflow).
   NS_IMETHOD InitialUpdate()=0;
 
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation)=0;
+  // Flags to pass to NotifySVGChange:
+  //
+  // SUPPRESS_INVALIDATION - do not invalidate rendered areas (only to be
+  //                           used in conjunction with TRANSFORM_CHANGED)
+  // TRANSFORM_CHANGED     - the current transform matrix for this frame has changed
+  // COORD_CONTEXT_CHANGED - the dimensions of this frame's coordinate context has
+  //                           changed (percentage lengths must be reevaluated)
+  enum SVGChangedFlags {
+    SUPPRESS_INVALIDATION = 0x01,
+    TRANSFORM_CHANGED     = 0x02,
+    COORD_CONTEXT_CHANGED = 0x04
+  };
+  virtual void NotifySVGChanged(PRUint32 aFlags)=0;
   NS_IMETHOD NotifyRedrawSuspended()=0;
   NS_IMETHOD NotifyRedrawUnsuspended()=0;
 

layout/svg/base/src/nsSVGAFrame.cpp

@@ -81,7 +81,7 @@ public:
   }
 #endif
   // nsISVGChildFrame interface:
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   
   // nsSVGContainerFrame methods:
   virtual already_AddRefed<nsIDOMSVGMatrix> GetCanvasTM();
@@ -123,14 +123,7 @@ nsSVGAFrame::AttributeChanged(PRInt32         aNameSpaceID,
     // make sure our cached transform matrix gets (lazily) updated
     mCanvasTM = nsnull;
     
-    nsIFrame* kid = mFrames.FirstChild();
-    while (kid) {
-      nsISVGChildFrame* SVGFrame = nsnull;
-      CallQueryInterface(kid, &SVGFrame);
-      if (SVGFrame)
-        SVGFrame->NotifyCanvasTMChanged(PR_FALSE);
-      kid = kid->GetNextSibling();
-    }
+    nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
   }
 
  return NS_OK;
@@ -153,13 +146,15 @@ nsSVGAFrame::GetType() const
 //----------------------------------------------------------------------
 // nsISVGChildFrame methods
 
-NS_IMETHODIMP
-nsSVGAFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGAFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  // make sure our cached transform matrix gets (lazily) updated
-  mCanvasTM = nsnull;
+  if (aFlags & TRANSFORM_CHANGED) {
+    // make sure our cached transform matrix gets (lazily) updated
+    mCanvasTM = nsnull;
+  }
 
-  return nsSVGAFrameBase::NotifyCanvasTMChanged(suppressInvalidation);
+  nsSVGAFrameBase::NotifySVGChanged(aFlags);
 }
 
 //----------------------------------------------------------------------

layout/svg/base/src/nsSVGClipPathFrame.cpp

@@ -100,7 +100,9 @@ nsSVGClipPathFrame::ClipPaint(nsSVGRenderState* aContext,
     nsISVGChildFrame* SVGFrame = nsnull;
     CallQueryInterface(kid, &SVGFrame);
     if (SVGFrame) {
-      SVGFrame->NotifyCanvasTMChanged(PR_TRUE);
+      // The CTM of each frame referencing us can be different.
+      SVGFrame->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION | 
+                                 nsISVGChildFrame::TRANSFORM_CHANGED);
       SVGFrame->PaintSVG(aContext, nsnull);
     }
   }
@@ -139,7 +141,7 @@ nsSVGClipPathFrame::ClipHitTest(nsISVGChildFrame* aParent,
       // Notify the child frame that we may be working with a
       // different transform, so it can update its covered region
       // (used to shortcut hit testing).
-      SVGFrame->NotifyCanvasTMChanged(PR_FALSE);
+      SVGFrame->NotifySVGChanged(nsISVGChildFrame::TRANSFORM_CHANGED);
 
       nsIFrame *temp = nsnull;
       nsresult rv = SVGFrame->GetFrameForPointSVG(aX, aY, &temp);

layout/svg/base/src/nsSVGContainerFrame.cpp

@@ -241,15 +241,17 @@ nsSVGDisplayContainerFrame::InitialUpdate()
   return NS_OK;
 }  
 
-NS_IMETHODIMP
-nsSVGDisplayContainerFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGDisplayContainerFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  if (!suppressInvalidation &&
+  NS_ASSERTION(aFlags & (TRANSFORM_CHANGED | COORD_CONTEXT_CHANGED),
+               "Invalidation logic may need adjusting");
+
+  if (!(aFlags & SUPPRESS_INVALIDATION) &&
       !(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD))
     nsSVGUtils::UpdateFilterRegion(this);
 
-  nsSVGUtils::NotifyChildrenCanvasTMChanged(this, suppressInvalidation);
-  return NS_OK;
+  nsSVGUtils::NotifyChildrenOfSVGChange(this, aFlags);
 }
 
 NS_IMETHODIMP

layout/svg/base/src/nsSVGContainerFrame.h

@@ -107,7 +107,7 @@ public:
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD NotifyRedrawSuspended();
   NS_IMETHOD NotifyRedrawUnsuspended();
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate) { return NS_ERROR_FAILURE; }

layout/svg/base/src/nsSVGFilterFrame.cpp

@@ -79,7 +79,8 @@ nsSVGFilterFrame::FilterFailCleanup(nsSVGRenderState *aContext,
 {
   aTarget->SetOverrideCTM(nsnull);
   aTarget->SetMatrixPropagation(PR_TRUE);
-  aTarget->NotifyCanvasTMChanged(PR_TRUE);
+  aTarget->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                            nsISVGChildFrame::TRANSFORM_CHANGED);
   aTarget->PaintSVG(aContext, nsnull);
 }
 
@@ -121,7 +122,8 @@ nsSVGFilterFrame::FilterPaint(nsSVGRenderState *aContext,
   nsSVGElement *target = static_cast<nsSVGElement*>(frame->GetContent());
 
   aTarget->SetMatrixPropagation(PR_FALSE);
-  aTarget->NotifyCanvasTMChanged(PR_TRUE);
+  aTarget->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                            nsISVGChildFrame::TRANSFORM_CHANGED);
 
   nsSVGFilterElement *filter = static_cast<nsSVGFilterElement*>(mContent);
 
@@ -192,7 +194,8 @@ nsSVGFilterFrame::FilterPaint(nsSVGRenderState *aContext,
                   0.0f,                         filterRes.height / height,
                   -x * filterRes.width / width, -y * filterRes.height / height);
   aTarget->SetOverrideCTM(filterTransform);
-  aTarget->NotifyCanvasTMChanged(PR_TRUE);
+  aTarget->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                            nsISVGChildFrame::TRANSFORM_CHANGED);
 
   // paint the target geometry
   nsRefPtr<gfxImageSurface> tmpSurface =
@@ -289,7 +292,8 @@ nsSVGFilterFrame::FilterPaint(nsSVGRenderState *aContext,
 
   aTarget->SetOverrideCTM(nsnull);
   aTarget->SetMatrixPropagation(PR_TRUE);
-  aTarget->NotifyCanvasTMChanged(PR_TRUE);
+  aTarget->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                            nsISVGChildFrame::TRANSFORM_CHANGED);
 
   return NS_OK;
 }
@@ -314,12 +318,14 @@ nsSVGFilterFrame::GetInvalidationRegion(nsIFrame *aTarget)
   nsCOMPtr<nsIDOMSVGRect> bbox;
 
   svg->SetMatrixPropagation(PR_FALSE);
-  svg->NotifyCanvasTMChanged(PR_TRUE);
+  svg->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                        nsISVGChildFrame::TRANSFORM_CHANGED);
 
   svg->GetBBox(getter_AddRefs(bbox));
 
   svg->SetMatrixPropagation(PR_TRUE);
-  svg->NotifyCanvasTMChanged(PR_TRUE);
+  svg->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                        nsISVGChildFrame::TRANSFORM_CHANGED);
 
   nsSVGLength2 *tmpX, *tmpY, *tmpWidth, *tmpHeight;
   tmpX = &filter->mLengthAttributes[nsSVGFilterElement::X];

layout/svg/base/src/nsSVGForeignObjectFrame.cpp

@@ -380,40 +380,46 @@ nsSVGForeignObjectFrame::InitialUpdate()
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsSVGForeignObjectFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGForeignObjectFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  mCanvasTM = nsnull;
+  PRBool reflow = PR_FALSE;
 
-  // XXX we should really have a separate notification for viewport changes and
-  // not overload NotifyCanvasTMChanged, e.g. we wouldn't need to check
-  // IsReflowLocked below. Note both notifications would be required for
-  // viewport changes when there's a viewBox though!
-  //
-  // If our width/height have a percentage value then we need to reflow if the
-  // width/height of our parent coordinate context changes. XXX Perhaps
-  // unexpectedly we also reflow if our CTM changes. This is because glyph
-  // metrics do not necessarily scale uniformly with change in scale and, as a
-  // result, CTM changes may require text to break at different points. roc
-  // says we shouldn't do this. See bug 381285 comment 20.
+  if (aFlags & TRANSFORM_CHANGED) {
+    // Perhaps unexpectedly, we reflow if our CTM changes. This is because
+    // glyph metrics do not necessarily scale uniformly with change in scale
+    // and, as a result, CTM changes may require text to break at different
+    // points.
+    // XXX roc says we shouldn't do this. See bug 381285 comment 20.
+    reflow = PR_TRUE;
+    mCanvasTM = nsnull;
 
-  UpdateGraphic(); // update mRect before requesting reflow
-
-  // If we're called while the PresShell is handling reflow events then we
-  // must have been called as a result of the NotifyViewportChange() call in
-  // our nsSVGOuterSVGFrame's Reflow() method. We must not call RequestReflow
-  // at this point (i.e. during reflow) because it could confuse the PresShell
-  // and prevent it from reflowing us properly in future. Besides that,
-  // nsSVGOuterSVGFrame::DidReflow will take care of reflowing us
-  // synchronously, so there's no need.
-
-  PRBool reflowing;
-  PresContext()->PresShell()->IsReflowLocked(&reflowing);
-  if (!reflowing) {
-    RequestReflow(nsIPresShell::eResize); // XXX use mState & NS_FRAME_IN_REFLOW?
+  } else if (aFlags & COORD_CONTEXT_CHANGED) {
+    // Our coordinate context's width/height has changed. If we have a
+    // percentage width/height our dimensions will change so we must reflow.
+    nsSVGForeignObjectElement *fO =
+      static_cast<nsSVGForeignObjectElement*>(mContent);
+    if (fO->mLengthAttributes[nsSVGForeignObjectElement::WIDTH].IsPercentage() ||
+        fO->mLengthAttributes[nsSVGForeignObjectElement::HEIGHT].IsPercentage()) {
+      reflow = PR_TRUE;
+    }
   }
 
-  return NS_OK;
+  if (reflow) {
+    // If we're called while the PresShell is handling reflow events then we
+    // must have been called as a result of the NotifyViewportChange() call in
+    // our nsSVGOuterSVGFrame's Reflow() method. We must not call RequestReflow
+    // at this point (i.e. during reflow) because it could confuse the
+    // PresShell and prevent it from reflowing us properly in future. Besides
+    // that, nsSVGOuterSVGFrame::DidReflow will take care of reflowing us
+    // synchronously, so there's no need.
+    PRBool reflowing;
+    PresContext()->PresShell()->IsReflowLocked(&reflowing);
+    if (!reflowing) {
+      UpdateGraphic(); // update mRect before requesting reflow
+      RequestReflow(nsIPresShell::eResize);
+    }
+  }
 }
 
 NS_IMETHODIMP

layout/svg/base/src/nsSVGForeignObjectFrame.h

@@ -112,7 +112,7 @@ public:
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD NotifyRedrawSuspended();
   NS_IMETHOD NotifyRedrawUnsuspended();
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate);

layout/svg/base/src/nsSVGGFrame.cpp

@@ -69,13 +69,15 @@ nsSVGGFrame::GetType() const
 //----------------------------------------------------------------------
 // nsISVGChildFrame methods
 
-NS_IMETHODIMP
-nsSVGGFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGGFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  // make sure our cached transform matrix gets (lazily) updated
-  mCanvasTM = nsnull;
+  if (aFlags & TRANSFORM_CHANGED) {
+    // make sure our cached transform matrix gets (lazily) updated
+    mCanvasTM = nsnull;
+  }
 
-  return nsSVGGFrameBase::NotifyCanvasTMChanged(suppressInvalidation);
+  nsSVGGFrameBase::NotifySVGChanged(aFlags);
 }
 
 NS_IMETHODIMP
@@ -155,13 +157,7 @@ nsSVGGFrame::AttributeChanged(PRInt32         aNameSpaceID,
     // make sure our cached transform matrix gets (lazily) updated
     mCanvasTM = nsnull;
 
-    for (nsIFrame* kid = mFrames.FirstChild(); kid;
-         kid = kid->GetNextSibling()) {
-      nsISVGChildFrame* SVGFrame = nsnull;
-      CallQueryInterface(kid, &SVGFrame);
-      if (SVGFrame)
-        SVGFrame->NotifyCanvasTMChanged(PR_FALSE);
-    }  
+    nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
   }
   
   return NS_OK;

layout/svg/base/src/nsSVGGFrame.h

@@ -73,7 +73,7 @@ public:
                               PRInt32         aModType);
 
   // nsISVGChildFrame interface:
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate);
   NS_IMETHOD SetOverrideCTM(nsIDOMSVGMatrix *aCTM);
   virtual already_AddRefed<nsIDOMSVGMatrix> GetOverrideCTM();

layout/svg/base/src/nsSVGGlyphFrame.cpp

@@ -475,12 +475,10 @@ nsSVGGlyphFrame::InitialUpdate()
   return NS_OK;
 }  
 
-NS_IMETHODIMP
-nsSVGGlyphFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGGlyphFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  UpdateGeometry(PR_TRUE, suppressInvalidation);
-  
-  return NS_OK;
+  UpdateGeometry(PR_TRUE, (aFlags & SUPPRESS_INVALIDATION) != 0);
 }
 
 NS_IMETHODIMP

layout/svg/base/src/nsSVGGlyphFrame.h

@@ -112,7 +112,7 @@ public:
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD NotifyRedrawSuspended();
   NS_IMETHOD NotifyRedrawUnsuspended();
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate) { return NS_OK; }

layout/svg/base/src/nsSVGGradientFrame.cpp

@@ -272,11 +272,13 @@ nsSVGGradientFrame::GetGradientTransform(nsSVGGeometryFrame *aSource)
       nsCOMPtr<nsIDOMSVGMatrix> matrix = frame->GetOverrideCTM();
       frame->SetMatrixPropagation(PR_FALSE);
       frame->SetOverrideCTM(nsnull);
-      frame->NotifyCanvasTMChanged(PR_TRUE);
+      frame->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                              nsISVGChildFrame::TRANSFORM_CHANGED);
       frame->GetBBox(getter_AddRefs(rect));
       frame->SetMatrixPropagation(PR_TRUE);
       frame->SetOverrideCTM(matrix);
-      frame->NotifyCanvasTMChanged(PR_TRUE);
+      frame->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                              nsISVGChildFrame::TRANSFORM_CHANGED);
     }
     if (rect) {
       float x, y, width, height;

layout/svg/base/src/nsSVGInnerSVGFrame.cpp

@@ -87,7 +87,7 @@ public:
 
   // nsISVGChildFrame interface:
   NS_IMETHOD PaintSVG(nsSVGRenderState *aContext, nsRect *aDirtyRect);
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate);
   NS_IMETHOD SetOverrideCTM(nsIDOMSVGMatrix *aCTM);
   virtual already_AddRefed<nsIDOMSVGMatrix> GetOverrideCTM();
@@ -191,13 +191,49 @@ nsSVGInnerSVGFrame::PaintSVG(nsSVGRenderState *aContext, nsRect *aDirtyRect)
   return rv;
 }
 
-NS_IMETHODIMP
-nsSVGInnerSVGFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGInnerSVGFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  // make sure our cached transform matrix gets (lazily) updated
-  mCanvasTM = nsnull;
+  if (aFlags & COORD_CONTEXT_CHANGED) {
 
-  return nsSVGInnerSVGFrameBase::NotifyCanvasTMChanged(suppressInvalidation);
+    nsSVGSVGElement *svg = static_cast<nsSVGSVGElement*>(mContent);
+
+    // Coordinate context changes affect mCanvasTM if we have a
+    // percentage 'x' or 'y', or if we have a percentage 'width' or 'height' AND
+    // a 'viewBox'.
+
+    if (!(aFlags & TRANSFORM_CHANGED) &&
+        svg->mLengthAttributes[nsSVGSVGElement::X].IsPercentage() ||
+        svg->mLengthAttributes[nsSVGSVGElement::Y].IsPercentage() ||
+        (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox) &&
+         (svg->mLengthAttributes[nsSVGSVGElement::WIDTH].IsPercentage() ||
+          svg->mLengthAttributes[nsSVGSVGElement::HEIGHT].IsPercentage()))) {
+    
+      aFlags |= TRANSFORM_CHANGED;
+    }
+
+    // XXX We could clear the COORD_CONTEXT_CHANGED flag in some circumstances
+    // if we have a non-percentage 'width' AND 'height, or if we have a 'viewBox'
+    // rect. This is because, when we have a viewBox rect, the viewBox rect
+    // is the coordinate context for our children, and it isn't changing.
+    // Percentage lengths on our children will continue to resolve to the
+    // same number of user units because they're relative to our viewBox rect. The
+    // same is true if we have a non-percentage width and height and don't have a
+    // viewBox. We (the <svg>) establish the coordinate context for our children. Our
+    // children don't care about changes to our parent coordinate context unless that
+    // change results in a change to the coordinate context that _we_ establish. Hence
+    // we can (should, really) stop propagating COORD_CONTEXT_CHANGED in these cases.
+    // We'd actually need to check that we have a viewBox rect and not just
+    // that viewBox is set, since it could be set to none.
+    // Take care not to break the testcase for bug 394463 when implementing this
+  }
+
+  if (aFlags & TRANSFORM_CHANGED) {
+    // make sure our cached transform matrix gets (lazily) updated
+    mCanvasTM = nsnull;
+  }
+
+  nsSVGInnerSVGFrameBase::NotifySVGChanged(aFlags);
 }
 
 NS_IMETHODIMP
@@ -275,19 +311,30 @@ nsSVGInnerSVGFrame::UnsuspendRedraw()
 NS_IMETHODIMP
 nsSVGInnerSVGFrame::NotifyViewportChange()
 {
+  PRUint32 flags = COORD_CONTEXT_CHANGED;
+
+#if 1
+  // XXX nsSVGSVGElement::InvalidateTransformNotifyFrame calls us for changes
+  // to 'x' and 'y'. Until this is fixed, add TRANSFORM_CHANGED to flags
+  // unconditionally.
+
+  flags |= TRANSFORM_CHANGED;
+
   // make sure canvas transform matrix gets (lazily) recalculated:
   mCanvasTM = nsnull;
+#else
+  // viewport changes only affect our transform if we have a viewBox attribute
+  if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox)) {
+    // make sure canvas transform matrix gets (lazily) recalculated:
+    mCanvasTM = nsnull;
+
+    flags |= TRANSFORM_CHANGED;
+  }
+#endif
   
   // inform children
   SuspendRedraw();
-  nsIFrame* kid = mFrames.FirstChild();
-  while (kid) {
-    nsISVGChildFrame* SVGFrame = nsnull;
-    CallQueryInterface(kid, &SVGFrame);
-    if (SVGFrame)
-      SVGFrame->NotifyCanvasTMChanged(PR_FALSE); 
-    kid = kid->GetNextSibling();
-  }
+  nsSVGUtils::NotifyChildrenOfSVGChange(this, flags);
   UnsuspendRedraw();
   return NS_OK;
 }

layout/svg/base/src/nsSVGMarkerFrame.cpp

@@ -171,7 +171,9 @@ nsSVGMarkerFrame::PaintMark(nsSVGRenderState *aContext,
     nsISVGChildFrame* SVGFrame = nsnull;
     CallQueryInterface(kid, &SVGFrame);
     if (SVGFrame) {
-      SVGFrame->NotifyCanvasTMChanged(PR_TRUE);
+      // The CTM of each frame referencing us may be different.
+      SVGFrame->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                                 nsISVGChildFrame::TRANSFORM_CHANGED);
       nsSVGUtils::PaintChildWithEffects(aContext, nsnull, kid);
     }
   }

layout/svg/base/src/nsSVGMaskFrame.cpp

@@ -112,13 +112,15 @@ nsSVGMaskFrame::ComputeMaskAlpha(nsSVGRenderState *aContext,
     if (units == nsIDOMSVGUnitTypes::SVG_UNIT_TYPE_OBJECTBOUNDINGBOX) {
 
       aParent->SetMatrixPropagation(PR_FALSE);
-      aParent->NotifyCanvasTMChanged(PR_TRUE);
+      aParent->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                                nsISVGChildFrame::TRANSFORM_CHANGED);
 
       nsCOMPtr<nsIDOMSVGRect> bbox;
       aParent->GetBBox(getter_AddRefs(bbox));
 
       aParent->SetMatrixPropagation(PR_TRUE);
-      aParent->NotifyCanvasTMChanged(PR_TRUE);
+      aParent->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                                nsISVGChildFrame::TRANSFORM_CHANGED);
 
       if (!bbox)
         return nsnull;

layout/svg/base/src/nsSVGOuterSVGFrame.cpp

@@ -227,7 +227,7 @@ nsSVGOuterSVGFrame::GetPrefWidth(nsIRenderingContext *aRenderingContext)
   nsSVGSVGElement *svg = static_cast<nsSVGSVGElement*>(mContent);
   nsSVGLength2 &width = svg->mLengthAttributes[nsSVGSVGElement::WIDTH];
 
-  if (width.GetSpecifiedUnitType() == nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE) {
+  if (width.IsPercentage()) {
     result = nscoord(0);
   } else {
     result = nsPresContext::CSSPixelsToAppUnits(width.GetAnimValue(svg));
@@ -251,7 +251,7 @@ nsSVGOuterSVGFrame::GetIntrinsicSize()
   nsSVGLength2 &width  = content->mLengthAttributes[nsSVGSVGElement::WIDTH];
   nsSVGLength2 &height = content->mLengthAttributes[nsSVGSVGElement::HEIGHT];
 
-  if (width.GetSpecifiedUnitType() == nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE) {
+  if (width.IsPercentage()) {
     float val = width.GetAnimValInSpecifiedUnits() / 100.0f;
     if (val < 0.0f) val = 0.0f;
     intrinsicSize.width.SetPercentValue(val);
@@ -261,7 +261,7 @@ nsSVGOuterSVGFrame::GetIntrinsicSize()
     intrinsicSize.width.SetCoordValue(val);
   }
 
-  if (height.GetSpecifiedUnitType() == nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE) {
+  if (height.IsPercentage()) {
     float val = height.GetAnimValInSpecifiedUnits() / 100.0f;
     if (val < 0.0f) val = 0.0f;
     intrinsicSize.height.SetPercentValue(val);
@@ -285,8 +285,7 @@ nsSVGOuterSVGFrame::GetIntrinsicRatio()
   nsSVGLength2 &width  = content->mLengthAttributes[nsSVGSVGElement::WIDTH];
   nsSVGLength2 &height = content->mLengthAttributes[nsSVGSVGElement::HEIGHT];
 
-  if (width.GetSpecifiedUnitType()  != nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE &&
-      height.GetSpecifiedUnitType() != nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE) {
+  if (!width.IsPercentage() && !height.IsPercentage()) {
     nsSize ratio(width.GetAnimValue(content), height.GetAnimValue(content));
     if (ratio.width < 0) {
       ratio.width = 0;
@@ -718,22 +717,28 @@ NS_IMETHODIMP
 nsSVGOuterSVGFrame::NotifyViewportChange()
 {
   // no point in doing anything when were not init'ed yet:
-  if (!mViewportInitialized) return NS_OK;
-
-/* XXX this caused reftest failures
-  // viewport changes only affect our transform if we have a viewBox attribute
-  nsSVGSVGElement *svgElem = static_cast<nsSVGSVGElement*>(mContent);
-  if (!svgElem->HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox)) {
+  if (!mViewportInitialized) {
     return NS_OK;
   }
-*/
 
-  // make sure canvas transform matrix gets (lazily) recalculated:
-  mCanvasTM = nsnull;
-  
+  PRUint32 flags = COORD_CONTEXT_CHANGED;
+
+  // viewport changes only affect our transform if we have a viewBox attribute
+#if 1
+  {
+#else
+  // XXX this caused reftest failures (bug 413960)
+  if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::viewBox)) {
+#endif
+    // make sure canvas transform matrix gets (lazily) recalculated:
+    mCanvasTM = nsnull;
+
+    flags |= TRANSFORM_CHANGED;
+  }
+
   // inform children
   SuspendRedraw();
-  nsSVGUtils::NotifyChildrenCanvasTMChanged(this, PR_FALSE);
+  nsSVGUtils::NotifyChildrenOfSVGChange(this, flags);
   UnsuspendRedraw();
   return NS_OK;
 }

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -470,12 +470,10 @@ nsSVGPathGeometryFrame::InitialUpdate()
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsSVGPathGeometryFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGPathGeometryFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  UpdateGraphic(suppressInvalidation);
-
-  return NS_OK;
+  UpdateGraphic((aFlags & SUPPRESS_INVALIDATION) != 0);
 }
 
 NS_IMETHODIMP

layout/svg/base/src/nsSVGPathGeometryFrame.h

@@ -109,7 +109,7 @@ protected:
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD NotifyRedrawSuspended();
   NS_IMETHOD NotifyRedrawUnsuspended();
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate);

layout/svg/base/src/nsSVGPatternFrame.cpp

@@ -781,10 +781,12 @@ nsSVGPatternFrame::GetCallerGeometry(nsIDOMSVGMatrix **aCTM,
     CallQueryInterface(aSource, &callerSVGFrame);
 
   callerSVGFrame->SetMatrixPropagation(PR_FALSE);
-  callerSVGFrame->NotifyCanvasTMChanged(PR_TRUE);
+  callerSVGFrame->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION | 
+                                   nsISVGChildFrame::TRANSFORM_CHANGED );
   callerSVGFrame->GetBBox(aBBox);
   callerSVGFrame->SetMatrixPropagation(PR_TRUE);
-  callerSVGFrame->NotifyCanvasTMChanged(PR_TRUE);
+  callerSVGFrame->NotifySVGChanged(nsISVGChildFrame::SUPPRESS_INVALIDATION |
+                                   nsISVGChildFrame::TRANSFORM_CHANGED);
 
   // Sanity check
   PRUint16 type = GetPatternUnits();

layout/svg/base/src/nsSVGTextFrame.cpp

@@ -95,15 +95,9 @@ nsSVGTextFrame::AttributeChanged(PRInt32         aNameSpaceID,
 
     // make sure our cached transform matrix gets (lazily) updated
     mCanvasTM = nsnull;
-    
-    nsIFrame* kid = mFrames.FirstChild();
-    while (kid) {
-      nsISVGChildFrame* SVGFrame = nsnull;
-      CallQueryInterface(kid, &SVGFrame);
-      if (SVGFrame)
-        SVGFrame->NotifyCanvasTMChanged(PR_FALSE);
-      kid = kid->GetNextSibling();
-    }
+
+    nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
+   
   } else if (aAttribute == nsGkAtoms::x ||
              aAttribute == nsGkAtoms::y ||
              aAttribute == nsGkAtoms::dx ||
@@ -198,19 +192,25 @@ nsSVGTextFrame::GetCharNumAtPosition(nsIDOMSVGPoint *point, PRInt32 *_retval)
 //----------------------------------------------------------------------
 // nsISVGChildFrame methods
 
-NS_IMETHODIMP
-nsSVGTextFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
+void
+nsSVGTextFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  // make sure our cached transform matrix gets (lazily) updated
-  mCanvasTM = nsnull;
+  if (aFlags & TRANSFORM_CHANGED) {
+    // make sure our cached transform matrix gets (lazily) updated
+    mCanvasTM = nsnull;
+  }
 
-  // If we are positioned using percentage values we need to update our
-  // position whenever our viewport's dimensions change.
-  // XXX we should really have a separate notification for viewport changes and
-  // not overload NotifyCanvasTMChanged.
-  NotifyGlyphMetricsChange();
+  if (aFlags & COORD_CONTEXT_CHANGED) {
+    // If we are positioned using percentage values we need to update our
+    // position whenever our viewport's dimensions change.
 
-  return nsSVGTextFrameBase::NotifyCanvasTMChanged(suppressInvalidation);
+    // XXX We could check here whether the text frame or any of its children
+    // have any percentage co-ordinates and only update if they don't. This
+    // may not be worth it as we might need to check each glyph
+    NotifyGlyphMetricsChange();
+  }
+
+  nsSVGTextFrameBase::NotifySVGChanged(aFlags);
 }
 
 NS_IMETHODIMP

layout/svg/base/src/nsSVGTextFrame.h

@@ -81,7 +81,7 @@ public:
   NS_IMETHOD SetMatrixPropagation(PRBool aPropagate);
   NS_IMETHOD SetOverrideCTM(nsIDOMSVGMatrix *aCTM);
   virtual already_AddRefed<nsIDOMSVGMatrix> GetOverrideCTM();
-  NS_IMETHOD NotifyCanvasTMChanged(PRBool suppressInvalidation);
+  virtual void NotifySVGChanged(PRUint32 aFlags);
   NS_IMETHOD NotifyRedrawSuspended();
   NS_IMETHOD NotifyRedrawUnsuspended();
   NS_IMETHOD GetBBox(nsIDOMSVGRect **_retval);

layout/svg/base/src/nsSVGTextPathFrame.cpp

@@ -202,8 +202,7 @@ nsSVGTextPathFrame::GetStartOffset()
   if (val == 0.0f)
     return 0.0;
 
-  if (length->GetSpecifiedUnitType() ==
-      nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE) {
+  if (length->IsPercentage()) {
     nsRefPtr<gfxFlattenedPath> data = GetFlattenedPath();
     return data ? (val * data->GetLength() / 100.0) : 0.0;
   } else {

layout/svg/base/src/nsSVGUseFrame.cpp

@@ -130,13 +130,7 @@ nsSVGUseFrame::AttributeChanged(PRInt32         aNameSpaceID,
     // make sure our cached transform matrix gets (lazily) updated
     mCanvasTM = nsnull;
     
-    for (nsIFrame* kid = mFrames.FirstChild(); kid;
-         kid = kid->GetNextSibling()) {
-      nsISVGChildFrame* SVGFrame = nsnull;
-      CallQueryInterface(kid, &SVGFrame);
-      if (SVGFrame)
-        SVGFrame->NotifyCanvasTMChanged(PR_FALSE);
-    }
+    nsSVGUtils::NotifyChildrenOfSVGChange(this, TRANSFORM_CHANGED);
     return NS_OK;
   }
 

layout/svg/base/src/nsSVGUtils.cpp

@@ -918,8 +918,7 @@ nsSVGUtils::ObjectSpace(nsIDOMSVGRect *aRect, nsSVGLength2 *aLength)
   }
   }
 
-  if (aLength->GetSpecifiedUnitType() ==
-      nsIDOMSVGLength::SVG_LENGTHTYPE_PERCENTAGE) {
+  if (aLength->IsPercentage()) {
     fraction = aLength->GetAnimValInSpecifiedUnits() / 100;
   } else
     fraction = aLength->GetAnimValue(static_cast<nsSVGSVGElement*>
@@ -1115,7 +1114,7 @@ nsSVGUtils::GetCanvasTM(nsIFrame *aFrame)
 }
 
 void 
-nsSVGUtils::NotifyChildrenCanvasTMChanged(nsIFrame *aFrame, PRBool suppressInvalidation)
+nsSVGUtils::NotifyChildrenOfSVGChange(nsIFrame *aFrame, PRUint32 aFlags)
 {
   nsIFrame *aKid = aFrame->GetFirstChild(nsnull);
 
@@ -1123,12 +1122,12 @@ nsSVGUtils::NotifyChildrenCanvasTMChanged(nsIFrame *aFrame, PRBool suppressInval
     nsISVGChildFrame* SVGFrame = nsnull;
     CallQueryInterface(aKid, &SVGFrame);
     if (SVGFrame) {
-      SVGFrame->NotifyCanvasTMChanged(suppressInvalidation); 
+      SVGFrame->NotifySVGChanged(aFlags); 
     } else {
       NS_ASSERTION(aKid->IsFrameOfType(nsIFrame::eSVG), "SVG frame expected");
       // recurse into the children of container frames e.g. <clipPath>, <mask>
       // in case they have child frames with transformation matrices
-      nsSVGUtils::NotifyChildrenCanvasTMChanged(aKid, suppressInvalidation);
+      nsSVGUtils::NotifyChildrenOfSVGChange(aKid, aFlags);
     }
     aKid = aKid->GetNextSibling();
   }

layout/svg/base/src/nsSVGUtils.h

@@ -354,10 +354,10 @@ public:
   static already_AddRefed<nsIDOMSVGMatrix> GetCanvasTM(nsIFrame *aFrame);
 
   /*
-   * Tells child frames that the canvasTM has changed
+   * Tells child frames that something that might affect them has changed
    */
   static void
-  NotifyChildrenCanvasTMChanged(nsIFrame *aFrame, PRBool suppressInvalidation);
+  NotifyChildrenOfSVGChange(nsIFrame *aFrame, PRUint32 aFlags);
 
   /*
    * Get frame's covered region by walking the children and doing union.