Bug 423998 - Fix repainting regression(s) and multiple invalidation bugs. r=jwatt,sr=roc,a1.9=blocking1.9+ and beltzner

layout/svg/base/src/nsSVGContainerFrame.cpp

@@ -158,7 +158,7 @@ nsSVGDisplayContainerFrame::RemoveFrame(nsIAtom* aListName,
 
   nsresult rv = nsSVGContainerFrame::RemoveFrame(aListName, aOldFrame);
 
-  if (!GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD) {
+  if (!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
     nsSVGUtils::NotifyAncestorsOfFilterRegionChange(this);
   }
 

layout/svg/base/src/nsSVGForeignObjectFrame.cpp

@@ -372,6 +372,9 @@ nsSVGForeignObjectFrame::UpdateCoveredRegion()
   // XXXjwatt: _this_ is where we should reflow _if_ mRect.width has changed!
   // we should not unconditionally reflow in AttributeChanged
   mRect = GetTransformedRegion(x, y, w, h, ctm);
+
+  nsSVGUtils::UpdateFilterRegion(this);
+
   return NS_OK;
 }
 
@@ -574,33 +577,7 @@ void nsSVGForeignObjectFrame::RequestReflow(nsIPresShell::IntrinsicDirty aType)
 
 void nsSVGForeignObjectFrame::UpdateGraphic()
 {
-  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
-    return;
-
-  nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);
-  if (!outerSVGFrame) {
-    NS_ERROR("null outerSVGFrame");
-    return;
-  }
-  
-  if (outerSVGFrame->IsRedrawSuspended()) {
-    AddStateBits(NS_STATE_SVG_DIRTY);
-  } else {
-    RemoveStateBits(NS_STATE_SVG_DIRTY);
-
-    // Invalidate the area we used to cover
-    // XXXjwatt: if we fix the following XXX, try to subtract the new region from the old here.
-    // hmm, if x then y is changed, our second call could invalidate an "old" area we never actually painted to.
-    outerSVGFrame->InvalidateCoveredRegion(this);
-
-    UpdateCoveredRegion();
-
-    // Invalidate the area we now cover
-    // XXXjwatt: when we're called due to an event that also requires reflow,
-    // we want to let reflow trigger this rasterization so it doesn't happen twice.
-    outerSVGFrame->InvalidateCoveredRegion(this);
-    nsSVGUtils::NotifyAncestorsOfFilterRegionChange(this);
-  }
+  nsSVGUtils::UpdateGraphic(this);
 
   // Clear any layout dirty region since we invalidated our whole area.
   mDirtyRegion.SetEmpty();

layout/svg/base/src/nsSVGGeometryFrame.cpp

@@ -103,6 +103,8 @@ nsSVGGeometryFrame::DidModifySVGObservable(nsISVGValue* observable,
   if (!frame)
     return NS_OK;
 
+  PRBool refresh = PR_FALSE;
+
   if (GetStateBits() & NS_STATE_SVG_FILL_PSERVER) {
     nsIFrame *ps = static_cast<nsIFrame*>(GetProperty(nsGkAtoms::fill));
     if (frame == ps) {
@@ -110,7 +112,7 @@ nsSVGGeometryFrame::DidModifySVGObservable(nsISVGValue* observable,
         DeleteProperty(nsGkAtoms::fill);
         RemoveStateBits(NS_STATE_SVG_FILL_PSERVER);
       }
-      UpdateGraphic();
+      refresh = PR_TRUE;
     }
   }
 
@@ -121,7 +123,15 @@ nsSVGGeometryFrame::DidModifySVGObservable(nsISVGValue* observable,
         DeleteProperty(nsGkAtoms::stroke);
         RemoveStateBits(NS_STATE_SVG_STROKE_PSERVER);
       }
-      UpdateGraphic();
+      refresh = PR_TRUE;
+    }
+  }
+
+  if (refresh) {
+    nsISVGChildFrame* svgFrame = nsnull;
+    CallQueryInterface(this, &svgFrame);
+    if (svgFrame) {
+      nsSVGUtils::UpdateGraphic(svgFrame);
     }
   }
 

