Bug 1402109. Prevent crashes under SVGTextElement methods when the SVG is under non-displayed HTML. r=heycam
authorJonathan Watt <jwatt@jwatt.org>
Tue, 26 Sep 2017 14:54:27 +0100
changeset 385480 4d7ad98e5be6d28138573e77828218ffd3081c8f
parent 385479 73c1f78f0040ede8058e0dac7102c5dec7687b72
child 385481 0d2943666412331bb190e690592464c70417abf3
push id96025
push userjwatt@jwatt.org
push dateWed, 11 Oct 2017 09:11:22 +0000
treeherdermozilla-inbound@4d7ad98e5be6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1402109
milestone58.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 1402109. Prevent crashes under SVGTextElement methods when the SVG is under non-displayed HTML. r=heycam Summary: The implementations of most SVGTextElement methods flush layout when they're called. Any SVG <text> that is displayed normally will be reflowed by SVGTextFrame::ReflowSVG, and any <text> that is in displayed SVG but is not displayed directly (such as <text> under a <defs>) will be reflowed by SVGTextFrame::ReflowSVGNonDisplayText. Prior to this fix the changed code always assumed that one of these methods would reflow the text. However, in the case that the SVG is under an HTML element that isn't itself ever reflowed (such as if it's under an HTML <caption>) then the SVG text will not be reflowed either and the invariants of the code are broken. This patch modifies the functions that implement the SVGTextElement methods to return an error code if the text has not been reflowed. Note: I did try to reuse SVGTextFrame::ReflowSVGNonDisplayText in this case, but I ran into problems. Given that this is an edge case and likely one than normal authors will never run into I gave up trying to make that work for now. Reviewers: heycam Bug #: 1402109 Differential Revision: https://phabricator.services.mozilla.com/D107
layout/svg/SVGTextFrame.cpp
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -3991,16 +3991,23 @@ SVGTextFrame::ConvertTextElementCharInde
 
 /**
  * Implements the SVG DOM GetNumberOfChars method for the specified
  * text content element.
  */
 uint32_t
 SVGTextFrame::GetNumberOfChars(nsIContent* aContent)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return 0;
+  }
+
   UpdateGlyphPositioning();
 
   uint32_t n = 0;
   CharIterator it(this, CharIterator::eAddressable, aContent);
   if (it.AdvanceToSubtree()) {
     while (!it.AtEnd() && it.IsWithinSubtree()) {
       n++;
       it.Next();
@@ -4035,16 +4042,23 @@ SVGTextFrame::GetComputedTextLength(nsIC
 /**
  * Implements the SVG DOM SelectSubString method for the specified
  * text content element.
  */
 nsresult
 SVGTextFrame::SelectSubString(nsIContent* aContent,
                               uint32_t charnum, uint32_t nchars)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return NS_ERROR_FAILURE;
+  }
+
   UpdateGlyphPositioning();
 
   // Convert charnum/nchars from addressable characters relative to
   // aContent to global character indices.
   CharIterator chit(this, CharIterator::eAddressable, aContent);
   if (!chit.AdvanceToSubtree() ||
       !chit.Next(charnum) ||
       chit.IsAfterSubtree()) {
@@ -4266,16 +4280,23 @@ SVGTextFrame::GetSubStringLengthSlowFall
 /**
  * Implements the SVG DOM GetCharNumAtPosition method for the specified
  * text content element.
  */
 int32_t
 SVGTextFrame::GetCharNumAtPosition(nsIContent* aContent,
                                    mozilla::nsISVGPoint* aPoint)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return -1;
+  }
+
   UpdateGlyphPositioning();
 
   nsPresContext* context = PresContext();
 
   gfxPoint p(aPoint->X(), aPoint->Y());
 
   int32_t result = -1;
 
@@ -4299,16 +4320,23 @@ SVGTextFrame::GetCharNumAtPosition(nsICo
  * Implements the SVG DOM GetStartPositionOfChar method for the specified
  * text content element.
  */
 nsresult
 SVGTextFrame::GetStartPositionOfChar(nsIContent* aContent,
                                      uint32_t aCharNum,
                                      mozilla::nsISVGPoint** aResult)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return NS_ERROR_FAILURE;
+  }
+
   UpdateGlyphPositioning();
 
   CharIterator it(this, CharIterator::eAddressable, aContent);
   if (!it.AdvanceToSubtree() ||
       !it.Next(aCharNum)) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
 
@@ -4324,16 +4352,23 @@ SVGTextFrame::GetStartPositionOfChar(nsI
  * Implements the SVG DOM GetEndPositionOfChar method for the specified
  * text content element.
  */
 nsresult
 SVGTextFrame::GetEndPositionOfChar(nsIContent* aContent,
                                    uint32_t aCharNum,
                                    mozilla::nsISVGPoint** aResult)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return NS_ERROR_FAILURE;
+  }
+
   UpdateGlyphPositioning();
 
   CharIterator it(this, CharIterator::eAddressable, aContent);
   if (!it.AdvanceToSubtree() ||
       !it.Next(aCharNum)) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
 
@@ -4361,16 +4396,23 @@ SVGTextFrame::GetEndPositionOfChar(nsICo
  * Implements the SVG DOM GetExtentOfChar method for the specified
  * text content element.
  */
 nsresult
 SVGTextFrame::GetExtentOfChar(nsIContent* aContent,
                               uint32_t aCharNum,
                               dom::SVGIRect** aResult)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return NS_ERROR_FAILURE;
+  }
+
   UpdateGlyphPositioning();
 
   CharIterator it(this, CharIterator::eAddressable, aContent);
   if (!it.AdvanceToSubtree() ||
       !it.Next(aCharNum)) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }
 
@@ -4421,16 +4463,23 @@ SVGTextFrame::GetExtentOfChar(nsIContent
  * Implements the SVG DOM GetRotationOfChar method for the specified
  * text content element.
  */
 nsresult
 SVGTextFrame::GetRotationOfChar(nsIContent* aContent,
                                 uint32_t aCharNum,
                                 float* aResult)
 {
+  nsIFrame* kid = PrincipalChildList().FirstChild();
+  if (NS_SUBTREE_DIRTY(kid)) {
+    // We're never reflowed if we're under a non-SVG element that is
+    // never reflowed (such as the HTML 'caption' element).
+    return NS_ERROR_FAILURE;
+  }
+
   UpdateGlyphPositioning();
 
   CharIterator it(this, CharIterator::eAddressable, aContent);
   if (!it.AdvanceToSubtree() ||
       !it.Next(aCharNum)) {
     return NS_ERROR_DOM_INDEX_SIZE_ERR;
   }