Bug 734079, part 1 - Remove the SuspendRedraw code and make SVGSVGElement.suspendRedraw a no-op. r=longsonr.

content/svg/content/src/nsSVGSVGElement.cpp

@@ -202,7 +202,6 @@ nsSVGSVGElement::nsSVGSVGElement(already_AddRefed<nsINodeInfo> aNodeInfo,
     mCurrentScale(1.0f),
     mPreviousTranslate(0.0f, 0.0f),
     mPreviousScale(1.0f),
-    mRedrawSuspendCount(0),
     mStartAnimationOnBindToTree(!aFromParser),
     mImageNeedsTransformInvalidation(false),
     mIsPaintingSVGImageElement(false)
@@ -379,20 +378,9 @@ nsSVGSVGElement::GetCurrentTranslate(nsIDOMSVGPoint * *aCurrentTranslate)
 NS_IMETHODIMP
 nsSVGSVGElement::SuspendRedraw(PRUint32 max_wait_milliseconds, PRUint32 *_retval)
 {
+  // suspendRedraw is a no-op in Mozilla, so it doesn't matter what
+  // we set the ID out-param to:
   *_retval = 1;
-
-  if (++mRedrawSuspendCount > 1) 
-    return NS_OK;
-
-  nsIFrame* frame = GetPrimaryFrame();
-  if (frame) {
-    nsISVGSVGFrame* svgframe = do_QueryFrame(frame);
-    // might fail this check if we've failed conditional processing
-    if (svgframe) {
-      svgframe->SuspendRedraw();
-    }
-  }
-  
   return NS_OK;
 }
 
@@ -400,32 +388,15 @@ nsSVGSVGElement::SuspendRedraw(PRUint32 max_wait_milliseconds, PRUint32 *_retval
 NS_IMETHODIMP
 nsSVGSVGElement::UnsuspendRedraw(PRUint32 suspend_handle_id)
 {
-  if (mRedrawSuspendCount == 0) {
-    return NS_ERROR_FAILURE;
-  }
-                 
-  if (mRedrawSuspendCount > 1) {
-    --mRedrawSuspendCount;
+  // no-op
   return NS_OK;
 }
 
-  return UnsuspendRedrawAll();
-}
-
 /* void unsuspendRedrawAll (); */
 NS_IMETHODIMP
 nsSVGSVGElement::UnsuspendRedrawAll()
 {
-  mRedrawSuspendCount = 0;
-
-  nsIFrame* frame = GetPrimaryFrame();
-  if (frame) {
-    nsISVGSVGFrame* svgframe = do_QueryFrame(frame);
-    // might fail this check if we've failed conditional processing
-    if (svgframe) {
-      svgframe->UnsuspendRedraw();
-    }
-  }  
+  // no-op
   return NS_OK;
 }
 

layout/reftests/svg/reftest.list

@@ -227,9 +227,9 @@ random-if(gtk2Widget) == objectBoundingBox-and-fePointLight-02.svg objectBoundin
 == svg-in-foreignObject-01.xhtml svg-in-foreignObject-01-ref.xhtml
 == svg-in-foreignObject-02.xhtml svg-in-foreignObject-01-ref.xhtml # reuse -01-ref.xhtml
 == switch-01.svg pass.svg
-== suspend-01.svg about:blank
+== suspend-01.svg pass.svg
 == suspend-02.svg pass.svg
-fails == suspend-03.svg pass.svg # bug 724242
+== suspend-03.svg pass.svg
 == suspend-04.svg pass.svg
 == suspend-05.svg pass.svg
 == suspend-06.svg pass.svg

layout/reftests/svg/suspend-01.svg

@@ -4,10 +4,10 @@
      http://creativecommons.org/licenses/publicdomain/
 -->
 <svg xmlns="http://www.w3.org/2000/svg">
-  <title>Test that suspendRedraw works</title>
-  <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=615146 -->
+  <title>Test that suspendRedraw doesn't apply after the end of a script</title>
+  <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=734079 -->
 
-  <rect width="100%" height="100%" fill="red"/>
+  <rect width="100%" height="100%" fill="lime"/>
   <script>
     document.documentElement.suspendRedraw(10000);
   </script>

layout/reftests/svg/suspend-02.svg

@@ -4,8 +4,8 @@
      http://creativecommons.org/licenses/publicdomain/
 -->
 <svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait">
-  <title>Test that suspendRedraw works</title>
-  <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=615146 -->
+  <title>Test that suspendRedraw doesn't apply after the end of a script</title>
+  <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=734079 -->
 
   <script type="text/javascript">
     <![CDATA[
@@ -17,12 +17,12 @@
     function doTest() {
       var svg = document.documentElement;
       svg.suspendRedraw(10000);
-      document.getElementById("r").setAttribute("fill", "red");
+      document.getElementById("r").setAttribute("fill", "lime");
       setTimeout(function() {
         svg.removeAttribute("class");
       }, 50);
     }
     ]]>
   </script>
-  <rect id="r" width="100%" height="100%" fill="lime"/>
+  <rect id="r" width="100%" height="100%" fill="red"/>
 </svg>

layout/reftests/svg/suspend-03.svg

@@ -4,8 +4,8 @@
      http://creativecommons.org/licenses/publicdomain/
 -->
 <svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait" onload="f()">
-  <title>Test that suspendRedraw works</title>
-  <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=615146 -->
+  <title>Test that suspendRedraw doesn't apply after the end of a script</title>
+  <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=734079 -->
 
   <script>
     function f() {
@@ -19,6 +19,6 @@
       }, 1);
     }
   </script>
