Bug 1072758 - ScheduleReflowSVGNonDisplayText when changing style to display none r=emilio
authorviolet <violet.bugreport@gmail.com>
Sat, 16 Mar 2019 02:36:09 +0000
changeset 464683 2717903072b6
parent 464682 3c2169f7665e
child 464684 6e1f82657583
push id112465
push useraciure@mozilla.com
push dateSun, 17 Mar 2019 09:50:10 +0000
treeherdermozilla-inbound@e0861be8d6c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1072758
milestone67.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 1072758 - ScheduleReflowSVGNonDisplayText when changing style to display none r=emilio DidSetComputedStyle won't be called if the style changes to "display:none". To ensure the reflow is properly scheduled, we need to also hook DestroyFrom. Differential Revision: https://phabricator.services.mozilla.com/D23353
layout/generic/nsFrame.cpp
layout/svg/crashtests/1072758.html
layout/svg/crashtests/crashtests.list
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -561,16 +561,47 @@ static bool IsFontSizeInflationContainer
                    aFrame->IsBrFrame() ||
                    aFrame->IsFrameOfType(nsIFrame::eMathML),
                "line participants must not be containers");
   NS_ASSERTION(!aFrame->IsBulletFrame() || isInline,
                "bullets should not be containers");
   return !isInline;
 }
 
