Bug 779971 - Make nsSVGTextPathProperty::DoUpdate trigger nsSVGTextFrame::NotifyGlyphMetricsChange() off an asynchronous change hint (to avoid calling nsLayoutUtils::FrameNeedsReflow synchronously under nsISVGChildFrame::ReflowSVG or during frame teardown, and avoid infinite loops caused by using an event queue event). r=jwatt.
authorRobert Longson <longsonr@gmail.com>
Wed, 07 Nov 2012 09:53:44 +0000
changeset 112549 fd836a1f49c9aa678a95cc3093548d3d3e572e7e
parent 112548 557e4cdad7c5e74442ed2602e11b8ce09362f673
child 112550 d6f15467b587e8ca44c910c9a3b3b3653e708fda
push id17643
push userjwatt@jwatt.org
push dateWed, 07 Nov 2012 16:56:26 +0000
treeherdermozilla-inbound@fd836a1f49c9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs779971
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 779971 - Make nsSVGTextPathProperty::DoUpdate trigger nsSVGTextFrame::NotifyGlyphMetricsChange() off an asynchronous change hint (to avoid calling nsLayoutUtils::FrameNeedsReflow synchronously under nsISVGChildFrame::ReflowSVG or during frame teardown, and avoid infinite loops caused by using an event queue event). r=jwatt.
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsChangeHint.h
layout/svg/crashtests/779971-1.svg
layout/svg/crashtests/crashtests.list
layout/svg/nsSVGEffects.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -111,16 +111,17 @@
 #include "nsXBLService.h"
 
 #undef NOISY_FIRST_LETTER
 
 #include "nsMathMLParts.h"
 #include "nsIDOMSVGFilters.h"
 #include "DOMSVGTests.h"
 #include "nsSVGEffects.h"
+#include "nsSVGTextPathFrame.h"
 #include "nsSVGUtils.h"
 
 #include "nsRefreshDriver.h"
 #include "nsRuleProcessorData.h"
 #include "sampler.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -7792,16 +7793,22 @@ DoApplyRenderingChangeToTree(nsIFrame* a
           // Just invalidate our area:
           nsSVGUtils::InvalidateBounds(aFrame);
         }
       } else {
         needInvalidatingPaint = true;
         aFrame->InvalidateFrameSubtree();
       }
     }
