bug 609691 - check result of gfxFontGroup::MakeTextRun for failure. r=karlt a=joe

gfx/thebes/gfxTextRunWordCache.cpp

@@ -452,22 +452,29 @@ TextRunWordCache::FinishTextRun(gfxTextRun *aTextRun, gfxTextRun *aNewRun,
         }
         // If the word starts inside a cluster we don't want this word
         // in the cache, so we'll remove the associated cache entry
-        PRBool wordStartsInsideCluster =
+        PRBool wordStartsInsideCluster;
-            !source->IsClusterStart(word->mSourceOffset);
+        PRBool wordStartsInsideLigature;
-        PRBool wordStartsInsideLigature =
+        if (aSuccessful) {
-            !source->IsLigatureGroupStart(word->mSourceOffset);
+            wordStartsInsideCluster =
+                !source->IsClusterStart(word->mSourceOffset);
+            wordStartsInsideLigature =
+                !source->IsLigatureGroupStart(word->mSourceOffset);
+        }
         if (source == aNewRun) {
             // We created a cache entry for this word based on the assumption
             // that the word matches GetFontAt(0). If this assumption is false,
             // we need to remove that cache entry and replace it with an entry
             // keyed off the fontgroup.
-            PRBool rekeyWithFontGroup =
+            PRBool removeFontKey = !aSuccessful ||
-                GetWordFontOrGroup(aNewRun, word->mSourceOffset, word->mLength) != font && !useFontGroup;
+                wordStartsInsideCluster || wordStartsInsideLigature ||
-            if (!aSuccessful || rekeyWithFontGroup ||
+                (!useFontGroup && font != GetWordFontOrGroup(aNewRun,
-                wordStartsInsideCluster || wordStartsInsideLigature) {
+                                                             word->mSourceOffset,
+                                                             word->mLength));
+            if (removeFontKey) {
                 // We need to remove the current placeholder cache entry
-                CacheHashKey key(aTextRun, (useFontGroup ? (void*)fontGroup : (void*)font), word->mDestOffset, word->mLength,
+                CacheHashKey key(aTextRun,
-                                 word->mHash);
+                                 (useFontGroup ? (void*)fontGroup : (void*)font),
+                                 word->mDestOffset, word->mLength, word->mHash);
                 NS_ASSERTION(mCache.GetEntry(key),
                              "This entry should have been added previously!");
                 mCache.RemoveEntry(key);
@@ -525,9 +532,20 @@ TextRunWordCache::FinishTextRun(gfxTextRun *aTextRun, gfxTextRun *aNewRun,
                     tmpTextRun = aNewRun->GetFontGroup()->MakeTextRun(
                         source->GetTextUnicode() + sourceOffset, length, aParams,
                         aNewRun->GetFlags());
-                    source = tmpTextRun;
+                    if (tmpTextRun) {
-                    sourceOffset = 0;
+                        source = tmpTextRun;
-                    stealData = PR_TRUE;
+                        sourceOffset = 0;
+                        stealData = PR_TRUE;
+                    } else {
+                        // If we failed to create the temporary run (OOM),
+                        // skip the word, as if aSuccessful had been FALSE.
+                        // (In practice this is only likely to occur if
+                        // we're on the verge of an OOM crash anyhow.
+                        // But ignoring gfxFontGroup::MakeTextRun() failure
+                        // is bad because it means we'd be using an invalid
+                        // source pointer.)
+                        continue;
+                    }
                 }
             }
             aTextRun->CopyGlyphDataFrom(source, sourceOffset, length,
@@ -638,11 +656,23 @@ TextRunWordCache::MakeTextRun(const PRUnichar *aText, PRUint32 aLength,
                     aFontGroup->MakeTextRun(numString.get(), length, aParams,
                                             aFlags & ~(gfxTextRunFactory::TEXT_IS_PERSISTENT |
                                                        gfxTextRunFactory::TEXT_IS_8BIT));
-                DeferredWord word = { numRun, 0, wordStart, length, hash };
+                // If MakeTextRun failed, numRun will be null, which is bad...
-                deferredWords.AppendElement(word);
+                // we'll just pretend there wasn't a digit to process.
-                transientRuns.AppendElement(numRun);
+                // This means we won't have the correct numerals, but at least
-                seenDigitToModify = PR_FALSE;
+                // we're not trying to copy glyph data from an invalid source.
-            } else {
+                // In practice it's unlikely to happen unless we're very close
+                // to crashing due to OOM.
+                if (numRun) {
+                    DeferredWord word = { numRun, 0, wordStart, length, hash };
+                    deferredWords.AppendElement(word);
+                    transientRuns.AppendElement(numRun);
+                } else {
+                    seenDigitToModify = PR_FALSE;
+                }
+            }
+
+            if (!seenDigitToModify) {
+                // didn't need to modify digits (or failed to do so)
                 PRBool hit = LookupWord(textRun, font, wordStart, i, hash,
                                         deferredWords.Length() == 0 ? nsnull : &deferredWords);
                 if (!hit) {
@@ -667,7 +697,10 @@ TextRunWordCache::MakeTextRun(const PRUnichar *aText, PRUint32 aLength,
                     } // else we should set this character to be invisible missing,
                       // but it already is because the textrun is blank!
                 }
+            } else {
+                seenDigitToModify = PR_FALSE;
             }
+
             hash = 0;
             wordStart = i + 1;
         } else {
@@ -762,11 +795,16 @@ TextRunWordCache::MakeTextRun(const PRUint8 *aText, PRUint32 aLength,
                     aFontGroup->MakeTextRun(numString.get(), length, aParams,
                                             aFlags & ~(gfxTextRunFactory::TEXT_IS_PERSISTENT |
                                                        gfxTextRunFactory::TEXT_IS_8BIT));
-                DeferredWord word = { numRun, 0, wordStart, length, hash };
+                if (numRun) {
-                deferredWords.AppendElement(word);
+                    DeferredWord word = { numRun, 0, wordStart, length, hash };
-                transientRuns.AppendElement(numRun);
+                    deferredWords.AppendElement(word);
-                seenDigitToModify = PR_FALSE;
+                    transientRuns.AppendElement(numRun);
-            } else {
+                } else {
+                    seenDigitToModify = PR_FALSE;
+                }
+            }
+
+            if (!seenDigitToModify) {
                 PRBool hit = LookupWord(textRun, font, wordStart, i, hash,
                                         deferredWords.Length() == 0 ? nsnull : &deferredWords);
                 if (!hit) {
@@ -791,7 +829,10 @@ TextRunWordCache::MakeTextRun(const PRUint8 *aText, PRUint32 aLength,
                     } // else we should set this character to be invisible missing,
                       // but it already is because the textrun is blank!
                 }
+            } else {
+                seenDigitToModify = PR_FALSE;
             }
+
             hash = 0;
             wordStart = i + 1;
         } else {