+static void MaybeScheduleReflowSVGNonDisplayText(nsFrame* aFrame) {
+  if (!nsSVGUtils::IsInSVGTextSubtree(aFrame)) {
+    return;
+  }
+
+  // We need to ensure that any non-display SVGTextFrames get reflowed when a
+  // child text frame gets new style. Thus we need to schedule a reflow in
+  // |DidSetComputedStyle|. We also need to call it from |DestroyFrom|,
+  // because otherwise we won't get notified when style changes to
+  // "display:none".
+  SVGTextFrame* svgTextFrame = static_cast<SVGTextFrame*>(
+      nsLayoutUtils::GetClosestFrameOfType(aFrame, LayoutFrameType::SVGText));
+  nsIFrame* anonBlock = svgTextFrame->PrincipalChildList().FirstChild();
+
+  // Note that we must check NS_FRAME_FIRST_REFLOW on our SVGTextFrame's
+  // anonymous block frame rather than our aFrame, since NS_FRAME_FIRST_REFLOW
+  // may be set on us if we're a new frame that has been inserted after the
+  // document's first reflow. (In which case this DidSetComputedStyle call may
+  // be happening under frame construction under a Reflow() call.)
+  if (!anonBlock || anonBlock->HasAnyStateBits(NS_FRAME_FIRST_REFLOW)) {
+    return;
+  }
+
+  if (!svgTextFrame->HasAnyStateBits(NS_FRAME_IS_NONDISPLAY) ||
+      svgTextFrame->HasAnyStateBits(NS_STATE_SVG_TEXT_IN_REFLOW)) {
+    return;
+  }
+
+  svgTextFrame->ScheduleReflowSVGNonDisplayText(nsIPresShell::eStyleChange);
+}
+
 void nsFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
                    nsIFrame* aPrevInFlow) {
   MOZ_ASSERT(nsQueryFrame::FrameIID(mClass) == GetFrameId());
   MOZ_ASSERT(!mContent, "Double-initing a frame?");
   NS_ASSERTION(IsFrameOfType(eDEBUGAllFrames) && !IsFrameOfType(eDEBUGNoFrames),
                "IsFrameOfType implementation that doesn't call base class");
 
   mContent = aContent;
@@ -694,16 +725,18 @@ void nsFrame::DestroyFrom(nsIFrame* aDes
                "destroy called on frame while scripts not blocked");
   NS_ASSERTION(!GetNextSibling() && !GetPrevSibling(),
                "Frames should be removed before destruction.");
   NS_ASSERTION(aDestructRoot, "Must specify destruct root");
   MOZ_ASSERT(!HasAbsolutelyPositionedChildren());
   MOZ_ASSERT(!HasAnyStateBits(NS_FRAME_PART_OF_IBSPLIT),
              "NS_FRAME_PART_OF_IBSPLIT set on non-nsContainerFrame?");
 
+  MaybeScheduleReflowSVGNonDisplayText(this);
+
   SVGObserverUtils::InvalidateDirectRenderingObservers(this);
 
   if (StyleDisplay()->mPosition == NS_STYLE_POSITION_STICKY) {
     StickyScrollContainer* ssc =
         StickyScrollContainer::GetStickyScrollContainerForFrame(this);
     if (ssc) {
       ssc->RemoveFrame(this);
     }
@@ -1055,35 +1088,17 @@ void nsIFrame::MarkNeedsDisplayItemRebui
       }
     }
   }
 }
 
 // Subclass hook for style post processing
 /* virtual */
 void nsFrame::DidSetComputedStyle(ComputedStyle* aOldComputedStyle) {
-  if (nsSVGUtils::IsInSVGTextSubtree(this)) {
-    SVGTextFrame* svgTextFrame = static_cast<SVGTextFrame*>(
-        nsLayoutUtils::GetClosestFrameOfType(this, LayoutFrameType::SVGText));
-    nsIFrame* anonBlock = svgTextFrame->PrincipalChildList().FirstChild();
-    // Just as in SVGTextFrame::DidSetComputedStyle, we need to ensure that
-    // any non-display SVGTextFrames get reflowed when a child text frame
-    // gets new style.
-    //
-    // Note that we must check NS_FRAME_FIRST_REFLOW on our SVGTextFrame's
-    // anonymous block frame rather than our self, since NS_FRAME_FIRST_REFLOW
-    // may be set on us if we're a new frame that has been inserted after the
-    // document's first reflow. (In which case this DidSetComputedStyle call may
-    // be happening under frame construction under a Reflow() call.)
-    if (anonBlock && !(anonBlock->GetStateBits() & NS_FRAME_FIRST_REFLOW) &&
-        (svgTextFrame->GetStateBits() & NS_FRAME_IS_NONDISPLAY) &&
-        !(svgTextFrame->GetStateBits() & NS_STATE_SVG_TEXT_IN_REFLOW)) {
-      svgTextFrame->ScheduleReflowSVGNonDisplayText(nsIPresShell::eStyleChange);
-    }
-  }
+  MaybeScheduleReflowSVGNonDisplayText(this);
 
   Document* doc = PresContext()->Document();
   ImageLoader* imageLoader = doc->StyleImageLoader();
   // Continuing text frame doesn't initialize its continuation pointer before
   // reaching here for the first time, so we have to exclude text frames. This
   // doesn't affect correctness because text can't match selectors.
   //
   // FIXME(emilio): We should consider fixing that.
new file mode 100644
--- /dev/null
+++ b/layout/svg/crashtests/1072758.html
@@ -0,0 +1,35 @@
+<style>
+#x9 {
+  display:none;
+}
+</style>
+
+<body onload="go()">
+<svg>
+<path id="a"></path>
+
+<mask id="m">
+  <text id="y">
+    <tspan id="x1"></tspan>
+    <textPath id="x2"></textPath>
+    <a id="x3">Hello</a>
+    <tspan><tspan id="x4"></tspan></tspan>
+    <tspan id="x5"></tspan>
+  </text>
+</mask>
+
+<rect width="600" height="400" mask="url(#m)"/>
+</svg>
+</body>
+
+<script>
+
+function go() {
+  x1.style.display = "none";
+  x2.style.display = "none";
+  x3.style.display = "none";
+  x4.style.display = "none";
+  x5.id = "x9";
+};
+
+</script>
--- a/layout/svg/crashtests/crashtests.list
+++ b/layout/svg/crashtests/crashtests.list
@@ -216,8 +216,9 @@ load blob-merging-and-retained-display-l
 load empty-blob-merging.html
 load grouping-empty-bounds.html
 load 1480275.html
 load 1480224.html
 load 1502936.html
 load 1504918.svg
 load perspective-invalidation.html
 load invalid_url.html
+load 1072758.html