-  <rect width="100%" height="100%" fill="red"/>
-  <rect id="r" width="100%" height="100%" fill="lime"/>
+  <rect width="100%" height="100%" fill="lime"/>
+  <rect id="r" width="100%" height="100%" fill="red"/>
 </svg>

layout/reftests/svg/suspend-04.svg

@@ -4,7 +4,7 @@
      http://creativecommons.org/licenses/publicdomain/
 -->
 <svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait">
-  <title>Test that suspendRedraw works</title>
+  <title>Test that suspendRedraw doesn't apply after the end of a script</title>
   <!-- https://bugzilla.mozilla.org/show_bug.cgi?id=615146 -->
 
   <script type="text/javascript">
@@ -21,12 +21,12 @@
         var r = document.createElementNS("http://www.w3.org/2000/svg", "rect");
         r.setAttribute("width", "100%");
         r.setAttribute("height", "100%");
-        r.setAttribute("fill", "red");
+        r.setAttribute("fill", "lime");
         svg.appendChild(r);
         svg.removeAttribute("class");
       }, 50);
     }
     ]]>
   </script>
-  <rect width="100%" height="100%" fill="lime"/>
+  <rect width="100%" height="100%" fill="red"/>
 </svg>

layout/svg/base/src/nsISVGChildFrame.h

@@ -112,8 +112,6 @@ public:
     COORD_CONTEXT_CHANGED = 0x04
   };
   virtual void NotifySVGChanged(PRUint32 aFlags)=0;
