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

@@ -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
+    // If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the
-    // may be null if aLength is zero).
+    // 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
+     * Make a textrun for a given string.
-     * aFlags & TEXT_IS_PERSISTENT --- in that case, the caller must destroy
+     * If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the
-     * the textrun before aString dies.
+     * 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
+     * Make a textrun for a given string.
-     * aFlags & TEXT_IS_PERSISTENT --- in that case, the caller must destroy
+     * If aText is not persistent (aFlags & TEXT_IS_PERSISTENT), the
-     * the textrun before aString dies.
+     * textrun will copy it.
      */
     virtual gfxTextRun *MakeTextRun(const PRUint8 *aString, PRUint32 aLength,
                                     const Parameters *aParams, PRUint32 aFlags) = 0;

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;    
+        }
     }
 }
 

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<PRUnichar> *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<PRUint8> *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<PRUnichar> 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<PRUint8> 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;
 }
 

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   += \

mozilla/layout/generic/nsTextFrameThebes.cpp

@@ -1488,6 +1488,7 @@ BuildTextRunsScanner::BuildTextRunForFrames(void* aTextBuffer)
       textRun = transformingFactory->MakeTextRun(text, transformedLength, &params,
                                                  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, &params,
                                                  fontGroup, textFlags, styles.Elements());
       if (textRun) {
+        // ownership of the factory has passed to the textrun
         transformingFactory.forget();
       }
     } else {

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<nsTransformingTextRunFactory> mFactory;
+  nsTransformingTextRunFactory       *mFactory;
-  nsRefPtr<gfxContext>                    mRefContext;
+  nsRefPtr<gfxContext>                mRefContext;
-  nsTArray<PRUint32>                      mLineBreaks;
+  nsTArray<PRUint32>                  mLineBreaks;
-  nsTArray<nsRefPtr<nsStyleContext> >     mStyles;
+  nsTArray<nsRefPtr<nsStyleContext> > 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);
+                             aString, aLength, aFlags, aStyles, aOwnsFactory);
-  if (!textRun) {
+  if (!textRun)
-    delete[] text;
     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<nsStyleContext>* styles = aTextRun->mStyles.Elements();
   // Create a textrun so we can check cluster-start properties
   nsAutoPtr<gfxTextRun> 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,
+            &innerParams, smallFont, flags, styleArray.Elements(), PR_FALSE);
-                                             styleArray.Elements());
       } 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,

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),

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()];
+  return mFontGroup->MakeTextRun(aText.get(), aText.Length(), &params, 0);
-  if (!text)
-    return nsnull;
-  memcpy(text, aText.get(), sizeof(PRUnichar)*aText.Length());
-
-  return mFontGroup->MakeTextRun(text, aText.Length(), &params, 0);
 }
 
 //----------------------------------------------------------------------