layout/svg/base/src/nsSVGGeometryFrame.h

@@ -113,8 +113,6 @@ public:
   PRBool SetupCairoStroke(gfxContext *aContext);
 
 protected:
-  virtual nsresult UpdateGraphic(PRBool suppressInvalidation = PR_FALSE) = 0;
-
   nsSVGPaintServerFrame *GetPaintServer(const nsStyleSVGPaint *aPaint);
 
 private:

layout/svg/base/src/nsSVGGlyphFrame.cpp

@@ -36,7 +36,6 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
-#include "nsSVGOuterSVGFrame.h"
 #include "nsSVGTextFrame.h"
 #include "nsILookAndFeel.h"
 #include "nsTextFragment.h"
@@ -216,16 +215,7 @@ nsSVGGlyphFrame::CharacterDataChanged(nsPresContext*  aPresContext,
                                       PRBool          aAppend)
 {
   ClearTextRun();
-  return UpdateGraphic();
-}
-
-nsresult
-nsSVGGlyphFrame::UpdateGraphic(PRBool suppressInvalidation)
-{
-  nsSVGTextContainerFrame *containerFrame =
-    static_cast<nsSVGTextContainerFrame *>(mParent);
-  if (containerFrame)
-    containerFrame->UpdateGraphic();
+  NotifyGlyphMetricsChange();
 
   return NS_OK;
 }
@@ -240,7 +230,9 @@ nsSVGGlyphFrame::DidSetStyleContext()
 {
   nsSVGGlyphFrameBase::DidSetStyleContext();
   ClearTextRun();
-  return UpdateGraphic();
+  NotifyGlyphMetricsChange();
+
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -266,7 +258,7 @@ nsSVGGlyphFrame::SetSelected(nsPresContext* aPresContext,
   else
     mState &= ~NS_FRAME_SELECTED_CONTENT;
 
-  UpdateGeometry(PR_FALSE, PR_FALSE);
+  nsSVGUtils::UpdateGraphic(this);
 
   return NS_OK;
 }
@@ -466,7 +458,9 @@ nsSVGGlyphFrame::InitialUpdate()
 
   NS_ASSERTION(!(mState & NS_FRAME_IN_REFLOW),
                "We don't actually participate in reflow");
-  
+
+  NotifyGlyphMetricsChange();
+
   // Do unset the various reflow bits, though.
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
@@ -480,7 +474,9 @@ nsSVGGlyphFrame::NotifySVGChanged(PRUint32 aFlags)
   if (aFlags & TRANSFORM_CHANGED) {
     ClearTextRun();
   }
-  UpdateGeometry(PR_TRUE, (aFlags & SUPPRESS_INVALIDATION) != 0);
+  if (!(aFlags & SUPPRESS_INVALIDATION)) {
+    nsSVGUtils::UpdateGraphic(this);
+  }
 }
 
 NS_IMETHODIMP
@@ -494,7 +490,7 @@ NS_IMETHODIMP
 nsSVGGlyphFrame::NotifyRedrawUnsuspended()
 {
   if (GetStateBits() & NS_STATE_SVG_DIRTY)
-    UpdateGeometry(PR_TRUE, PR_FALSE);
+    nsSVGUtils::UpdateGraphic(this);
 
   return NS_OK;
 }
@@ -821,7 +817,7 @@ NS_IMETHODIMP_(void)
 nsSVGGlyphFrame::SetGlyphPosition(float x, float y)
 {
   mPosition.MoveTo(x, y);
-  UpdateGeometry(PR_TRUE, PR_FALSE);
+  nsSVGUtils::UpdateGraphic(this);
 }
 
 NS_IMETHODIMP
