Bug 1149542 - Part 1: Return early from SVG text layout if we discover mPositions is not long enough. r=dholbert, a=abillings
authorCameron McCormack <cam@mcc.id.au>
Mon, 06 Apr 2015 09:11:55 -0400
changeset 258301 984f9cdef799
parent 258300 8c5c12705b50
child 258302 32d78bac2cfa
push id4638
push userryanvm@gmail.com
push date2015-04-06 20:41 +0000
treeherdermozilla-beta@499e1c563939 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, abillings
bugs1149542
milestone38.0
Bug 1149542 - Part 1: Return early from SVG text layout if we discover mPositions is not long enough. r=dholbert, a=abillings
layout/svg/SVGTextFrame.cpp
layout/svg/SVGTextFrame.h
--- a/layout/svg/SVGTextFrame.cpp
+++ b/layout/svg/SVGTextFrame.cpp
@@ -11,16 +11,17 @@
 #include "gfx2DGlue.h"
 #include "gfxFont.h"
 #include "gfxSkipChars.h"
 #include "gfxTypes.h"
 #include "gfxUtils.h"
 #include "LookAndFeel.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/PatternHelpers.h"
+#include "mozilla/Likely.h"
 #include "nsAlgorithm.h"
 #include "nsBlockFrame.h"
 #include "nsCaret.h"
 #include "nsContentUtils.h"
 #include "nsGkAtoms.h"
 #include "nsIDOMSVGLength.h"
 #include "nsISelection.h"
 #include "nsQuickSort.h"
@@ -4395,57 +4396,67 @@ ShouldStartRunAtIndex(const nsTArray<Cha
         aDeltas[aIndex].y != 0.0) {
       return true;
     }
   }
 
   return false;
 }
 
-uint32_t
-SVGTextFrame::ResolvePositions(nsIContent* aContent,
-                               uint32_t aIndex,
-                               bool aInTextPath,
-                               bool& aForceStartOfChunk,
-                               nsTArray<gfxPoint>& aDeltas)
+bool
+SVGTextFrame::ResolvePositionsForNode(nsIContent* aContent,
+                                      uint32_t& aIndex,
+                                      bool aInTextPath,
+                                      bool& aForceStartOfChunk,
+                                      nsTArray<gfxPoint>& aDeltas)
 {
   if (aContent->IsNodeOfType(nsINode::eTEXT)) {
     // We found a text node.
     uint32_t length = static_cast<nsTextNode*>(aContent)->TextLength();
     if (length) {
+      uint32_t end = aIndex + length;
+      if (MOZ_UNLIKELY(end > mPositions.Length())) {
+        MOZ_ASSERT_UNREACHABLE("length of mPositions does not match characters "
+                               "found by iterating content");
+        return false;
+      }
       if (aForceStartOfChunk) {
         // Note this character as starting a new anchored chunk.
         mPositions[aIndex].mStartOfChunk = true;
         aForceStartOfChunk = false;
       }
-      uint32_t end = aIndex + length;
       while (aIndex < end) {
         // Record whether each of these characters should start a new rendered
         // run.  That is always the case for characters on a text path.
         //
         // Run boundaries due to rotate="" values are handled in
         // DoGlyphPositioning.
         if (aInTextPath || ShouldStartRunAtIndex(mPositions, aDeltas, aIndex)) {
           mPositions[aIndex].mRunBoundary = true;
         }
         aIndex++;
       }
     }
-    return aIndex;
+    return true;
   }
 
   // Skip past elements that aren't text content elements.
   if (!IsTextContentElement(aContent)) {
-    return aIndex;
+    return true;
   }
 
   if (aContent->Tag() == nsGkAtoms::textPath) {
     // <textPath> elements are as if they are specified with x="0" y="0", but
     // only if they actually have some text content.
     if (HasTextContent(aContent)) {
+      if (MOZ_UNLIKELY(aIndex >= mPositions.Length())) {
+        MOZ_ASSERT_UNREACHABLE("length of mPositions does not match characters "
+                               "found by iterating content");
+        return false;
+      }
       mPositions[aIndex].mPosition = gfxPoint();
       mPositions[aIndex].mStartOfChunk = true;
     }
   } else if (aContent->Tag() != nsGkAtoms::a) {
     // We have a text content element that can have x/y/dx/dy/rotate attributes.
     nsSVGElement* element = static_cast<nsSVGElement*>(aContent);
 
     // Get x, y, dx, dy.
@@ -4455,18 +4466,24 @@ SVGTextFrame::ResolvePositions(nsIConten
     // Get rotate.
     const SVGNumberList* rotate = nullptr;
     SVGAnimatedNumberList* animatedRotate =
       element->GetAnimatedNumberList(nsGkAtoms::rotate);
     if (animatedRotate) {
       rotate = &animatedRotate->GetAnimValue();
     }
 
+    bool percentages = false;
     uint32_t count = GetTextContentLength(aContent);
-    bool percentages = false;
+
+    if (MOZ_UNLIKELY(aIndex + count > mPositions.Length())) {
+      MOZ_ASSERT_UNREACHABLE("length of mPositions does not match characters "
+                             "found by iterating content");
+      return false;
+    }
 
     // New text anchoring chunks start at each character assigned a position
     // with x="" or y="", or if we forced one with aForceStartOfChunk due to
     // being just after a <textPath>.
     uint32_t newChunkCount = std::max(x.Length(), y.Length());
     if (!newChunkCount && aForceStartOfChunk) {
       newChunkCount = 1;
     }
@@ -4535,26 +4552,29 @@ SVGTextFrame::ResolvePositions(nsIConten
     }
   }
 
   // Recurse to children.
   bool inTextPath = aInTextPath || aContent->Tag() == nsGkAtoms::textPath;
   for (nsIContent* child = aContent->GetFirstChild();
        child;
        child = child->GetNextSibling()) {
-    aIndex = ResolvePositions(child, aIndex, inTextPath, aForceStartOfChunk,
-                              aDeltas);
+    bool ok = ResolvePositionsForNode(child, aIndex, inTextPath,
+                                      aForceStartOfChunk, aDeltas);
+    if (!ok) {
+      return false;
+    }
   }
 
   if (aContent->Tag() == nsGkAtoms::textPath) {
     // Force a new anchored chunk just after a <textPath>.
     aForceStartOfChunk = true;
   }
 
-  return aIndex;
+  return true;
 }
 
 bool
 SVGTextFrame::ResolvePositions(nsTArray<gfxPoint>& aDeltas,
                                bool aRunPerGlyph)
 {
   NS_ASSERTION(mPositions.IsEmpty(), "expected mPositions to be empty");
   RemoveStateBits(NS_STATE_SVG_POSITIONING_MAY_USE_PERCENTAGES);
@@ -4580,18 +4600,20 @@ SVGTextFrame::ResolvePositions(nsTArray<
                                              it.IsOriginalCharUnaddressable()));
   }
   while (++index < it.TextElementCharIndex()) {
     mPositions.AppendElement(CharPosition::Unspecified(false));
   }
 
   // Recurse over the content and fill in character positions as we go.
   bool forceStartOfChunk = false;
