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 a=lsblakk
authorRobert Longson <longsonr@gmail.com>
Wed, 07 Nov 2012 09:53:44 +0000
changeset 114084 b6c41959679d30cc19da1fd2238cb4529cd35399
parent 114083 43250c0b2bd298851544f94dd8e7f06a16ed8669
child 114085 7be95f3c39b22d27ee72860854dfd953e963978a
push id2639
push userdholbert@mozilla.com
push dateFri, 09 Nov 2012 19:57:15 +0000
treeherdermozilla-aurora@b6c41959679d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt, lsblakk
bugs779971
milestone18.0a2
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 a=lsblakk
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
@@ -108,16 +108,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;
@@ -7783,16 +7784,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
@@ -86,17 +86,23 @@ enum nsChangeHint {
   nsChangeHint_ChildrenOnlyTransform = 0x1000,
 
   /**
    * 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 = 0x2000
+  nsChangeHint_BorderStyleNoneChange = 0x2000,
+
+  /**
+   * 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 = 0x4000
 
   // 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
@@ -128,11 +128,12 @@ load 725918-1.svg
 load 732836-1.svg
 load 740627-1.svg
 load 740627-2.svg
 load 757704-1.svg
 load 757718-1.svg
 load 757751-1.svg
 load 767056-1.svg
 load 768351.svg
+load 779971-1.svg
 load 780963-1.html
 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)