@@ -1142,37 +1138,13 @@ nsSVGGlyphFrame::SetWhitespaceHandling(PRUint8 aWhitespaceHandling)
 //----------------------------------------------------------------------
 //
 
-void nsSVGGlyphFrame::UpdateGeometry(PRBool bRedraw,
-                                     PRBool suppressInvalidation)
+void
+nsSVGGlyphFrame::NotifyGlyphMetricsChange()
 {
-  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
-    return;
-
-  nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);
-  if (!outerSVGFrame) {
-    NS_ERROR("null outerSVGFrame");
-    return;
-  }
-  
-  if (outerSVGFrame->IsRedrawSuspended()) {
-    AddStateBits(NS_STATE_SVG_DIRTY);
-  } else {
-    RemoveStateBits(NS_STATE_SVG_DIRTY);
-
-    if (suppressInvalidation)
-      return;
-
-    outerSVGFrame->InvalidateRect(mRect);
-    UpdateCoveredRegion();
-
-    nsRect filterRect;
-    filterRect = nsSVGUtils::FindFilterInvalidation(this);
-    if (!filterRect.IsEmpty()) {
-      outerSVGFrame->InvalidateRect(filterRect);
-    } else {
-      outerSVGFrame->InvalidateRect(mRect);
-    }
-  }  
+  nsSVGTextContainerFrame *containerFrame =
+    static_cast<nsSVGTextContainerFrame *>(mParent);
+  if (containerFrame)
+    containerFrame->NotifyGlyphMetricsChange();
 }
 
 PRBool

layout/svg/base/src/nsSVGGlyphFrame.h

@@ -130,9 +130,8 @@ public:
   NS_IMETHOD_(PRBool) IsDisplayContainer() { return PR_FALSE; }
   NS_IMETHOD_(PRBool) HasValidCoveredRect() { return PR_TRUE; }
 
-  // nsISVGGeometrySource interface: 
+  // nsSVGGeometryFrame methods
   NS_IMETHOD GetCanvasTM(nsIDOMSVGMatrix * *aCTM);
-  virtual nsresult UpdateGraphic(PRBool suppressInvalidation = PR_FALSE);
 
   // nsISVGGlyphFragmentLeaf interface:
   // These do not use the global transform if NS_STATE_NONDISPLAY_CHILD
@@ -207,8 +206,7 @@ protected:
   void FillCharacters(CharacterIterator *aIter,
                       gfxContext *aContext);
 
-  void UpdateGeometry(PRBool bRedraw, PRBool suppressInvalidation);
-  void UpdateMetrics();
+  void NotifyGlyphMetricsChange();
   PRBool ContainsPoint(float x, float y);
   PRBool GetGlobalTransform(gfxMatrix *aMatrix);
   void SetupGlobalTransform(gfxContext *aContext);

layout/svg/base/src/nsSVGImageFrame.cpp

@@ -187,7 +187,7 @@ nsSVGImageFrame::AttributeChanged(PRInt32         aNameSpaceID,
         aAttribute == nsGkAtoms::width ||
         aAttribute == nsGkAtoms::height ||
         aAttribute == nsGkAtoms::preserveAspectRatio)) {
-     UpdateGraphic();
+     nsSVGUtils::UpdateGraphic(this);
      return NS_OK;
    }
 
@@ -389,7 +389,7 @@ NS_IMETHODIMP nsSVGImageListener::OnStopDecode(imgIRequest *aRequest,
   if (!mFrame)
     return NS_ERROR_FAILURE;
 
-  mFrame->UpdateGraphic();
+  nsSVGUtils::UpdateGraphic(mFrame);
   return NS_OK;
 }
 
@@ -400,7 +400,7 @@ NS_IMETHODIMP nsSVGImageListener::FrameChanged(imgIContainer *aContainer,
   if (!mFrame)
     return NS_ERROR_FAILURE;
 
-  mFrame->UpdateGraphic();
+  nsSVGUtils::UpdateGraphic(mFrame);
   return NS_OK;
 }
 