-  return ResolvePositions(mContent, 0, aRunPerGlyph,
-                          forceStartOfChunk, aDeltas) != 0;
+  index = 0;
+  bool ok = ResolvePositionsForNode(mContent, index, aRunPerGlyph,
+                                    forceStartOfChunk, aDeltas);
+  return ok && index > 0;
 }
 
 void
 SVGTextFrame::DetermineCharPositions(nsTArray<nsPoint>& aPositions)
 {
   NS_ASSERTION(aPositions.IsEmpty(), "expected aPositions to be empty");
 
   nsPoint position, lastPosition;
@@ -5034,19 +5056,20 @@ SVGTextFrame::DoGlyphPositioning()
   if (adjustingTextLength && expectedTextLength < 0.0f) {
     // If textLength="" is less than zero, ignore it.
     adjustingTextLength = false;
   }
 
   // Get the x, y, dx, dy, rotate values for the subtree.
   nsTArray<gfxPoint> deltas;
   if (!ResolvePositions(deltas, adjustingTextLength)) {
-    // If ResolvePositions returned false, it means that there were some
-    // characters in the DOM but none of them are displayed.  Clear out
-    // mPositions so that we don't attempt to do any painting later.
+    // If ResolvePositions returned false, it means either there were some
+    // characters in the DOM but none of them are displayed, or there was
+    // an error in processing mPositions.  Clear out mPositions so that we don't
+    // attempt to do any painting later.
     mPositions.Clear();
     return;
   }
 
   // XXX We might be able to do less work when there is at most a single
   // x/y/dx/dy position.
 
   // Truncate the positioning arrays to the actual number of characters present.
--- a/layout/svg/SVGTextFrame.h
+++ b/layout/svg/SVGTextFrame.h
@@ -511,35 +511,42 @@ private:
   int32_t
   ConvertTextElementCharIndexToAddressableIndex(int32_t aIndex,
                                                 nsIContent* aContent);
 
   /**
    * Recursive helper for ResolvePositions below.
    *
    * @param aContent The current node.
-   * @param aIndex The current character index.
+   * @param aIndex (in/out) The current character index.
    * @param aInTextPath Whether we are currently under a <textPath> element.
-   * @param aForceStartOfChunk Whether the next character we find should start a
-   *   new anchored chunk.
-   * @return The character index we got up to.
+   * @param aForceStartOfChunk (in/out) Whether the next character we find
+   *   should start a new anchored chunk.
+   * @param aDeltas (in/out) Receives the resolved dx/dy values for each
+   *   character.
+   * @return false if we discover that mPositions did not have enough
+   *   elements; true otherwise.
    */
-  uint32_t ResolvePositions(nsIContent* aContent, uint32_t aIndex,
-                            bool aInTextPath, bool& aForceStartOfChunk,
-                            nsTArray<gfxPoint>& aDeltas);
+  bool ResolvePositionsForNode(nsIContent* aContent, uint32_t& aIndex,
+                               bool aInTextPath, bool& aForceStartOfChunk,
+                               nsTArray<gfxPoint>& aDeltas);
 
   /**
    * Initializes mPositions with character position information based on
    * x/y/rotate attributes, leaving unspecified values in the array if a position
    * was not given for that character.  Also fills aDeltas with values based on
    * dx/dy attributes.
    *
+   * @param aDeltas (in/out) Receives the resolved dx/dy values for each
+   *   character.
    * @param aRunPerGlyph Whether mPositions should record that a new run begins
    *   at each glyph.
-   * @return True if we recorded any positions.
+   * @return false if we did not record any positions (due to having no
+   *   displayed characters) or if we discover that mPositions did not have
+   *   enough elements; true otherwise.
    */
   bool ResolvePositions(nsTArray<gfxPoint>& aDeltas, bool aRunPerGlyph);
 
   /**
    * Determines the position, in app units, of each character in the <text> as
    * laid out by reflow, and appends them to aPositions.  Any characters that
    * are undisplayed or trimmed away just get the last position.
    */