-  virtual void NotifyRedrawSuspended()=0;
-  virtual void NotifyRedrawUnsuspended()=0;
 
   /**
    * Get this frame's contribution to the rect returned by a GetBBox() call

layout/svg/base/src/nsISVGSVGFrame.h

@@ -46,8 +46,6 @@ class nsISVGSVGFrame
 public:
   NS_DECL_QUERYFRAME_TARGET(nsISVGSVGFrame)
 
-  virtual void SuspendRedraw()=0;        
-  virtual void UnsuspendRedraw()=0;      
   virtual void NotifyViewportChange()=0; 
 };
 

layout/svg/base/src/nsSVGContainerFrame.cpp

@@ -101,8 +101,7 @@ nsSVGDisplayContainerFrame::Init(nsIContent* aContent,
 {
   if (!(GetStateBits() & NS_STATE_IS_OUTER_SVG)) {
     AddStateBits(aParent->GetStateBits() &
-      (NS_STATE_SVG_NONDISPLAY_CHILD | NS_STATE_SVG_CLIPPATH_CHILD |
-       NS_STATE_SVG_REDRAW_SUSPENDED));
+      (NS_STATE_SVG_NONDISPLAY_CHILD | NS_STATE_SVG_CLIPPATH_CHILD));
   }
   nsresult rv = nsSVGContainerFrame::Init(aContent, aParent, aPrevInFlow);
   return rv;
@@ -142,9 +141,6 @@ NS_IMETHODIMP
 nsSVGDisplayContainerFrame::RemoveFrame(ChildListID aListID,
                                         nsIFrame* aOldFrame)
 {
-  // Force the invalidation before it's too late
-  RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
   nsSVGUtils::InvalidateCoveredRegion(aOldFrame);
 
   nsresult rv = nsSVGContainerFrame::RemoveFrame(aListID, aOldFrame);
@@ -207,6 +203,20 @@ nsSVGDisplayContainerFrame::InitialUpdate()
                "Yikes! We've been called already! Hopefully we weren't called "
                "before our nsSVGOuterSVGFrame's initial Reflow()!!!");
 
+  // If the NS_FRAME_FIRST_REFLOW bit has been removed from our parent frame,
+  // then our outer-<svg> has previously had its initial reflow. In that case
+  // we need to make sure that that bit has been removed from ourself _before_
+  // recursing over our children to ensure that they know too. Otherwise, we
+  // need to remove it _after_ recursing over our children so that they know
+  // the initial reflow is currently underway.
+
+  bool outerSVGHasHadFirstReflow =
+    (GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW) == 0;
+
+  if (outerSVGHasHadFirstReflow) {
+    mState &= ~NS_FRAME_FIRST_REFLOW; // tell our children
+  }
+
   for (nsIFrame* kid = mFrames.FirstChild(); kid;
        kid = kid->GetNextSibling()) {
     nsISVGChildFrame* SVGFrame = do_QueryFrame(kid);
@@ -218,7 +228,6 @@ nsSVGDisplayContainerFrame::InitialUpdate()
   NS_ASSERTION(!(mState & NS_FRAME_IN_REFLOW),
                "We don't actually participate in reflow");
 
-  // Do unset the various reflow bits, though.
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
   
@@ -238,18 +247,6 @@ nsSVGDisplayContainerFrame::NotifySVGChanged(PRUint32 aFlags)
   nsSVGUtils::NotifyChildrenOfSVGChange(this, aFlags);
 }
 
-void
-nsSVGDisplayContainerFrame::NotifyRedrawSuspended()
-{
-  nsSVGUtils::NotifyRedrawSuspended(this);
-}
-
-void
-nsSVGDisplayContainerFrame::NotifyRedrawUnsuspended()
-{
-  nsSVGUtils::NotifyRedrawUnsuspended(this);
-}
-
 gfxRect
 nsSVGDisplayContainerFrame::GetBBoxContribution(
   const gfxMatrix &aToBBoxUserspace,

layout/svg/base/src/nsSVGContainerFrame.h

@@ -119,8 +119,6 @@ public:
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual void NotifyRedrawSuspended();
-  virtual void NotifyRedrawUnsuspended();
   virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return true; }

layout/svg/base/src/nsSVGForeignObjectFrame.cpp

@@ -48,6 +48,7 @@
 #include "nsIURI.h"
 #include "nsSVGRect.h"
 #include "nsINameSpaceManager.h"
+#include "nsSVGEffects.h"
 #include "nsSVGForeignObjectElement.h"
 #include "nsSVGContainerFrame.h"
 #include "gfxContext.h"
@@ -91,9 +92,9 @@ nsSVGForeignObjectFrame::Init(nsIContent* aContent,
 
   nsresult rv = nsSVGForeignObjectFrameBase::Init(aContent, aParent, aPrevInFlow);
   AddStateBits(aParent->GetStateBits() &
-               (NS_STATE_SVG_NONDISPLAY_CHILD | NS_STATE_SVG_CLIPPATH_CHILD |
-                NS_STATE_SVG_REDRAW_SUSPENDED));
-  if (NS_SUCCEEDED(rv)) {
+               (NS_STATE_SVG_NONDISPLAY_CHILD | NS_STATE_SVG_CLIPPATH_CHILD));
+  if (NS_SUCCEEDED(rv) &&
+      !(mState & NS_STATE_SVG_NONDISPLAY_CHILD)) {
     nsSVGUtils::GetOuterSVGFrame(this)->RegisterForeignObject(this);
   }
   return rv;
@@ -101,7 +102,10 @@ nsSVGForeignObjectFrame::Init(nsIContent* aContent,
 
 void nsSVGForeignObjectFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
+  // Only unregister if we registered in the first place:
+  if (!(mState & NS_STATE_SVG_NONDISPLAY_CHILD)) {
     nsSVGUtils::GetOuterSVGFrame(this)->UnregisterForeignObject(this);
+  }
   nsSVGForeignObjectFrameBase::DestroyFrom(aDestructRoot);
 }
 
@@ -155,6 +159,9 @@ nsSVGForeignObjectFrame::Reflow(nsPresContext*           aPresContext,
                                 const nsHTMLReflowState& aReflowState,
                                 nsReflowStatus&          aStatus)
 {
+  NS_ABORT_IF_FALSE(!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD),
+                    "Should not have been called");
+
   // InitialUpdate and AttributeChanged make sure mRect is up to date before
   // we're called (UpdateCoveredRegion sets mRect).
 
@@ -183,12 +190,26 @@ nsSVGForeignObjectFrame::InvalidateInternal(const nsRect& aDamageRect,
 {
   // This is called by our descendants when they change.
 
-  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
+  if (GetStateBits() & NS_FRAME_FIRST_REFLOW) {
+    // When our outer-<svg> gets its first reflow its entire area
+    // will be invalidated.
     return;
+  }
+
+  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD) {
+    nsSVGEffects::InvalidateRenderingObservers(this);
+    return;
+  }
 
   nsRegion* region = (aFlags & INVALIDATE_CROSS_DOC)
     ? &mSubDocDirtyRegion : &mSameDocDirtyRegion;
   region->Or(*region, aDamageRect + nsPoint(aX, aY));
+
+  // XXXjwatt: Why are we calling FlushDirtyRegion here? Don't we only get
+  // called under DoReflow? In which case the FlushDirtyRegion call at the end
+  // of that method should be sufficient, no? And what is the point in having
+  // the mSubDocDirtyRegion/mSameDocDirtyRegion members if we flush immediately
+  // after we add anything to them?
   FlushDirtyRegion(aFlags);
 }
 
@@ -375,19 +396,39 @@ nsSVGForeignObjectFrame::InitialUpdate()
                "Yikes! We've been called already! Hopefully we weren't called "
                "before our nsSVGOuterSVGFrame's initial Reflow()!!!");
 
+  // XXX make this an NS_ABORT_IF_FALSE after fixing the failure
+  // caused by layout/base/crashtests/615146-1.html
+  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD) {
+    return NS_OK;
+  }
+
+  // Do this before the DoReflow call so that DoReflow uses the correct
+  // dimensions:
   UpdateCoveredRegion();
 
   // Make sure to not allow interrupts if we're not being reflown as a root
   nsPresContext::InterruptPreventer noInterrupts(PresContext());
+
+  // This also calls nsSVGUtils::UpdateGraphic, which calls
+  // nsSVGEffects::InvalidateRenderingObservers (but returns without
+  // unnecessarily invalidating and re-updating our covered region,
+  // since we haven't removed NS_FRAME_FIRST_REFLOW yet):
   DoReflow();
 
   NS_ASSERTION(!(mState & NS_FRAME_IN_REFLOW),
                "We don't actually participate in reflow");
 
-  // Do unset the various reflow bits, though.
+  // Now unset the various reflow bits:
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
 
+  if (!(GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    // We only invalidate if our outer-<svg> has already had its
+    // initial reflow (since if it hasn't, its entire area will be
+    // invalidated when it gets that initial reflow):
+    nsSVGUtils::InvalidateCoveredRegion(this);
+  }
+
   return NS_OK;
 }
 
@@ -448,26 +489,6 @@ nsSVGForeignObjectFrame::NotifySVGChanged(PRUint32 aFlags)
   }
 }
 
-void
-nsSVGForeignObjectFrame::NotifyRedrawSuspended()
-{
-  AddStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-}
-
-void
-nsSVGForeignObjectFrame::NotifyRedrawUnsuspended()
-{
-  RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  if (!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
-    if (GetStateBits() & NS_STATE_SVG_DIRTY) {
-      UpdateGraphic(); // invalidate our entire area
-    } else {
-      FlushDirtyRegion(0); // only invalidate areas dirtied by our descendants
-    }
-  }
-}
-
 gfxRect
 nsSVGForeignObjectFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                              PRUint32 aFlags)
@@ -545,6 +566,9 @@ void nsSVGForeignObjectFrame::UpdateGraphic()
 void
 nsSVGForeignObjectFrame::MaybeReflowFromOuterSVGFrame()
 {
+  NS_ABORT_IF_FALSE(!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD),
+                    "Should not have been called");
+
   // If IsDisabled() is true, then we know that our DoReflow() call will return
   // early, leaving us with a marked-dirty but not-reflowed kid. That'd be bad;
   // it'd mean that all future calls to this method would be doomed to take the
@@ -577,18 +601,11 @@ nsSVGForeignObjectFrame::MaybeReflowFromOuterSVGFrame()
 void
 nsSVGForeignObjectFrame::DoReflow()
 {
-  NS_ASSERTION(!(nsSVGUtils::GetOuterSVGFrame(this)->
-                             GetStateBits() & NS_FRAME_FIRST_REFLOW),
-               "Calling InitialUpdate too early - must not call DoReflow!!!");
-
   // Skip reflow if we're zero-sized, unless this is our first reflow.
   if (IsDisabled() &&
       !(GetStateBits() & NS_FRAME_FIRST_REFLOW))
     return;
 
-  if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
-    return;
-
   nsPresContext *presContext = PresContext();
   nsIFrame* kid = GetFirstPrincipalChild();
   if (!kid)
@@ -643,8 +660,11 @@ nsSVGForeignObjectFrame::DoReflow()
                     NS_FRAME_NO_MOVE_FRAME);
   
   mInReflow = false;
+
+  if (!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
     FlushDirtyRegion(0);
   }
+}
 
 void
 nsSVGForeignObjectFrame::InvalidateDirtyRect(nsSVGOuterSVGFrame* aOuter,
@@ -672,9 +692,11 @@ nsSVGForeignObjectFrame::InvalidateDirtyRect(nsSVGOuterSVGFrame* aOuter,
 void
 nsSVGForeignObjectFrame::FlushDirtyRegion(PRUint32 aFlags)
 {
+  NS_ABORT_IF_FALSE(!(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD),
+                    "Should not have been called");
+
   if ((mSameDocDirtyRegion.IsEmpty() && mSubDocDirtyRegion.IsEmpty()) ||
-      mInReflow ||
-      (GetStateBits() & NS_STATE_SVG_REDRAW_SUSPENDED))
+      mInReflow)
     return;
 
   nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(this);

layout/svg/base/src/nsSVGForeignObjectFrame.h

@@ -129,8 +129,6 @@ public:
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual void NotifyRedrawSuspended();
-  virtual void NotifyRedrawUnsuspended();
   virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return true; }

layout/svg/base/src/nsSVGGeometryFrame.cpp

@@ -54,8 +54,7 @@ nsSVGGeometryFrame::Init(nsIContent* aContent,
                          nsIFrame* aPrevInFlow)
 {
   AddStateBits(aParent->GetStateBits() &
-               (NS_STATE_SVG_NONDISPLAY_CHILD | NS_STATE_SVG_CLIPPATH_CHILD |
-                NS_STATE_SVG_REDRAW_SUSPENDED));
+               (NS_STATE_SVG_NONDISPLAY_CHILD | NS_STATE_SVG_CLIPPATH_CHILD));
   nsresult rv = nsSVGGeometryFrameBase::Init(aContent, aParent, aPrevInFlow);
   return rv;
 }

layout/svg/base/src/nsSVGGlyphFrame.cpp

@@ -511,10 +511,22 @@ nsSVGGlyphFrame::InitialUpdate()
   NS_ASSERTION(!(mState & NS_FRAME_IN_REFLOW),
                "We don't actually participate in reflow");
 
-  // Do unset the various reflow bits, though.
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
 
+  if (!(GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    // We only invalidate if our outer-<svg> has already had its
+    // initial reflow (since if it hasn't, its entire area will be
+    // invalidated when it gets that initial reflow):
+    nsSVGUtils::InvalidateCoveredRegion(this);
+
+    // We also only call UpdateCoveredRegion here, since if our
+    // outer-<svg> has yet to receive its initial reflow,
+    // we're going to get updated when our nsSVGTextFrame's
+    // InitialUpdate calls UpdateGlyphPositioning.
+    UpdateCoveredRegion();
+  }
+
   return NS_OK;
 }  
 
@@ -536,21 +548,6 @@ nsSVGGlyphFrame::NotifySVGChanged(PRUint32 aFlags)
   }
 }
 
-void
-nsSVGGlyphFrame::NotifyRedrawSuspended()
-{
-  AddStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-}
-
-void
-nsSVGGlyphFrame::NotifyRedrawUnsuspended()
-{
-  RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  if (GetStateBits() & NS_STATE_SVG_DIRTY)
-    nsSVGUtils::UpdateGraphic(this);
-}
-
 void
 nsSVGGlyphFrame::AddBoundingBoxesToPath(CharacterIterator *aIter,
                                         gfxContext *aContext)

layout/svg/base/src/nsSVGGlyphFrame.h

@@ -183,8 +183,6 @@ public:
   NS_IMETHOD_(nsRect) GetCoveredRegion();
   NS_IMETHOD InitialUpdate();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual void NotifyRedrawSuspended();
-  virtual void NotifyRedrawUnsuspended();
   NS_IMETHOD_(bool) IsDisplayContainer() { return false; }
   NS_IMETHOD_(bool) HasValidCoveredRect() {
     return !(GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD);

layout/svg/base/src/nsSVGInnerSVGFrame.cpp

@@ -232,24 +232,6 @@ nsSVGInnerSVGFrame::GetFrameForPoint(const nsPoint &aPoint)
 //----------------------------------------------------------------------
 // nsISVGSVGFrame methods:
 
-void
-nsSVGInnerSVGFrame::SuspendRedraw()
-{
-  if (GetParent()->GetStateBits() & NS_STATE_SVG_REDRAW_SUSPENDED)
-    return;
-
-  nsSVGUtils::NotifyRedrawSuspended(this);
-}
-
-void
-nsSVGInnerSVGFrame::UnsuspendRedraw()
-{
-  if (GetParent()->GetStateBits() & NS_STATE_SVG_REDRAW_SUSPENDED)
-    return;
-
-  nsSVGUtils::NotifyRedrawUnsuspended(this);
-}
-
 void
 nsSVGInnerSVGFrame::NotifyViewportChange()
 {

layout/svg/base/src/nsSVGInnerSVGFrame.h

@@ -91,8 +91,6 @@ public:
   virtual gfxMatrix GetCanvasTM();
 
   // nsISVGSVGFrame interface:
-  virtual void SuspendRedraw();
-  virtual void UnsuspendRedraw();
   virtual void NotifyViewportChange();
 
 protected:

layout/svg/base/src/nsSVGOuterSVGFrame.cpp

@@ -141,7 +141,6 @@ NS_IMPL_FRAMEARENA_HELPERS(nsSVGOuterSVGFrame)
 
 nsSVGOuterSVGFrame::nsSVGOuterSVGFrame(nsStyleContext* aContext)
     : nsSVGOuterSVGFrameBase(aContext)
-    , mRedrawSuspendCount(0)
     , mFullZoom(0)
     , mViewportInitialized(false)
 #ifdef XP_MACOSX
@@ -184,8 +183,6 @@ nsSVGOuterSVGFrame::Init(nsIContent* aContent,
     doc->AddMutationObserverUnlessExists(&sSVGMutationObserver);
   }
 
-  SuspendRedraw();  // UnsuspendRedraw is in DidReflow
-
   return rv;
 }
 
@@ -432,6 +429,11 @@ nsSVGOuterSVGFrame::DidReflow(nsPresContext*   aPresContext,
   nsresult rv = nsSVGOuterSVGFrameBase::DidReflow(aPresContext,aReflowState,aStatus);
 
   if (firstReflow) {
+    // Temporarily add back the NS_FRAME_FIRST_REFLOW bit to indicate
+    // to the children that we are still to receive the invalidation
+    // for our first reflow:
+    AddStateBits(NS_FRAME_FIRST_REFLOW);
+
     // call InitialUpdate() on all frames:
     nsIFrame* kid = mFrames.FirstChild();
     while (kid) {
@@ -442,7 +444,8 @@ nsSVGOuterSVGFrame::DidReflow(nsPresContext*   aPresContext,
       kid = kid->GetNextSibling();
     }
 
-    UnsuspendRedraw(); // For the SuspendRedraw in InitSVG
+    // And now remove it again:
+    RemoveStateBits(NS_FRAME_FIRST_REFLOW);
   } else {
     // Now that all viewport establishing descendants have their correct size,
     // tell our foreignObject descendants to reflow their children.
@@ -686,26 +689,6 @@ nsSVGOuterSVGFrame::GetType() const
 //----------------------------------------------------------------------
 // nsISVGSVGFrame methods:
 
-void
-nsSVGOuterSVGFrame::SuspendRedraw()
-{
-  if (++mRedrawSuspendCount != 1)
-    return;
-
-  nsSVGUtils::NotifyRedrawSuspended(this);
-}
-
-void
-nsSVGOuterSVGFrame::UnsuspendRedraw()
-{
-  NS_ASSERTION(mRedrawSuspendCount >=0, "unbalanced suspend count!");
-
-  if (--mRedrawSuspendCount > 0)
-    return;
-
-  nsSVGUtils::NotifyRedrawUnsuspended(this);
-}
-
 void
 nsSVGOuterSVGFrame::NotifyViewportChange()
 {

layout/svg/base/src/nsSVGOuterSVGFrame.h

@@ -125,8 +125,6 @@ public:
                                PRInt32         aModType);
 
   // nsISVGSVGFrame interface:
-  virtual void SuspendRedraw();
-  virtual void UnsuspendRedraw();
   virtual void NotifyViewportChange();
 
   // nsSVGContainerFrame methods:
@@ -176,7 +174,6 @@ protected:
 
   nsAutoPtr<gfxMatrix> mCanvasTM;
 
-  PRInt32 mRedrawSuspendCount;
   float mFullZoom;
 
   bool mViewportInitialized;

layout/svg/base/src/nsSVGPathGeometryFrame.cpp

@@ -233,14 +233,22 @@ nsSVGPathGeometryFrame::InitialUpdate()
                "Yikes! We've been called already! Hopefully we weren't called "
                "before our nsSVGOuterSVGFrame's initial Reflow()!!!");
 
-  nsSVGUtils::UpdateGraphic(this);
-
   NS_ASSERTION(!(mState & NS_FRAME_IN_REFLOW),
                "We don't actually participate in reflow");
   
-  // Do unset the various reflow bits, though.
   mState &= ~(NS_FRAME_FIRST_REFLOW | NS_FRAME_IS_DIRTY |
               NS_FRAME_HAS_DIRTY_CHILDREN);
+
+  nsSVGEffects::InvalidateRenderingObservers(this);
+  UpdateCoveredRegion();
+
+  if (!(GetParent()->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
+    // We only invalidate if our outer-<svg> has already had its
+    // initial reflow (since if it hasn't, its entire area will be
+    // invalidated when it gets that initial reflow):
+    nsSVGUtils::InvalidateCoveredRegion(this);
+  }
+
   return NS_OK;
 }
 
@@ -259,21 +267,6 @@ nsSVGPathGeometryFrame::NotifySVGChanged(PRUint32 aFlags)
   }
 }
 
-void
-nsSVGPathGeometryFrame::NotifyRedrawSuspended()
-{
-  AddStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-}
-
-void
-nsSVGPathGeometryFrame::NotifyRedrawUnsuspended()
-{
-  RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  if (GetStateBits() & NS_STATE_SVG_DIRTY)
-    nsSVGUtils::UpdateGraphic(this);
-}
-
 gfxRect
 nsSVGPathGeometryFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                             PRUint32 aFlags)

layout/svg/base/src/nsSVGPathGeometryFrame.h

@@ -97,8 +97,6 @@ protected:
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual void NotifyRedrawSuspended();
-  virtual void NotifyRedrawUnsuspended();
   virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
   NS_IMETHOD_(bool) IsDisplayContainer() { return false; }

layout/svg/base/src/nsSVGSwitchFrame.cpp

@@ -36,6 +36,7 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+#include "nsSVGEffects.h"
 #include "nsSVGGFrame.h"
 #include "nsSVGSwitchElement.h"
 #include "nsSVGUtils.h"
@@ -83,7 +84,6 @@ public:
   NS_IMETHODIMP_(nsRect) GetCoveredRegion();
   NS_IMETHOD UpdateCoveredRegion();
   NS_IMETHOD InitialUpdate();
-  virtual void NotifyRedrawUnsuspended();
   virtual gfxRect GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags);
 
@@ -184,22 +184,13 @@ nsSVGSwitchFrame::UpdateCoveredRegion()
 NS_IMETHODIMP
 nsSVGSwitchFrame::InitialUpdate()
 {
-  nsSVGUtils::UpdateGraphic(this);
+  static_cast<nsSVGSwitchElement*>(mContent)->UpdateActiveChild();
+
+  nsSVGEffects::InvalidateRenderingObservers(this);
 
   return nsSVGSwitchFrameBase::InitialUpdate();
 }
 
-void
-nsSVGSwitchFrame::NotifyRedrawUnsuspended()
-{
-  RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  if (GetStateBits() & NS_STATE_SVG_DIRTY)
-    nsSVGUtils::UpdateGraphic(this);
-
-  nsSVGSwitchFrameBase::NotifyRedrawUnsuspended();
-}
-
 gfxRect
 nsSVGSwitchFrame::GetBBoxContribution(const gfxMatrix &aToBBoxUserspace,
                                       PRUint32 aFlags)

layout/svg/base/src/nsSVGTextFrame.cpp

@@ -216,15 +216,6 @@ nsSVGTextFrame::NotifySVGChanged(PRUint32 aFlags)
   }
 }
 
-void
-nsSVGTextFrame::NotifyRedrawUnsuspended()
-{
-  RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  UpdateGlyphPositioning(false);
-  nsSVGTextFrameBase::NotifyRedrawUnsuspended();
-}
-
 NS_IMETHODIMP
 nsSVGTextFrame::PaintSVG(nsRenderingContext* aContext,
                          const nsIntRect *aDirtyRect)
@@ -253,8 +244,13 @@ nsSVGTextFrame::UpdateCoveredRegion()
 NS_IMETHODIMP
 nsSVGTextFrame::InitialUpdate()
 {
+  // Removes NS_FRAME_FIRST_REFLOW from our descendants and us:
   nsresult rv = nsSVGTextFrameBase::InitialUpdate();
 
+  // With NS_FRAME_FIRST_REFLOW removed, this will update text
+  // positions, covered regions, and invalidate. The invalidation
+  // part is redundant work if our outer-<svg> hasn't had its
+  // first reflow, but that's not too bad.
   UpdateGlyphPositioning(false);
 
   return rv;
@@ -344,7 +340,7 @@ nsSVGTextFrame::SetWhitespaceHandling(nsSVGGlyphFrame *aFrame)
 void
 nsSVGTextFrame::UpdateGlyphPositioning(bool aForceGlobalTransform)
 {
-  if ((GetStateBits() & NS_STATE_SVG_REDRAW_SUSPENDED) || !mPositioningDirty)
+  if (!mPositioningDirty)
     return;
 
   mPositioningDirty = false;

layout/svg/base/src/nsSVGTextFrame.h

@@ -86,7 +86,6 @@ public:
 
   // nsISVGChildFrame interface:
   virtual void NotifySVGChanged(PRUint32 aFlags);
-  virtual void NotifyRedrawUnsuspended();
   // Override these four to ensure that UpdateGlyphPositioning is called
   // to bring glyph positions up to date
   NS_IMETHOD PaintSVG(nsRenderingContext* aContext,

layout/svg/base/src/nsSVGUtils.cpp

@@ -657,13 +657,12 @@ nsSVGUtils::InvalidateCoveredRegion(nsIFrame *aFrame)
   if (aFrame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
     return;
 
-  if (aFrame->GetStateBits() & NS_STATE_SVG_REDRAW_SUSPENDED) {
-    aFrame->AddStateBits(NS_STATE_SVG_DIRTY);
+  if (aFrame->GetStateBits() & NS_FRAME_FIRST_REFLOW) {
+    // The nsSVGOuterSVGFrame has not received its first reflow,
+    // and when it does its entire area will be invalidated.
     return;
   }
 
-  aFrame->RemoveStateBits(NS_STATE_SVG_DIRTY);
-
   nsSVGOuterSVGFrame* outerSVGFrame = GetOuterSVGFrame(aFrame);
   NS_ASSERTION(outerSVGFrame, "no outer svg frame");
   if (outerSVGFrame) {
@@ -688,13 +687,12 @@ nsSVGUtils::UpdateGraphic(nsIFrame *aFrame)
   if (aFrame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)
     return;
 
-  if (aFrame->GetStateBits() & NS_STATE_SVG_REDRAW_SUSPENDED) {
-    aFrame->AddStateBits(NS_STATE_SVG_DIRTY);
+  if (aFrame->GetStateBits() & NS_FRAME_FIRST_REFLOW) {
+    // The nsSVGOuterSVGFrame has not received its first reflow,
+    // and when it does its entire area will be invalidated.
     return;
   }
 
-  aFrame->RemoveStateBits(NS_STATE_SVG_DIRTY);
-
   nsISVGChildFrame *svgFrame = do_QueryFrame(aFrame);
   if (!svgFrame)
     return;
@@ -973,38 +971,6 @@ nsSVGUtils::NotifyChildrenOfSVGChange(nsIFrame *aFrame, PRUint32 aFlags)
   }
 }
 
-void
-nsSVGUtils::NotifyRedrawSuspended(nsIFrame *aFrame)
-{
-  aFrame->AddStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  nsIFrame *kid = aFrame->GetFirstPrincipalChild();
-
-  while (kid) {
-    nsISVGChildFrame* SVGFrame = do_QueryFrame(kid);
-    if (SVGFrame) {
-      SVGFrame->NotifyRedrawSuspended();
-    }
-    kid = kid->GetNextSibling();
-  }
-}
-
-void
-nsSVGUtils::NotifyRedrawUnsuspended(nsIFrame *aFrame)
-{
-  aFrame->RemoveStateBits(NS_STATE_SVG_REDRAW_SUSPENDED);
-
-  nsIFrame *kid = aFrame->GetFirstPrincipalChild();
-
-  while (kid) {
-    nsISVGChildFrame* SVGFrame = do_QueryFrame(kid);
-    if (SVGFrame) {
-      SVGFrame->NotifyRedrawUnsuspended();
-    }
-    kid = kid->GetNextSibling();
-  }
-}
-
 // ************************************************************
 
 class SVGPaintCallback : public nsSVGFilterPaintCallback

layout/svg/base/src/nsSVGUtils.h

@@ -94,17 +94,12 @@ class Element;
 // SVG Frame state bits
 #define NS_STATE_IS_OUTER_SVG                    NS_FRAME_STATE_BIT(20)
 
-#define NS_STATE_SVG_DIRTY                       NS_FRAME_STATE_BIT(21)
-
 /* are we the child of a non-display container? */
 #define NS_STATE_SVG_NONDISPLAY_CHILD            NS_FRAME_STATE_BIT(22)
 
 // If this bit is set, we are a <clipPath> element or descendant.
 #define NS_STATE_SVG_CLIPPATH_CHILD              NS_FRAME_STATE_BIT(23)
 
-// If this bit is set, redraw is suspended.
-#define NS_STATE_SVG_REDRAW_SUSPENDED            NS_FRAME_STATE_BIT(24)
-
 /**
  * Byte offsets of channels in a native packed gfxColor or cairo image surface.
  */
@@ -399,19 +394,6 @@ public:
   static void
   NotifyChildrenOfSVGChange(nsIFrame *aFrame, PRUint32 aFlags);
 
-  /*
-   * Tells child frames that redraw is suspended
-   */
-  static void
-  NotifyRedrawSuspended(nsIFrame *aFrame);
-
-  /*
-   * Tells child frames that redraw is no longer suspended
-   * @return true if any of the child frames are dirty
-   */
-  static void
-  NotifyRedrawUnsuspended(nsIFrame *aFrame);
-
   /*
    * Get frame's covered region by walking the children and doing union.
    */