Bug 841163 - Ensure that we recompute SVG text glyph positions only once frames have been reconstructed. r=roc

layout/svg/crashtests/841163-1.svg

@@ -0,0 +1,29 @@
+<svg xmlns="http://www.w3.org/2000/svg" class="reftest-wait">
+
+<filter id="f"/>
+
+<g filter="url(#f)">
+  <text>AB</text>
+</g>
+
+<script>
+
+function forceFrameConstruction()
+{
+  document.documentElement.getBoundingClientRect();
+}
+
+function boom()
+{
+  document.getElementsByTagName("text")[0].firstChild.splitText(1);
+  forceFrameConstruction();
+  document.normalize();
+  forceFrameConstruction();
+  document.documentElement.removeAttribute("class");
+}
+
+window.addEventListener("load", boom, false);
+
+</script>
+
+</svg>

layout/svg/crashtests/crashtests.list

@@ -151,4 +151,4 @@ load 791826-1.svg
 load 789390-1.html
 load 808318-1.svg
 load 813420-1.svg
-
+load 841163-1.svg

layout/svg/nsSVGTextFrame2.cpp

@@ -2444,6 +2444,9 @@ CharIterator::MatchesFilter() const
          IsClusterAndLigatureGroupStart();
 }
 
+// -----------------------------------------------------------------------------
+// nsCharClipDisplayItem
+
 /**
  * An nsCharClipDisplayItem that obtains its left and right clip edges from a
  * TextRenderedRun object.
@@ -2459,6 +2462,9 @@ public:
   NS_DISPLAY_DECL_NAME("SVGText", TYPE_TEXT)
 };
 
+// -----------------------------------------------------------------------------
+// SVGTextDrawPathCallbacks
+
 /**
  * Text frame draw callback class that paints the text and text decoration parts
  * of an nsTextFrame using SVG painting properties, and selection backgrounds
@@ -2730,6 +2736,21 @@ SVGTextDrawPathCallbacks::FillAndStroke()
   }
 }
 
+// -----------------------------------------------------------------------------
+// GlyphMetricsUpdater
+
+NS_IMETHODIMP
+GlyphMetricsUpdater::Run()
+{
+  if (mFrame) {
+    mFrame->mPositioningDirty = true;
+    nsSVGUtils::InvalidateBounds(mFrame, false);
+    nsSVGUtils::ScheduleReflowSVG(mFrame);
+    mFrame->mGlyphMetricsUpdater = nullptr;
+  }
+  return NS_OK;
+}
+
 }
 
 // ============================================================================
@@ -2829,6 +2850,15 @@ nsSVGTextFrame2::Init(nsIContent* aContent,
   return rv;
 }
 
+void
+nsSVGTextFrame2::DestroyFrom(nsIFrame* aDestructRoot)
+{
+  if (mGlyphMetricsUpdater) {
+    mGlyphMetricsUpdater->Revoke();
+  }
+  nsSVGTextFrame2Base::DestroyFrom(aDestructRoot);
+}
+
 NS_IMETHODIMP
 nsSVGTextFrame2::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                   const nsRect& aDirtyRect,
@@ -4548,9 +4578,23 @@ nsSVGTextFrame2::ShouldRenderAsPath(nsRenderingContext* aContext,
 void
 nsSVGTextFrame2::NotifyGlyphMetricsChange()
 {
-  mPositioningDirty = true;
-  nsSVGUtils::InvalidateBounds(this, false);
-  nsSVGUtils::ScheduleReflowSVG(this);
+  // We need to perform the operations in GlyphMetricsUpdater in a
+  // script runner since we can get called just after a DOM mutation,
+  // before frames have been reconstructed, and UpdateGlyphPositioning
+  // will be called with out-of-date frames.  This occurs when the
+  // <text> element is being filtered, as the InvalidateBounds()
+  // call needs to call in to GetBBoxContribution() to determine the
+  // filtered region, and that needs to iterate over the text frames.
+  // We would flush frame construction, but that needs to be done
+  // inside a script runner.
+  //
+  // Much of the time, this will perform the GlyphMetricsUpdater
+  // operations immediately.
+  if (mGlyphMetricsUpdater) {
+    return;
+  }
+  mGlyphMetricsUpdater = new GlyphMetricsUpdater(this);
+  nsContentUtils::AddScriptRunner(mGlyphMetricsUpdater.get());
 }
 
 void

layout/svg/nsSVGTextFrame2.h

@@ -16,6 +16,7 @@
 
 class nsDisplaySVGText;
 class nsRenderingContext;
+class nsSVGTextFrame2;
 class nsTextFrame;
 
 typedef nsSVGDisplayContainerFrame nsSVGTextFrame2Base;
@@ -114,6 +115,19 @@ private:
   }
 };
 
+/**
+ * A runnable to mark glyph positions as needing to be recomputed
+ * and to invalid the bounds of the nsSVGTextFrame2 frame.
+ */
+class GlyphMetricsUpdater : public nsRunnable {
+public:
+  NS_DECL_NSIRUNNABLE
+  GlyphMetricsUpdater(nsSVGTextFrame2* aFrame) : mFrame(aFrame) { }
+  void Revoke() { mFrame = nullptr; }
+private:
+  nsSVGTextFrame2* mFrame;
+};
+
 }
 
 /**
@@ -155,6 +169,7 @@ class nsSVGTextFrame2 : public nsSVGTextFrame2Base
   friend nsIFrame*
   NS_NewSVGTextFrame2(nsIPresShell* aPresShell, nsStyleContext* aContext);
 
+  friend class mozilla::GlyphMetricsUpdater;
   friend class mozilla::TextFrameIterator;
   friend class mozilla::TextNodeCorrespondenceRecorder;
   friend struct mozilla::TextRenderedRun;
@@ -182,6 +197,8 @@ public:
                   nsIFrame*   aParent,
                   nsIFrame*   aPrevInFlow);
 
+  virtual void DestroyFrom(nsIFrame* aDestructRoot) MOZ_OVERRIDE;
+
   NS_IMETHOD AttributeChanged(int32_t aNamespaceID,
                               nsIAtom* aAttribute,
                               int32_t aModType);
@@ -523,6 +540,12 @@ private:
    */
   MutationObserver mMutationObserver;
 
+  /**
+   * The runnable we have dispatched to perform the work of
+   * NotifyGlyphMetricsChange.
+   */
+  nsRefPtr<GlyphMetricsUpdater> mGlyphMetricsUpdater;
+
   /**
    * Cached canvasTM value.
    */