@@ -411,7 +411,7 @@ NS_IMETHODIMP nsSVGImageListener::OnStartContainer(imgIRequest *aRequest,
     return NS_ERROR_FAILURE;
 
   mFrame->mImageContainer = aContainer;
-  mFrame->UpdateGraphic();
+  nsSVGUtils::UpdateGraphic(mFrame);
 
   return NS_OK;
 }

layout/svg/base/src/nsSVGOuterSVGFrame.cpp

@@ -109,7 +109,7 @@ nsSVGMutationObserver::AttributeChanged(nsIDocument *aDocument,
     if (metrics) {
       nsSVGTextContainerFrame *containerFrame =
         static_cast<nsSVGTextContainerFrame *>(frame);
-      containerFrame->UpdateGraphic();
+      containerFrame->NotifyGlyphMetricsChange();
       continue;
     }
     // if not, are there text elements amongst its descendents

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -162,7 +162,7 @@ nsSVGMarkerProperty::RemoveMutationObserver(nsWeakPtr aObservedMarker)
 void
 nsSVGMarkerProperty::DoUpdate()
 {
-  mFrame->UpdateGraphic();
+  nsSVGUtils::UpdateGraphic(mFrame);
 }
 
 void
@@ -248,7 +248,7 @@ nsSVGPathGeometryFrame::AttributeChanged(PRInt32         aNameSpaceID,
       (static_cast<nsSVGPathGeometryElement*>
                   (mContent)->IsDependentAttribute(aAttribute) ||
        aAttribute == nsGkAtoms::transform))
-    UpdateGraphic();
+    nsSVGUtils::UpdateGraphic(this);
 
   return NS_OK;
 }
@@ -272,7 +272,7 @@ nsSVGPathGeometryFrame::DidSetStyleContext()
   // style_hints don't map very well onto svg. Here seems to be the
   // best place to deal with style changes:
 
-  UpdateGraphic();
+  nsSVGUtils::UpdateGraphic(this);
 
   return NS_OK;
 }
@@ -447,8 +447,10 @@ nsSVGPathGeometryFrame::UpdateCoveredRegion()
   }
 
   // Add in markers
+  UpdateMarkerProperty();
   mRect = GetCoveredRegion();
 
+  nsSVGUtils::UpdateFilterRegion(this);
   return NS_OK;
 }
 
@@ -459,7 +461,7 @@ nsSVGPathGeometryFrame::InitialUpdate()
                "Yikes! We've been called already! Hopefully we weren't called "
                "before our nsSVGOuterSVGFrame's initial Reflow()!!!");
 
-  UpdateGraphic();
+  nsSVGUtils::UpdateGraphic(this);
 
   NS_ASSERTION(!(mState & NS_FRAME_IN_REFLOW),
                "We don't actually participate in reflow");
@@ -473,7 +475,9 @@ nsSVGPathGeometryFrame::InitialUpdate()
 void
 nsSVGPathGeometryFrame::NotifySVGChanged(PRUint32 aFlags)
 {
-  UpdateGraphic((aFlags & SUPPRESS_INVALIDATION) != 0);
+  if (!(aFlags & SUPPRESS_INVALIDATION)) {
+    nsSVGUtils::UpdateGraphic(this);
+  }
 }
 
 NS_IMETHODIMP
@@ -487,7 +491,7 @@ NS_IMETHODIMP
 nsSVGPathGeometryFrame::NotifyRedrawUnsuspended()
 {
   if (GetStateBits() & NS_STATE_SVG_DIRTY)
-    UpdateGraphic();
+    nsSVGUtils::UpdateGraphic(this);
 
   return NS_OK;
 }
@@ -734,40 +738,3 @@ nsSVGPathGeometryFrame::GetHittestMask()
 
   return mask;
 }
