bug 678181 - back out second part of bug 668813 (changeset 431a8297db1f) on suspicion of causing crashiness. r=backout
authorJonathan Kew <jfkthame@gmail.com>
Sat, 13 Aug 2011 13:34:56 +0100
changeset 74336 8e1dd6f8b903ccbb4fcdff817b606fda45d41311
parent 74335 6de555980733e2838d081c6532e33b83925c25c2
child 74340 145c98d55ae16d6213fb952e8995da12b62e02a2
child 74373 19ab9ba1c62369998649c5c0192aeafc0b74a230
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
reviewersbackout
bugs678181, 668813
milestone8.0a1
bug 678181 - back out second part of bug 668813 (changeset 431a8297db1f) on suspicion of causing crashiness. r=backout
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFont.h
gfx/thebes/gfxFontUtils.cpp
gfx/thebes/gfxFontUtils.h
gfx/thebes/gfxUserFontSet.h
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -646,25 +646,22 @@ void gfxFontFamily::LocalizedName(nsAStr
     // just return the primary name; subclasses should override
     aLocalizedName = mName;
 }
 
 
 void
 gfxFontFamily::FindFontForChar(FontSearch *aMatchData)
 {
-    if (!mHasStyles) {
+    if (!mHasStyles)
         FindStyleVariations();
-    }
-
-    if (!TestCharacterMap(aMatchData->mCh)) {
-        // none of the faces in the family support the required char,
-        // so bail out immediately
-        return;
-    }
+
+    // xxx - optimization point - keep a bit vector with the union of supported unicode ranges
+    // by all fonts for this family and bail immediately if the character is not in any of
+    // this family's cmaps
 
     // iterate over fonts
     PRUint32 numFonts = mAvailableFonts.Length();
     for (PRUint32 i = 0; i < numFonts; i++) {
         gfxFontEntry *fe = mAvailableFonts[i];
 
         // skip certain fonts during system fallback
         if (!fe || fe->SkipDuringSystemFallback())
@@ -2609,18 +2606,21 @@ gfxFontGroup::FindFontForChar(PRUint32 a
     // 1. check fonts in the font group
     for (PRUint32 i = 0; i < FontListLength(); i++) {
         nsRefPtr<gfxFont> font = GetFontAt(i);
         if (font->HasCharacter(aCh)) {
             *aMatchType = gfxTextRange::kFontGroup;
             return font.forget();
         }
         // check other faces of the family
+        // XXX optimization point: give the family a charmap that is the union
+        // of the char maps of all its faces, so we can quickly test whether
+        // it's worth doing this search
         gfxFontFamily *family = font->GetFontEntry()->Family();
-        if (family && family->TestCharacterMap(aCh)) {
+        if (family) {
             FontSearch matchData(aCh, font);
             family->FindFontForChar(&matchData);
             gfxFontEntry *fe = matchData.mBestMatch;
             if (fe) {
                 PRBool needsBold =
                     font->GetStyle()->weight >= 600 && !fe->IsBold();
                 selectedFont =
                     fe->FindOrMakeFont(font->GetStyle(), needsBold);
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -477,18 +477,17 @@ public:
 
     gfxFontFamily(const nsAString& aName) :
         mName(aName),
         mOtherFamilyNamesInitialized(PR_FALSE),
         mHasOtherFamilyNames(PR_FALSE),
         mFaceNamesInitialized(PR_FALSE),
         mHasStyles(PR_FALSE),
         mIsSimpleFamily(PR_FALSE),
-        mIsBadUnderlineFamily(PR_FALSE),
-        mCharacterMapInitialized(PR_FALSE)
+        mIsBadUnderlineFamily(PR_FALSE)
         { }
 
     virtual ~gfxFontFamily() { }
 
     const nsString& Name() { return mName; }
 
     virtual void LocalizedName(nsAString& aLocalizedName);
     virtual PRBool HasOtherFamilyNames();
@@ -540,38 +539,20 @@ public:
     virtual void FindStyleVariations() { }
 
     // search for a specific face using the Postscript name
     gfxFontEntry* FindFont(const nsAString& aPostscriptName);
 
     // read in cmaps for all the faces
     void ReadCMAP() {
         PRUint32 i, numFonts = mAvailableFonts.Length();
-        for (i = 0; i < numFonts; i++) {
-            gfxFontEntry *fe = mAvailableFonts[i];
-            if (!fe) {
-                continue;
-            }
-            fe->ReadCMAP();
-            mCharacterMap.Union(fe->mCharacterMap);
-        }
-        mCharacterMap.Compact();
-        mCharacterMapInitialized = PR_TRUE;
-    }
-
-    PRBool TestCharacterMap(PRUint32 aCh) {
-        if (!mCharacterMapInitialized) {
-            ReadCMAP();
-        }
-        return mCharacterMap.test(aCh);
-    }
-
-    void ResetCharacterMap() {
-        mCharacterMap.reset();
-        mCharacterMapInitialized = PR_FALSE;
+        // called from RunLoader BEFORE CheckForSimpleFamily so that there cannot
+        // be any NULL entries in mAvailableFonts
+        for (i = 0; i < numFonts; i++)
+            mAvailableFonts[i]->ReadCMAP();
     }
 
     // mark this family as being in the "bad" underline offset blacklist
     void SetBadUnderlineFamily() {
         mIsBadUnderlineFamily = PR_TRUE;
         if (mHasStyles) {
             SetBadUnderlineFonts();
         }
@@ -604,24 +585,22 @@ protected:
             if (mAvailableFonts[i]) {
                 mAvailableFonts[i]->mIsBadUnderlineFont = PR_TRUE;
             }
         }
     }
 
     nsString mName;
     nsTArray<nsRefPtr<gfxFontEntry> >  mAvailableFonts;
-    gfxSparseBitSet mCharacterMap;
     PRPackedBool mOtherFamilyNamesInitialized;
     PRPackedBool mHasOtherFamilyNames;
     PRPackedBool mFaceNamesInitialized;
     PRPackedBool mHasStyles;
     PRPackedBool mIsSimpleFamily;
     PRPackedBool mIsBadUnderlineFamily;
-    PRPackedBool mCharacterMapInitialized;
 
     enum {
         // for "simple" families, the faces are stored in mAvailableFonts
         // with fixed positions:
         kRegularFaceIndex    = 0,
         kBoldFaceIndex       = 1,
         kItalicFaceIndex     = 2,
         kBoldItalicFaceIndex = 3,
--- a/gfx/thebes/gfxFontUtils.cpp
+++ b/gfx/thebes/gfxFontUtils.cpp
@@ -308,17 +308,17 @@ gfxFontUtils::ReadCMAPTableFormat12(cons
         NS_ENSURE_TRUE((prevEndCharCode < startCharCode || i == 0) &&
                        startCharCode <= endCharCode &&
                        endCharCode <= CMAP_MAX_CODEPOINT, 
                        NS_ERROR_GFX_CMAP_MALFORMED);
         aCharacterMap.SetRange(startCharCode, endCharCode);
         prevEndCharCode = endCharCode;
     }
 
-    aCharacterMap.Compact();
+    aCharacterMap.mBlocks.Compact();
 
     return NS_OK;
 }
 
 nsresult 
 gfxFontUtils::ReadCMAPTableFormat4(const PRUint8 *aBuf, PRUint32 aLength,
                                    gfxSparseBitSet& aCharacterMap)
 {
@@ -386,17 +386,17 @@ gfxFontUtils::ReadCMAPTableFormat4(const
                     // glyph = (ReadShortAt16(idDeltas, i) + *gdata) % 65536;
 
                     aCharacterMap.set(c);
                 }
             }
         }
     }
 
-    aCharacterMap.Compact();
+    aCharacterMap.mBlocks.Compact();
 
     return NS_OK;
 }
 
 nsresult
 gfxFontUtils::ReadCMAPTableFormat14(const PRUint8 *aBuf, PRUint32 aLength,
                                     PRUint8*& aTable)
 {
--- a/gfx/thebes/gfxFontUtils.h
+++ b/gfx/thebes/gfxFontUtils.h
@@ -160,16 +160,18 @@ public:
         if (blockIndex >= mBlocks.Length()) {
             nsAutoPtr<Block> *blocks = mBlocks.AppendElements(blockIndex + 1 - mBlocks.Length());
             if (NS_UNLIKELY(!blocks)) // OOM
                 return;
         }
         Block *block = mBlocks[blockIndex];
         if (!block) {
             block = new Block;
+            if (NS_UNLIKELY(!block)) // OOM
+                return;
             mBlocks[blockIndex] = block;
         }
         block->mBits[(aIndex>>3) & (BLOCK_SIZE - 1)] |= 1 << (aIndex & 0x7);
     }
 
     void set(PRUint32 aIndex, PRBool aValue) {
         if (aValue)
             set(aIndex);
@@ -194,16 +196,19 @@ public:
 
             Block *block = mBlocks[i];
             if (!block) {
                 PRBool fullBlock = PR_FALSE;
                 if (aStart <= blockFirstBit && aEnd >= blockLastBit)
                     fullBlock = PR_TRUE;
 
                 block = new Block(fullBlock ? 0xFF : 0);
+
+                if (NS_UNLIKELY(!block)) // OOM
+                    return;
                 mBlocks[i] = block;
 
                 if (fullBlock)
                     continue;
             }
 
             const PRUint32 start = aStart > blockFirstBit ? aStart - blockFirstBit : 0;
             const PRUint32 end = NS_MIN<PRUint32>(aEnd - blockFirstBit, BLOCK_SIZE_BITS - 1);
@@ -269,54 +274,17 @@ public:
     }
 
     // clear out all blocks in the array
     void reset() {
         PRUint32 i;
         for (i = 0; i < mBlocks.Length(); i++)
             mBlocks[i] = nsnull;    
     }
-
-    // set this bitset to the union of its current contents and another
-    void Union(const gfxSparseBitSet& aBitset) {
-        // ensure mBlocks is large enough
-        PRUint32 blockCount = aBitset.mBlocks.Length();
-        if (blockCount > mBlocks.Length()) {
-            PRUint32 needed = blockCount - mBlocks.Length();
-            nsAutoPtr<Block> *blocks = mBlocks.AppendElements(needed);
-            if (NS_UNLIKELY(!blocks)) { // OOM
-                return;
-            }
-        }
-        // for each block that may be present in aBitset...
-        for (PRUint32 i = 0; i < blockCount; ++i) {
-            // if it is missing (implicitly empty), just skip
-            if (!aBitset.mBlocks[i]) {
-                continue;
-            }
-            // if the block is missing in this set, just copy the other
-            if (!mBlocks[i]) {
-                mBlocks[i] = new Block(*aBitset.mBlocks[i]);
-                continue;
-            }
-            // else set existing block to the union of both
-            PRUint32 *dst = reinterpret_cast<PRUint32*>(mBlocks[i]->mBits);
-            const PRUint32 *src =
-                reinterpret_cast<const PRUint32*>(aBitset.mBlocks[i]->mBits);
-            for (PRUint32 j = 0; j < BLOCK_SIZE / 4; ++j) {
-                dst[j] |= src[j];
-            }
-        }
-    }
-
-    void Compact() {
-        mBlocks.Compact();
-    }
-
-private:
+    
     nsTArray< nsAutoPtr<Block> > mBlocks;
 };
 
 #define TRUETYPE_TAG(a, b, c, d) ((a) << 24 | (b) << 16 | (c) << 8 | (d))
 
 namespace mozilla {
 
 // Byte-swapping types and name table structure definitions moved from
--- a/gfx/thebes/gfxUserFontSet.h
+++ b/gfx/thebes/gfxUserFontSet.h
@@ -101,53 +101,49 @@ class gfxMixedFontFamily : public gfxFon
 public:
     friend class gfxUserFontSet;
 
     gfxMixedFontFamily(const nsAString& aName)
         : gfxFontFamily(aName) { }
 
     virtual ~gfxMixedFontFamily() { }
 
-    void AddFontEntry(gfxFontEntry *aFontEntry)
-    {
+    void AddFontEntry(gfxFontEntry *aFontEntry) {
         nsRefPtr<gfxFontEntry> fe = aFontEntry;
         mAvailableFonts.AppendElement(fe);
         aFontEntry->SetFamily(this);
-        ResetCharacterMap();
     }
 
     void ReplaceFontEntry(gfxFontEntry *aOldFontEntry, gfxFontEntry *aNewFontEntry) 
     {
         PRUint32 numFonts = mAvailableFonts.Length();
         for (PRUint32 i = 0; i < numFonts; i++) {
             gfxFontEntry *fe = mAvailableFonts[i];
             if (fe == aOldFontEntry) {
                 aOldFontEntry->SetFamily(nsnull);
                 // note that this may delete aOldFontEntry, if there's no
                 // other reference to it except from its family
                 mAvailableFonts[i] = aNewFontEntry;
                 aNewFontEntry->SetFamily(this);
-                break;
+                return;
             }
         }
-        ResetCharacterMap();
     }
 
     void RemoveFontEntry(gfxFontEntry *aFontEntry) 
     {
         PRUint32 numFonts = mAvailableFonts.Length();
         for (PRUint32 i = 0; i < numFonts; i++) {
             gfxFontEntry *fe = mAvailableFonts[i];
             if (fe == aFontEntry) {
                 aFontEntry->SetFamily(nsnull);
                 mAvailableFonts.RemoveElementAt(i);
-                break;
+                return;
             }
         }
-        ResetCharacterMap();
     }
 
     // temp method to determine if all proxies are loaded
     PRBool AllLoaded() 
     {
         PRUint32 numFonts = mAvailableFonts.Length();
         for (PRUint32 i = 0; i < numFonts; i++) {
             gfxFontEntry *fe = mAvailableFonts[i];