From 1b45e462ab28cbce51dc5048fcdedc05b95cb15f Mon Sep 17 00:00:00 2001 From: "roc+%cs.cmu.edu" Date: Tue, 15 May 2007 03:56:48 +0000 Subject: [PATCH] Bug 380692. Change gfxTextRun API so the textrun copies text, if necessary, instead of the caller having to do it. r=vlad git-svn-id: svn://10.0.0.236/trunk@226413 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/gfx/thebes/public/gfxFont.h | 19 +++---- mozilla/gfx/thebes/src/gfxFont.cpp | 20 ++++++++ mozilla/gfx/thebes/src/gfxTextRunCache.cpp | 47 +----------------- mozilla/layout/generic/Makefile.in | 2 +- mozilla/layout/generic/nsTextFrameThebes.cpp | 2 + .../generic/nsTextRunTransformations.cpp | 49 ++++++++++--------- .../layout/generic/nsTextRunTransformations.h | 5 +- .../layout/svg/base/src/nsSVGGlyphFrame.cpp | 7 +-- 8 files changed, 64 insertions(+), 87 deletions(-) diff --git a/mozilla/gfx/thebes/public/gfxFont.h b/mozilla/gfx/thebes/public/gfxFont.h index 678f97a03fb..c8709b09b7b 100644 --- a/mozilla/gfx/thebes/public/gfxFont.h +++ b/mozilla/gfx/thebes/public/gfxFont.h @@ -796,8 +796,8 @@ public: // The caller is responsible for initializing our glyphs after construction. // Initially all glyphs are such that GetCharacterGlyphs()[i].IsMissing() is true. - // We take ownership of aText, which must have been allocated by new[] (it - // may be null if aLength is zero). + // If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the + // textrun will copy it. gfxTextRun(const gfxTextRunFactory::Parameters *aParams, const void *aText, PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags); @@ -805,7 +805,8 @@ public: // glyph data is copied, so the text and length must be the same as this // textrun's. If there's a problem, return null. Actual linebreaks will // be set as per aParams; there will be no potential linebreaks. - // If successful, we take ownership of aText, which must have been allocated by new[]. + // If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the + // textrun will copy it. virtual gfxTextRun *Clone(const gfxTextRunFactory::Parameters *aParams, const void *aText, PRUint32 aLength, gfxFontGroup *aFontGroup, PRUint32 aFlags); @@ -1143,16 +1144,16 @@ public: gfxTextRun *MakeSpaceTextRun(const Parameters *aParams, PRUint32 aFlags); /** - * Make a textrun for a given string. Takes ownership of aString unless - * aFlags & TEXT_IS_PERSISTENT --- in that case, the caller must destroy - * the textrun before aString dies. + * Make a textrun for a given string. + * If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the + * textrun will copy it. */ virtual gfxTextRun *MakeTextRun(const PRUnichar *aString, PRUint32 aLength, const Parameters *aParams, PRUint32 aFlags) = 0; /** - * Make a textrun for a given string. Takes ownership of aString unless - * aFlags & TEXT_IS_PERSISTENT --- in that case, the caller must destroy - * the textrun before aString dies. + * Make a textrun for a given string. + * If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the + * textrun will copy it. */ virtual gfxTextRun *MakeTextRun(const PRUint8 *aString, PRUint32 aLength, const Parameters *aParams, PRUint32 aFlags) = 0; diff --git a/mozilla/gfx/thebes/src/gfxFont.cpp b/mozilla/gfx/thebes/src/gfxFont.cpp index 4736f54a251..ef77c640ab8 100644 --- a/mozilla/gfx/thebes/src/gfxFont.cpp +++ b/mozilla/gfx/thebes/src/gfxFont.cpp @@ -633,8 +633,28 @@ gfxTextRun::gfxTextRun(const gfxTextRunFactory::Parameters *aParams, const void } if (mFlags & gfxTextRunFactory::TEXT_IS_8BIT) { mText.mSingle = NS_STATIC_CAST(const PRUint8 *, aText); + if (!(mFlags & gfxTextRunFactory::TEXT_IS_PERSISTENT)) { + PRUint8 *newText = new PRUint8[aLength]; + if (!newText) { + // indicate textrun failure + mCharacterGlyphs = nsnull; + } else { + memcpy(newText, aText, aLength); + } + mText.mSingle = newText; + } } else { mText.mDouble = NS_STATIC_CAST(const PRUnichar *, aText); + if (!(mFlags & gfxTextRunFactory::TEXT_IS_PERSISTENT)) { + PRUnichar *newText = new PRUnichar[aLength]; + if (!newText) { + // indicate textrun failure + mCharacterGlyphs = nsnull; + } else { + memcpy(newText, aText, aLength*sizeof(PRUnichar)); + } + mText.mDouble = newText; + } } } diff --git a/mozilla/gfx/thebes/src/gfxTextRunCache.cpp b/mozilla/gfx/thebes/src/gfxTextRunCache.cpp index a144fe9623b..6a2b0536447 100644 --- a/mozilla/gfx/thebes/src/gfxTextRunCache.cpp +++ b/mozilla/gfx/thebes/src/gfxTextRunCache.cpp @@ -96,34 +96,6 @@ static void *GetCacheKeyFontOrGroup(gfxTextRun *aTextRun) : NS_STATIC_CAST(void *, fontGroup); } -static const PRUnichar * -CloneText(const PRUnichar *aText, PRUint32 aLength, - nsAutoArrayPtr *aBuffer, PRUint32 aFlags) -{ - if (*aBuffer == aText || (aFlags & gfxFontGroup::TEXT_IS_PERSISTENT)) - return aText; - PRUnichar *newText = new PRUnichar[aLength]; - if (!newText) - return nsnull; - memcpy(newText, aText, aLength*sizeof(PRUnichar)); - *aBuffer = newText; - return newText; -} - -static const PRUint8 * -CloneText(const PRUint8 *aText, PRUint32 aLength, - nsAutoArrayPtr *aBuffer, PRUint32 aFlags) -{ - if (*aBuffer == aText || (aFlags & gfxFontGroup::TEXT_IS_PERSISTENT)) - return aText; - PRUint8 *newText = new PRUint8[aLength]; - if (!newText) - return nsnull; - memcpy(newText, aText, aLength); - *aBuffer = newText; - return newText; -} - gfxTextRun * gfxTextRunCache::GetOrMakeTextRun(const PRUnichar *aText, PRUint32 aLength, gfxFontGroup *aFontGroup, @@ -152,30 +124,22 @@ gfxTextRunCache::GetOrMakeTextRun(const PRUnichar *aText, PRUint32 aLength, key.mFontOrGroup = aFontGroup; entry = mCache.GetEntry(key); } - nsAutoArrayPtr text; if (entry) { gfxTextRun *textRun = entry->mTextRun; if (aCallerOwns) { *aCallerOwns = PR_FALSE; return textRun; } - aText = CloneText(aText, aLength, &text, aFlags); - if (!aText) - return nsnull; gfxTextRun *newRun = textRun->Clone(aParams, aText, aLength, aFontGroup, aFlags); if (newRun) { newRun->SetHashCode(hashCode); entry->mTextRun = newRun; NotifyRemovedFromCache(textRun); - text.forget(); return newRun; } } - aText = CloneText(aText, aLength, &text, aFlags); - if (!aText) - return nsnull; gfxTextRun *newRun = aFontGroup->MakeTextRun(aText, aLength, aParams, aFlags); if (newRun) { @@ -188,7 +152,6 @@ gfxTextRunCache::GetOrMakeTextRun(const PRUnichar *aText, PRUint32 aLength, NS_ASSERTION(!entry || entry == mCache.GetEntry(GetKeyForTextRun(newRun)), "Inconsistent hashing"); } - text.forget(); return newRun; } @@ -220,16 +183,13 @@ gfxTextRunCache::GetOrMakeTextRun(const PRUint8 *aText, PRUint32 aLength, key.mFontOrGroup = aFontGroup; entry = mCache.GetEntry(key); } - nsAutoArrayPtr text; if (entry) { gfxTextRun *textRun = entry->mTextRun; if (aCallerOwns) { *aCallerOwns = PR_FALSE; return textRun; } - aText = CloneText(aText, aLength, &text, aFlags); - if (!aText) - return nsnull; + gfxTextRun *newRun = textRun->Clone(aParams, aText, aLength, aFontGroup, aFlags); @@ -237,14 +197,10 @@ gfxTextRunCache::GetOrMakeTextRun(const PRUint8 *aText, PRUint32 aLength, newRun->SetHashCode(hashCode); entry->mTextRun = newRun; NotifyRemovedFromCache(textRun); - text.forget(); return newRun; } } - aText = CloneText(aText, aLength, &text, aFlags); - if (!aText) - return nsnull; gfxTextRun *newRun = aFontGroup->MakeTextRun(aText, aLength, aParams, aFlags); if (newRun) { @@ -257,7 +213,6 @@ gfxTextRunCache::GetOrMakeTextRun(const PRUint8 *aText, PRUint32 aLength, NS_ASSERTION(!entry || entry == mCache.GetEntry(GetKeyForTextRun(newRun)), "Inconsistent hashing"); } - text.forget(); return newRun; } diff --git a/mozilla/layout/generic/Makefile.in b/mozilla/layout/generic/Makefile.in index 0e7d4da0cfc..3fe34bcabd5 100644 --- a/mozilla/layout/generic/Makefile.in +++ b/mozilla/layout/generic/Makefile.in @@ -169,7 +169,7 @@ CPPSRCS += \ endif # set this to 1 to enable the new text frame -MOZ_ENABLE_NEW_TEXT_FRAME = +MOZ_ENABLE_NEW_TEXT_FRAME = 1 ifdef MOZ_ENABLE_NEW_TEXT_FRAME CPPSRCS += \ diff --git a/mozilla/layout/generic/nsTextFrameThebes.cpp b/mozilla/layout/generic/nsTextFrameThebes.cpp index 6698a537383..927eb37d393 100644 --- a/mozilla/layout/generic/nsTextFrameThebes.cpp +++ b/mozilla/layout/generic/nsTextFrameThebes.cpp @@ -1488,6 +1488,7 @@ BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer) textRun = transformingFactory->MakeTextRun(text, transformedLength, ¶ms, fontGroup, textFlags, styles.Elements()); if (textRun) { + // ownership of the factory has passed to the textrun transformingFactory.forget(); } } else { @@ -1504,6 +1505,7 @@ BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer) textRun = transformingFactory->MakeTextRun(text, transformedLength, ¶ms, fontGroup, textFlags, styles.Elements()); if (textRun) { + // ownership of the factory has passed to the textrun transformingFactory.forget(); } } else { diff --git a/mozilla/layout/generic/nsTextRunTransformations.cpp b/mozilla/layout/generic/nsTextRunTransformations.cpp index 58bd31e1be3..d7f722d4324 100644 --- a/mozilla/layout/generic/nsTextRunTransformations.cpp +++ b/mozilla/layout/generic/nsTextRunTransformations.cpp @@ -58,9 +58,11 @@ public: nsTransformingTextRunFactory* aFactory, gfxFontGroup* aFontGroup, const PRUnichar* aString, PRUint32 aLength, - const PRUint32 aFlags, nsStyleContext** aStyles) + const PRUint32 aFlags, nsStyleContext** aStyles, + PRBool aOwnsFactory) : gfxTextRun(aParams, aString, aLength, aFontGroup, aFlags), - mFactory(aFactory), mRefContext(aParams->mContext) + mFactory(aFactory), mRefContext(aParams->mContext), + mOwnsFactory(aOwnsFactory) { PRUint32 i; for (i = 0; i < aLength; ++i) { @@ -70,6 +72,12 @@ public: mLineBreaks.AppendElement(aParams->mInitialBreaks[i]); } } + + ~nsTransformedTextRun() { + if (mOwnsFactory) { + delete mFactory; + } + } virtual PRBool SetPotentialLineBreaks(PRUint32 aStart, PRUint32 aLength, PRPackedBool* aBreakBefore) @@ -82,10 +90,11 @@ public: PRBool aLineBreakBefore, PRBool aLineBreakAfter, gfxFloat* aAdvanceWidthDelta); - nsAutoPtr mFactory; - nsRefPtr mRefContext; - nsTArray mLineBreaks; - nsTArray > mStyles; + nsTransformingTextRunFactory *mFactory; + nsRefPtr mRefContext; + nsTArray mLineBreaks; + nsTArray > mStyles; + PRPackedBool mOwnsFactory; }; PRBool @@ -143,22 +152,14 @@ gfxTextRun* nsTransformingTextRunFactory::MakeTextRun(const PRUnichar* aString, PRUint32 aLength, const gfxTextRunFactory::Parameters* aParams, gfxFontGroup* aFontGroup, PRUint32 aFlags, - nsStyleContext** aStyles) + nsStyleContext** aStyles, PRBool aOwnsFactory) { - PRUnichar* text = nsnull; - if (!(aFlags & gfxFontGroup::TEXT_IS_PERSISTENT)) { - text = new PRUnichar[aLength]; - if (!text) - return nsnull; - memcpy(text, aString, aLength*sizeof(PRUnichar)); - } nsTransformedTextRun* textRun = new nsTransformedTextRun(aParams, this, aFontGroup, - text ? text : aString, aLength, aFlags, aStyles); - if (!textRun) { - delete[] text; + aString, aLength, aFlags, aStyles, aOwnsFactory); + if (!textRun) return nsnull; - } + RebuildTextRun(textRun); return textRun; } @@ -167,14 +168,14 @@ gfxTextRun* nsTransformingTextRunFactory::MakeTextRun(const PRUint8* aString, PRUint32 aLength, const gfxTextRunFactory::Parameters* aParams, gfxFontGroup* aFontGroup, PRUint32 aFlags, - nsStyleContext** aStyles) + nsStyleContext** aStyles, PRBool aOwnsFactory) { // We'll only have a Unicode code path to minimize the amount of code needed // for these rarely used features NS_ConvertASCIItoUTF16 unicodeString(NS_REINTERPRET_CAST(const char*, aString), aLength); return MakeTextRun(unicodeString.get(), aLength, aParams, aFontGroup, aFlags & ~(gfxFontGroup::TEXT_IS_PERSISTENT | gfxFontGroup::TEXT_IS_8BIT), - aStyles); + aStyles, aOwnsFactory); } static PRUint32 @@ -335,12 +336,15 @@ nsFontVariantTextRunFactory::RebuildTextRun(nsTransformedTextRun* aTextRun) PRUint32 flags; gfxTextRunFactory::Parameters innerParams = GetParametersForInner(aTextRun, &flags); + // The text outlives the child and inner textruns + flags |= gfxFontGroup::TEXT_IS_PERSISTENT; PRUint32 length = aTextRun->GetLength(); const PRUnichar* str = aTextRun->GetTextUnicode(); nsRefPtr* styles = aTextRun->mStyles.Elements(); // Create a textrun so we can check cluster-start properties nsAutoPtr inner; + // This text is going to outlive the inner text run inner = fontGroup->MakeTextRun(str, length, &innerParams, flags); if (!inner) return; @@ -388,8 +392,7 @@ nsFontVariantTextRunFactory::RebuildTextRun(nsTransformedTextRun* aTextRun) innerParams.mInitialBreakCount = lineBreakBeforeArray.Length(); if (runIsLowercase) { child = uppercaseFactory.MakeTextRun(str + runStart, i - runStart, - &innerParams, smallFont, flags, - styleArray.Elements()); + &innerParams, smallFont, flags, styleArray.Elements(), PR_FALSE); } else { child = fontGroup-> MakeTextRun(str + runStart, i - runStart, &innerParams, flags); @@ -515,7 +518,7 @@ nsCaseTransformTextRunFactory::RebuildTextRun(nsTransformedTextRun* aTextRun) if (mInnerTransformingTextRunFactory) { child = mInnerTransformingTextRunFactory->MakeTextRun( convertedString.BeginReading(), convertedString.Length(), - &innerParams, fontGroup, flags, styleArray.Elements()); + &innerParams, fontGroup, flags, styleArray.Elements(), PR_FALSE); } else { child = fontGroup->MakeTextRun( convertedString.BeginReading(), convertedString.Length(), &innerParams, diff --git a/mozilla/layout/generic/nsTextRunTransformations.h b/mozilla/layout/generic/nsTextRunTransformations.h index 5f90bc4fc7b..2c259fda21e 100644 --- a/mozilla/layout/generic/nsTextRunTransformations.h +++ b/mozilla/layout/generic/nsTextRunTransformations.h @@ -51,11 +51,11 @@ public: gfxTextRun* MakeTextRun(const PRUint8* aString, PRUint32 aLength, const gfxFontGroup::Parameters* aParams, gfxFontGroup* aFontGroup, PRUint32 aFlags, - nsStyleContext** aStyles); + nsStyleContext** aStyles, PRBool aOwnsFactory = PR_TRUE); gfxTextRun* MakeTextRun(const PRUnichar* aString, PRUint32 aLength, const gfxFontGroup::Parameters* aParams, gfxFontGroup* aFontGroup, PRUint32 aFlags, - nsStyleContext** aStyles); + nsStyleContext** aStyles, PRBool aOwnsFactory = PR_TRUE); virtual void RebuildTextRun(nsTransformedTextRun* aTextRun) = 0; }; @@ -81,6 +81,7 @@ public: // just convert the string to uppercase or lowercase and create the textrun // via the fontgroup. + // Takes ownership of aInnerTransformTextRunFactory nsCaseTransformTextRunFactory(nsTransformingTextRunFactory* aInnerTransformingTextRunFactory, PRBool aAllUppercase = PR_FALSE) : mInnerTransformingTextRunFactory(aInnerTransformingTextRunFactory), diff --git a/mozilla/layout/svg/base/src/nsSVGGlyphFrame.cpp b/mozilla/layout/svg/base/src/nsSVGGlyphFrame.cpp index 9871846309d..ea8debf6b5d 100644 --- a/mozilla/layout/svg/base/src/nsSVGGlyphFrame.cpp +++ b/mozilla/layout/svg/base/src/nsSVGGlyphFrame.cpp @@ -1359,12 +1359,7 @@ nsSVGGlyphFrame::GetTextRun(gfxContext *aCtx, const nsString &aText) if (!mFontGroup) return nsnull; - PRUnichar* text = new PRUnichar[aText.Length()]; - if (!text) - return nsnull; - memcpy(text, aText.get(), sizeof(PRUnichar)*aText.Length()); - - return mFontGroup->MakeTextRun(text, aText.Length(), ¶ms, 0); + return mFontGroup->MakeTextRun(aText.get(), aText.Length(), ¶ms, 0); } //----------------------------------------------------------------------