From 0bd8486f4088c0845514986f61861688e0be011d Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 6 Apr 2015 09:11:55 -0400 Subject: [PATCH] Bug 1149542 - Part 1: Return early from SVG text layout if we discover mPositions is not long enough. r=dholbert, a=sledru --- layout/svg/SVGTextFrame.cpp | 59 +++++++++++++++++++++++++++++++-------------- layout/svg/SVGTextFrame.h | 23 ++++++++++++------ 2 files changed, 56 insertions(+), 26 deletions(-) diff --git a/layout/svg/SVGTextFrame.cpp b/layout/svg/SVGTextFrame.cpp index 721e699..45327881 100644 --- a/layout/svg/SVGTextFrame.cpp +++ b/layout/svg/SVGTextFrame.cpp @@ -14,6 +14,7 @@ #include "gfxTypes.h" #include "LookAndFeel.h" #include "mozilla/gfx/2D.h" +#include "mozilla/Likely.h" #include "nsAlgorithm.h" #include "nsBlockFrame.h" #include "nsCaret.h" @@ -4316,23 +4317,28 @@ ShouldStartRunAtIndex(const nsTArray& aPositions, return false; } -uint32_t -SVGTextFrame::ResolvePositions(nsIContent* aContent, - uint32_t aIndex, - bool aInTextPath, - bool& aForceStartOfChunk, - nsTArray& aDeltas) +bool +SVGTextFrame::ResolvePositionsForNode(nsIContent* aContent, + uint32_t& aIndex, + bool aInTextPath, + bool& aForceStartOfChunk, + nsTArray& aDeltas) { if (aContent->IsNodeOfType(nsINode::eTEXT)) { // We found a text node. uint32_t length = static_cast(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. @@ -4345,18 +4351,23 @@ SVGTextFrame::ResolvePositions(nsIContent* aContent, 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) { // 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; } @@ -4376,8 +4387,14 @@ SVGTextFrame::ResolvePositions(nsIContent* aContent, rotate = &animatedRotate->GetAnimValue(); } - uint32_t count = GetTextContentLength(aContent); bool percentages = false; + uint32_t count = GetTextContentLength(aContent); + + 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 @@ -4456,8 +4473,11 @@ SVGTextFrame::ResolvePositions(nsIContent* aContent, 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) { @@ -4465,7 +4485,7 @@ SVGTextFrame::ResolvePositions(nsIContent* aContent, aForceStartOfChunk = true; } - return aIndex; + return true; } bool @@ -4501,8 +4521,10 @@ SVGTextFrame::ResolvePositions(nsTArray& aDeltas, // 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 @@ -4958,9 +4980,10 @@ SVGTextFrame::DoGlyphPositioning() // Get the x, y, dx, dy, rotate values for the subtree. nsTArray 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; } diff --git a/layout/svg/SVGTextFrame.h b/layout/svg/SVGTextFrame.h index 48951f7..912af8b 100644 --- a/layout/svg/SVGTextFrame.h +++ b/layout/svg/SVGTextFrame.h @@ -505,15 +505,18 @@ private: * 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 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& aDeltas); + bool ResolvePositionsForNode(nsIContent* aContent, uint32_t& aIndex, + bool aInTextPath, bool& aForceStartOfChunk, + nsTArray& aDeltas); /** * Initializes mPositions with character position information based on @@ -521,9 +524,13 @@ private: * 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& aDeltas, bool aRunPerGlyph); -- 2.2.1