+    if (aChange & nsChangeHint_UpdateTextPath) {
+      NS_ABORT_IF_FALSE(aFrame->GetType() == nsGkAtoms::svgTextPathFrame,
+                        "textPath frame expected");
+      // Invalidate and reflow the entire nsSVGTextFrame:
+      static_cast<nsSVGTextPathFrame*>(aFrame)->NotifyGlyphMetricsChange();
+    }
     if (aChange & nsChangeHint_UpdateOpacityLayer) {
       // FIXME/bug 796697: we can get away with empty transactions for
       // opacity updates in many cases.
       needInvalidatingPaint = true;
       aFrame->MarkLayersActive(nsChangeHint_UpdateOpacityLayer);
     }
     if (aChange & nsChangeHint_UpdateTransformLayer) {
       aFrame->MarkLayersActive(nsChangeHint_UpdateTransformLayer);
--- a/layout/base/nsChangeHint.h
+++ b/layout/base/nsChangeHint.h
@@ -105,17 +105,23 @@ enum nsChangeHint {
   nsChangeHint_AddOrRemoveTransform = 0x4000,
 
   /**
    * This change hint has *no* change handling behavior.  However, it
    * exists to be a non-inherited hint, because when the border-style
    * changes, and it's inherited by a child, that might require a reflow
    * due to the border-width change on the child.
    */
-  nsChangeHint_BorderStyleNoneChange = 0x8000
+  nsChangeHint_BorderStyleNoneChange = 0x8000,
+
+  /**
+   * SVG textPath needs to be recomputed because the path has changed.
+   * This means that the glyph positions of the text need to be recomputed.
+   */
+  nsChangeHint_UpdateTextPath = 0x10000
 
   // IMPORTANT NOTE: When adding new hints, consider whether you need to
   // add them to NS_HintsNotHandledForDescendantsIn() below.
 };
 
 // Redefine these operators to return nothing. This will catch any use
 // of these operators on hints. We should not be using these operators
 // on nsChangeHints
new file mode 100644
--- /dev/null
+++ b/layout/svg/crashtests/779971-1.svg
@@ -0,0 +1,14 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" id="r" class="reftest-wait">
+<text id="t"><textPath xlink:href="#r">x</textPath>1</text>
+<script>
+
+window.addEventListener("load", function() {
+    setTimeout(function() {
+        document.getElementById("t").lastChild.data = "2";
+
+        document.documentElement.removeAttribute("class");
+    }, 200);
+}, false);
+
+</script>
+</svg>
--- a/layout/svg/crashtests/crashtests.list
+++ b/layout/svg/crashtests/crashtests.list
@@ -131,12 +131,13 @@ load 732836-1.svg
 load 740627-1.svg
 load 740627-2.svg
 load 757704-1.svg
 load 757718-1.svg
 load 767056-1.svg
 load 768351.svg
 load 780963-1.html
 load 757751-1.svg
+load 779971-1.svg
 load 782141-1.svg
 load 784061-1.svg
 load 790072.svg
 load 791826-1.svg
--- a/layout/svg/nsSVGEffects.cpp
+++ b/layout/svg/nsSVGEffects.cpp
@@ -281,59 +281,31 @@ nsSVGMarkerProperty::DoUpdate()
     // XXXjwatt: We need to unify SVG into standard reflow so we can just use
     // nsChangeHint_NeedReflow | nsChangeHint_NeedDirtyReflow here.
     nsSVGUtils::InvalidateAndScheduleReflowSVG(mFrame);
   }
   mFramePresShell->FrameConstructor()->PostRestyleEvent(
     mFrame->GetContent()->AsElement(), nsRestyleHint(0), changeHint);
 }
 
-class nsAsyncNotifyGlyphMetricsChange MOZ_FINAL : public nsIReflowCallback
-{
-public:
-    nsAsyncNotifyGlyphMetricsChange(nsIFrame* aFrame) : mWeakFrame(aFrame)
-    {
-    }
-
-    virtual bool ReflowFinished()
-    {
-        nsSVGTextPathFrame* frame =
-            static_cast<nsSVGTextPathFrame*>(mWeakFrame.GetFrame());
-        if (frame) {
-            frame->NotifyGlyphMetricsChange();
-        }
-        delete this;
-        return true;
-    }
-
-    virtual void ReflowCallbackCanceled()
-    {
-        delete this;
-    }
-
-    nsWeakFrame mWeakFrame;
-};
-
 void
 nsSVGTextPathProperty::DoUpdate()
 {
   nsSVGIDRenderingObserver::DoUpdate();
   if (!mFrame)
     return;
 
   NS_ASSERTION(mFrame->IsFrameOfType(nsIFrame::eSVG), "SVG frame expected");
 
   if (mFrame->GetType() == nsGkAtoms::svgTextPathFrame) {
-    if (mFrame->PresContext()->PresShell()->IsReflowLocked()) {
-      nsIReflowCallback* cb = new nsAsyncNotifyGlyphMetricsChange(mFrame);
-      mFrame->PresContext()->PresShell()->PostReflowCallback(cb);
-    } else {
-      nsSVGTextPathFrame* textPathFrame = static_cast<nsSVGTextPathFrame*>(mFrame);
-      textPathFrame->NotifyGlyphMetricsChange();
-    }
+    // Repaint asynchronously in case the path frame is being torn down
+    nsChangeHint changeHint =
+      nsChangeHint(nsChangeHint_RepaintFrame | nsChangeHint_UpdateTextPath);
+    mFramePresShell->FrameConstructor()->PostRestyleEvent(
+      mFrame->GetContent()->AsElement(), nsRestyleHint(0), changeHint);
   }
 }
 
 void
 nsSVGPaintingProperty::DoUpdate()
 {
   nsSVGIDRenderingObserver::DoUpdate();
   if (!mFrame)