-
-//---------------------------------------------------------------------- 
-
-nsresult
-nsSVGPathGeometryFrame::UpdateGraphic(PRBool suppressInvalidation)
-{
-  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
-    return NS_OK;
-
-  nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);
-  if (!outerSVGFrame) {
-    NS_ERROR("null outerSVGFrame");
-    return NS_ERROR_FAILURE;
-  }
-
-  if (outerSVGFrame->IsRedrawSuspended()) {
-    AddStateBits(NS_STATE_SVG_DIRTY);
-  } else {
-    RemoveStateBits(NS_STATE_SVG_DIRTY);
-
-    if (suppressInvalidation)
-      return NS_OK;
-
-    outerSVGFrame->InvalidateCoveredRegion(this);
-
-    UpdateMarkerProperty();
-    UpdateCoveredRegion();
-    nsSVGUtils::UpdateFilterRegion(this);
-
-    outerSVGFrame->InvalidateCoveredRegion(this);
-    nsSVGUtils::NotifyAncestorsOfFilterRegionChange(this);
-  }
-
-  return NS_OK;
-}
-
-

layout/svg/base/src/nsSVGPathGeometryFrame.h

@@ -98,9 +98,8 @@ public:
   }
 #endif
 
-  // nsISVGGeometrySource interface:
+  // nsSVGGeometryFrame methods
   NS_IMETHOD GetCanvasTM(nsIDOMSVGMatrix * *aCTM);
-  virtual nsresult UpdateGraphic(PRBool suppressInvalidation = PR_FALSE);
 
 protected:
   // nsISVGChildFrame interface:

layout/svg/base/src/nsSVGTSpanFrame.cpp

@@ -93,7 +93,7 @@ nsSVGTSpanFrame::AttributeChanged(PRInt32         aNameSpaceID,
        aAttribute == nsGkAtoms::y ||
        aAttribute == nsGkAtoms::dx ||
        aAttribute == nsGkAtoms::dy)) {
-    UpdateGraphic();
+    NotifyGlyphMetricsChange();
   }
 
   return NS_OK;

layout/svg/base/src/nsSVGTextContainerFrame.cpp

@@ -52,7 +52,7 @@ NS_INTERFACE_MAP_BEGIN(nsSVGTextContainerFrame)
 NS_INTERFACE_MAP_END_INHERITING(nsSVGDisplayContainerFrame)
 
 void
-nsSVGTextContainerFrame::UpdateGraphic()
+nsSVGTextContainerFrame::NotifyGlyphMetricsChange()
 {
   nsSVGTextFrame *textFrame = GetTextFrame();
   if (textFrame)
@@ -141,7 +141,7 @@ nsSVGTextContainerFrame::InsertFrames(nsIAtom* aListName,
                                                          aPrevFrame,
                                                          aFrameList);
 
-  UpdateGraphic();
+  NotifyGlyphMetricsChange();
   return rv;
 }
 

layout/svg/base/src/nsSVGTextContainerFrame.h

@@ -53,7 +53,7 @@ public:
   nsSVGTextContainerFrame(nsStyleContext* aContext) :
     nsSVGDisplayContainerFrame(aContext) {}
 
-  void UpdateGraphic();
+  void NotifyGlyphMetricsChange();
   NS_IMETHOD_(already_AddRefed<nsIDOMSVGLengthList>) GetX();
   NS_IMETHOD_(already_AddRefed<nsIDOMSVGLengthList>) GetY();
   NS_IMETHOD_(already_AddRefed<nsIDOMSVGLengthList>) GetDx();

layout/svg/base/src/nsSVGTextFrame.cpp

@@ -322,6 +322,7 @@ void
 nsSVGTextFrame::NotifyGlyphMetricsChange()
 {
   mPositioningDirty = PR_TRUE;
+  UpdateGlyphPositioning(PR_FALSE);
 }
 
 static void
