Bug 472135. Invalidate markers using style system hints instead of doing an eager invalidation (which reacquires frames, breaking our one-shot nsSVGRenderingObservers setup). r+sr=roc

--HG--
extra : rebase_source : 48ab3b34460f5908bcec5c3e15a5d12f71496579

layout/base/nsCSSFrameConstructor.cpp

@@ -149,6 +149,8 @@
 #endif
 #ifdef MOZ_SVG
 #include "nsSVGEffects.h"
+#include "nsSVGUtils.h"
+#include "nsSVGOuterSVGFrame.h"
 #endif
 
 nsIFrame*
@@ -8815,9 +8817,20 @@ DoApplyRenderingChangeToTree(nsIFrame* aFrame,
     UpdateViewsForTree(aFrame, aViewManager, aFrameManager, aChange);
 
     // if frame has view, will already be invalidated
-    if ((aChange & nsChangeHint_RepaintFrame) &&
-        !aFrame->IsFrameOfType(nsIFrame::eSVG)) {
-      aFrame->Invalidate(aFrame->GetOverflowRect());
+    if (aChange & nsChangeHint_RepaintFrame) {
+      if (aFrame->IsFrameOfType(nsIFrame::eSVG)) {
+#ifdef MOZ_SVG
+        if (!(aFrame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
+          nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(aFrame);
+          if (outerSVGFrame) {
+            // marker changes can change the covered region
+            outerSVGFrame->UpdateAndInvalidateCoveredRegion(aFrame);
+          }
+        }
+#endif
+      } else {
+        aFrame->Invalidate(aFrame->GetOverflowRect());
+      }
     }
   }
 }

layout/reftests/svg/dynamic-filter-contents-01.svg

@@ -2,7 +2,7 @@
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/licenses/publicdomain/
 -->
-<svg class="reftest-wait" xmlns="http://www.w3.org/2000/svg" onload="setTimeout(doTest,500)">
+<svg class="reftest-wait" xmlns="http://www.w3.org/2000/svg" onload="startTest()">
   <style>
   .foo feComposite { color-interpolation-filters:sRGB; }
   </style>
@@ -18,9 +18,15 @@
   <rect width="100" height="100" fill="lime" filter="url(#f)"/>
 
   <script>
+  function startTest() {
+    document.addEventListener("MozReftestInvalidate", doTest, false);
+    // in case we're not gecko
+    setTimeout(doTest, 5000);
+  }
+
   function doTest() {
     document.getElementById("d").setAttribute("class", "foo");
-    document.documentElement.removeAttribute("class");
+    document.documentElement.removeAttribute('class');
   }
   </script>
 </svg>

layout/reftests/svg/dynamic-marker-01.svg

@@ -2,7 +2,7 @@
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/licenses/publicdomain/
 -->
-<svg xmlns="http://www.w3.org/2000/svg" version="1.1" class="reftest-wait" onload="setTimeout(doTest,10)" xmlns:xlink="http://www.w3.org/1999/xlink">
+<svg xmlns="http://www.w3.org/2000/svg" version="1.1" class="reftest-wait" onload="startTest()" xmlns:xlink="http://www.w3.org/1999/xlink">
   <title>Testing that dynamic changes to the element for a given ID are reflected in marker</title>
   <!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=309220 -->
   <defs>
@@ -16,12 +16,18 @@
   <line id="l1" x1="0" x2="0" y1="0" y2="0" fill="none" stroke="black" stroke-width="1" marker-end="url(#m1)"/>
 
   <script>
+  function startTest() {
+    document.addEventListener("MozReftestInvalidate", doTest, false);
+    // in case we're not gecko
+    setTimeout(doTest, 5000);
+  }
+  
   function doTest() {
     // check that changing an id to "m1" lets l1 find it
     var x = document.getElementById("x");
     x.setAttribute("id", "m1");
 
-    document.documentElement.removeAttribute("class");
+    document.documentElement.removeAttribute('class');
   }
   </script>
-</svg>
+</svg>

layout/reftests/svg/reftest.list

@@ -24,7 +24,7 @@ include moz-only/reftest.list
 == dynamic-clipPath-01.svg pass.svg
 == dynamic-feFlood-01.svg pass.svg
 == dynamic-feImage-01.svg pass.svg
-fails == dynamic-filter-contents-01.svg dynamic-filter-contents-01-ref.svg # bug 471631
+== dynamic-filter-contents-01.svg dynamic-filter-contents-01-ref.svg
 == dynamic-gradient-contents-01.svg pass.svg
 == dynamic-link-style-01.svg pass.svg
 == dynamic-marker-01.svg pass.svg

layout/style/nsStyleStruct.cpp

@@ -844,6 +844,14 @@ nsChangeHint nsStyleSVG::CalcDifference(const nsStyleSVG& aOther) const
     NS_UpdateHint(hint, nsChangeHint_ReflowFrame);
   }
 
