Backing out changeset ee61ea93a2fa from bug 609691 due to causing permanent orange on OSX crashtests. CLOSED TREE. a=backout
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 01 Dec 2010 11:07:40 -0800
changeset 58451 aa71e65493395f1874142e0e4f7e608b31b758da
parent 58450 ca55c84f43fd52d0c1ba7face219bef7061ad69c
child 58452 01bdda1252f075114b204248097f491c5c2d9af9
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersbackout
bugs609691
milestone2.0b8pre
Backing out changeset ee61ea93a2fa from bug 609691 due to causing permanent orange on OSX crashtests. CLOSED TREE. a=backout
gfx/thebes/gfxTextRunWordCache.cpp
--- a/gfx/thebes/gfxTextRunWordCache.cpp
+++ b/gfx/thebes/gfxTextRunWordCache.cpp
@@ -447,39 +447,32 @@ TextRunWordCache::FinishTextRun(gfxTextR
     for (i = 0; i < aDeferredWords.Length(); ++i) {
         const DeferredWord *word = &aDeferredWords[i];
         gfxTextRun *source = word->mSourceTextRun;
         if (!source) {
             source = 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 wordStartsInsideLigature;
-        if (aSuccessful) {
-            wordStartsInsideCluster =
-                !source->IsClusterStart(word->mSourceOffset);
-            wordStartsInsideLigature =
-                !source->IsLigatureGroupStart(word->mSourceOffset);
-        }
+        PRBool wordStartsInsideCluster =
+            !source->IsClusterStart(word->mSourceOffset);
+        PRBool 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 removeFontKey = !aSuccessful ||
-                wordStartsInsideCluster || wordStartsInsideLigature ||
-                (!useFontGroup && font != GetWordFontOrGroup(aNewRun,
-                                                             word->mSourceOffset,
-                                                             word->mLength));
-            if (removeFontKey) {
+            PRBool rekeyWithFontGroup =
+                GetWordFontOrGroup(aNewRun, word->mSourceOffset, word->mLength) != font && !useFontGroup;
+            if (!aSuccessful || rekeyWithFontGroup ||
+                wordStartsInsideCluster || wordStartsInsideLigature) {
                 // We need to remove the current placeholder cache entry
-                CacheHashKey key(aTextRun,
-                                 (useFontGroup ? (void*)fontGroup : (void*)font),
-                                 word->mDestOffset, word->mLength, word->mHash);
+                CacheHashKey key(aTextRun, (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);
 #ifdef DEBUG
                 --aTextRun->mCachedWords;
 #endif
                 PR_LOG(gWordCacheLog, PR_LOG_DEBUG, ("%p(%d-%d,%d): removed using font", aTextRun, word->mDestOffset, word->mLength, word->mHash));
                 
@@ -527,30 +520,19 @@ TextRunWordCache::FinishTextRun(gfxTextR
                     // had to prepend a space just so we could detect this
                     // situation (so we can keep this "word" out of the
                     // cache). But now the data in aNewRun is no use to us.
                     // We need to find out what the platform would do
                     // if the characters were at the start of the text.
                     tmpTextRun = aNewRun->GetFontGroup()->MakeTextRun(
                         source->GetTextUnicode() + sourceOffset, length, aParams,
                         aNewRun->GetFlags());
-                    if (tmpTextRun) {
-                        source = tmpTextRun;
-                        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;
-                    }
+                    source = tmpTextRun;
+                    sourceOffset = 0;
+                    stealData = PR_TRUE;
                 }
             }
             aTextRun->CopyGlyphDataFrom(source, sourceOffset, length,
                 destOffset, stealData);
             // Fill in additional spaces
             PRUint32 endCharIndex;
             if (i + 1 < aDeferredWords.Length()) {
                 endCharIndex = aDeferredWords[i + 1].mDestOffset;
@@ -651,33 +633,21 @@ TextRunWordCache::MakeTextRun(const PRUn
                                                       mBidiNumeral);
                 }
                 // now we make a transient textRun for the transformed word; this will not be cached
                 gfxTextRun *numRun;
                 numRun =
                     aFontGroup->MakeTextRun(numString.get(), length, aParams,
                                             aFlags & ~(gfxTextRunFactory::TEXT_IS_PERSISTENT |
                                                        gfxTextRunFactory::TEXT_IS_8BIT));
-                // If MakeTextRun failed, numRun will be null, which is bad...
-                // we'll just pretend there wasn't a digit to process.
-                // This means we won't have the correct numerals, but at least
-                // we're not trying to copy glyph data from an invalid source.
-                // 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)
+                DeferredWord word = { numRun, 0, wordStart, length, hash };
+                deferredWords.AppendElement(word);
+                transientRuns.AppendElement(numRun);
+                seenDigitToModify = PR_FALSE;
+            } else {
                 PRBool hit = LookupWord(textRun, font, wordStart, i, hash,
                                         deferredWords.Length() == 0 ? nsnull : &deferredWords);
                 if (!hit) {
                     // Always put a space before the word so we can detect
                     // combining characters at the start of a word
                     tempString.AppendElement(' ');
                     PRUint32 offset = tempString.Length();
                     PRUint32 length = i - wordStart;
@@ -692,20 +662,17 @@ TextRunWordCache::MakeTextRun(const PRUn
                 }
 
                 if (deferredWords.Length() == 0) {
                     if (IsBoundarySpace(ch) && i < aLength) {
                         textRun->SetSpaceGlyph(font, aParams->mContext, i);
                     } // 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 {
             hash = HashMix(hash, ch);
         }
     }
 
     if (deferredWords.Length() == 0) {
@@ -790,26 +757,21 @@ TextRunWordCache::MakeTextRun(const PRUi
                                                       mBidiNumeral);
                 }
                 // now we make a transient textRun for the transformed word; this will not be cached
                 gfxTextRun *numRun;
                 numRun =
                     aFontGroup->MakeTextRun(numString.get(), length, aParams,
                                             aFlags & ~(gfxTextRunFactory::TEXT_IS_PERSISTENT |
                                                        gfxTextRunFactory::TEXT_IS_8BIT));
-                if (numRun) {
-                    DeferredWord word = { numRun, 0, wordStart, length, hash };
-                    deferredWords.AppendElement(word);
-                    transientRuns.AppendElement(numRun);
-                } else {
-                    seenDigitToModify = PR_FALSE;
-                }
-            }
-
-            if (!seenDigitToModify) {
+                DeferredWord word = { numRun, 0, wordStart, length, hash };
+                deferredWords.AppendElement(word);
+                transientRuns.AppendElement(numRun);
+                seenDigitToModify = PR_FALSE;
+            } else {
                 PRBool hit = LookupWord(textRun, font, wordStart, i, hash,
                                         deferredWords.Length() == 0 ? nsnull : &deferredWords);
                 if (!hit) {
                     if (tempString.Length() > 0) {
                         tempString.AppendElement(' ');
                     }
                     PRUint32 offset = tempString.Length();
                     PRUint32 length = i - wordStart;
@@ -824,20 +786,17 @@ TextRunWordCache::MakeTextRun(const PRUi
                 }
 
                 if (deferredWords.Length() == 0) {
                     if (IsBoundarySpace(ch) && i < aLength) {
                         textRun->SetSpaceGlyph(font, aParams->mContext, i);
                     } // 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 {
             hash = HashMix(hash, ch);
         }
     }
 
     if (deferredWords.Length() == 0) {