@@ -355,6 +356,8 @@ nsSVGTextFrame::UpdateGlyphPositioning(PRBool aForceGlobalTransform)
   nsISVGGlyphFragmentNode* node = GetFirstGlyphFragmentChildNode();
   if (!node) return;
 
+  mPositioningDirty = PR_FALSE;
+
   // we'll align every fragment in this chunk on the dominant-baseline:
   // XXX should actually inspect 'alignment-baseline' for each fragment
   
@@ -393,7 +396,6 @@ nsSVGTextFrame::UpdateGlyphPositioning(PRBool aForceGlobalTransform)
 
   firstFragment = node->GetFirstGlyphFragment();
   if (!firstFragment) {
-    mPositioningDirty = PR_FALSE;
     return;
   }
 
@@ -477,6 +479,4 @@ nsSVGTextFrame::UpdateGlyphPositioning(PRBool aForceGlobalTransform)
     }
     firstFragment = fragment;
   }
-
-  mPositioningDirty = PR_FALSE;
 }

layout/svg/base/src/nsSVGTextPathFrame.cpp

@@ -72,7 +72,7 @@ nsSVGPathListener::AttributeChanged(nsIDocument *aDocument,
                                     PRInt32 aModType,
                                     PRUint32 aStateMask)
 {
-  mTextPathFrame->UpdateGraphic();
+  mTextPathFrame->NotifyGlyphMetricsChange();
 }
 
 //----------------------------------------------------------------------
@@ -237,11 +237,11 @@ nsSVGTextPathFrame::AttributeChanged(PRInt32         aNameSpaceID,
 {
   if (aNameSpaceID == kNameSpaceID_None &&
       aAttribute == nsGkAtoms::startOffset) {
-    UpdateGraphic();
+    NotifyGlyphMetricsChange();
   } else if (aNameSpaceID == kNameSpaceID_XLink &&
              aAttribute == nsGkAtoms::href) {
     mPathListener = nsnull;
-    UpdateGraphic();
+    NotifyGlyphMetricsChange();
   }
 
   return NS_OK;

layout/svg/base/src/nsSVGUtils.cpp

@@ -888,7 +888,39 @@ nsSVGUtils::UpdateFilterRegion(nsIFrame *aFrame)
   }
 }
 
-void 
+void
+nsSVGUtils::UpdateGraphic(nsISVGChildFrame *aSVGFrame)
+{
+  nsIFrame *frame;
+  CallQueryInterface(aSVGFrame, &frame);
+
+  if (frame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
+    return;
+
+  nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(frame);
+  if (!outerSVGFrame) {
+    NS_ERROR("null outerSVGFrame");
+    return;
+  }
+
+  if (outerSVGFrame->IsRedrawSuspended()) {
+    frame->AddStateBits(NS_STATE_SVG_DIRTY);
+  } else {
+    frame->RemoveStateBits(NS_STATE_SVG_DIRTY);
+
+    // Invalidate the area we used to cover
+    outerSVGFrame->InvalidateCoveredRegion(frame);
+
+    aSVGFrame->UpdateCoveredRegion();
+
+    // Invalidate the area we now cover
+    outerSVGFrame->InvalidateCoveredRegion(frame);
+
+    NotifyAncestorsOfFilterRegionChange(frame);
+  }
+}
+
+void
 nsSVGUtils::NotifyAncestorsOfFilterRegionChange(nsIFrame *aFrame)
 {
   aFrame = aFrame->GetParent();

layout/svg/base/src/nsSVGUtils.h

@@ -270,6 +270,11 @@ public:
    */
   static void UpdateFilterRegion(nsIFrame *aFrame);
 
+  /*
+   * Update the area covered by the frame
+   */
+  static void UpdateGraphic(nsISVGChildFrame *aSVGFrame);
+
   /*
    * Update the filter invalidation region for ancestor frames, if relevant.
    */