+  if (!EqualURIs(mMarkerEnd, aOther.mMarkerEnd) ||
+      !EqualURIs(mMarkerMid, aOther.mMarkerMid) ||
+      !EqualURIs(mMarkerStart, aOther.mMarkerStart)) {
+    NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
+    NS_UpdateHint(hint, nsChangeHint_UpdateEffects);
+    return hint;
+  }
+
   if (mFill != aOther.mFill ||
       mStroke != aOther.mStroke) {
     NS_UpdateHint(hint, nsChangeHint_RepaintFrame);
@@ -855,11 +863,7 @@ nsChangeHint nsStyleSVG::CalcDifference(const nsStyleSVG& aOther) const
     return hint;
   }
 
-  if ( !EqualURIs(mMarkerEnd, aOther.mMarkerEnd)               ||
-       !EqualURIs(mMarkerMid, aOther.mMarkerMid)               ||
-       !EqualURIs(mMarkerStart, aOther.mMarkerStart)           ||
-
-       mStrokeDashoffset      != aOther.mStrokeDashoffset      ||
+  if ( mStrokeDashoffset      != aOther.mStrokeDashoffset      ||
        mStrokeWidth           != aOther.mStrokeWidth           ||
 
        mFillOpacity           != aOther.mFillOpacity           ||

layout/svg/base/src/nsSVGEffects.cpp

@@ -221,17 +221,14 @@ nsSVGMarkerProperty::DoUpdate()
   if (!mFrame)
     return;
 
-  if (mFrame->IsFrameOfType(nsIFrame::eSVG)) {
-    if (!(mFrame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD)) {
-      nsSVGOuterSVGFrame *outerSVGFrame = nsSVGUtils::GetOuterSVGFrame(mFrame);
-      if (outerSVGFrame) {
-        // marker changes can change the covered region
-        outerSVGFrame->UpdateAndInvalidateCoveredRegion(mFrame);
-      }
-    }
-  } else {
-    InvalidateAllContinuations(mFrame);
-  }
+  NS_ASSERTION(mFrame->IsFrameOfType(nsIFrame::eSVG), "SVG frame expected");
+
+  // Repaint asynchronously
+  nsChangeHint changeHint =
+    nsChangeHint(nsChangeHint_RepaintFrame | nsChangeHint_UpdateEffects);
+
+  mFramePresShell->FrameConstructor()->PostRestyleEvent(
+    mFrame->GetContent(), nsReStyleHint(0), changeHint);
 }
 
 void
@@ -368,10 +365,17 @@ nsSVGEffects::UpdateEffects(nsIFrame *aFrame)
 
   // Ensure that the filter is repainted correctly
   // We can't do that in DoUpdate as the referenced frame may not be valid
-  const nsStyleSVGReset *style = aFrame->GetStyleSVGReset();
-  if (style->mFilter) {
-    GetEffectProperty(style->mFilter, aFrame, nsGkAtoms::filter, CreateFilterProperty);
-  }
+  GetEffectProperty(aFrame->GetStyleSVGReset()->mFilter,
+                    aFrame, nsGkAtoms::filter, CreateFilterProperty);
+
+  // Set marker properties here to avoid reference loops
+  const nsStyleSVG *style = aFrame->GetStyleSVG();
+  GetEffectProperty(style->mMarkerStart, aFrame, nsGkAtoms::marker_start,
+                    CreateMarkerProperty);
+  GetEffectProperty(style->mMarkerMid, aFrame, nsGkAtoms::marker_mid,
+                    CreateMarkerProperty);
+  GetEffectProperty(style->mMarkerEnd, aFrame, nsGkAtoms::marker_end,
+                    CreateMarkerProperty);
 }
 
 nsSVGFilterProperty *