From c47631b0ac57bb5ac9bd8bf84cbbfa783235c061 Mon Sep 17 00:00:00 2001
From: Cameron McCormack <cam@mcc.id.au>
Date: Mon, 6 Oct 2014 15:29:35 +1100
Subject: [PATCH] Bug 1077746 - Move FontFace pointer from nsCSSFontFaceRule to
 a table on FontFaceSet. r=jdaggett

---
 layout/style/FontFaceSet.cpp | 24 +++++++++++++++++++-----
 layout/style/FontFaceSet.h   | 13 +++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/layout/style/FontFaceSet.cpp b/layout/style/FontFaceSet.cpp
index ad2b093f7e8e..0a0410ea11fe 100644
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -354,6 +354,14 @@ static PLDHashOperator DestroyIterator(nsPtrHashKey<nsFontFaceLoader>* aKey,
   return PL_DHASH_REMOVE;
 }
 
+void
+FontFaceSet::DisconnectFromRule(FontFace* aFontFace)
+{
+  nsCSSFontFaceRule* rule = aFontFace->GetRule();
+  aFontFace->DisconnectFromRule();
+  mRuleFaceMap.Remove(rule);
+}
+
 void
 FontFaceSet::DestroyUserFontSet()
 {
@@ -362,7 +370,7 @@ FontFaceSet::DestroyUserFontSet()
   mPresContext = nullptr;
   mLoaders.EnumerateEntries(DestroyIterator, nullptr);
   for (size_t i = 0; i < mRuleFaces.Length(); i++) {
-    mRuleFaces[i].mFontFace->DisconnectFromRule();
+    DisconnectFromRule(mRuleFaces[i].mFontFace);
     mRuleFaces[i].mFontFace->SetUserFontEntry(nullptr);
   }
   for (size_t i = 0; i < mNonRuleFaces.Length(); i++) {
@@ -587,7 +595,7 @@ FontFaceSet::UpdateRules(const nsTArray<nsFontFaceRuleContainer>& aRules)
                  "FontFace should not occur in mUnavailableFaces twice");
 
       mUnavailableFaces.AppendElement(f);
-      f->DisconnectFromRule();
+      DisconnectFromRule(f);
     }
   }
 
@@ -983,7 +991,7 @@ FontFaceSet::FindRuleForUserFontEntry(gfxUserFontEntry* aUserFontEntry)
 gfxUserFontEntry*
 FontFaceSet::FindUserFontEntryForRule(nsCSSFontFaceRule* aRule)
 {
-  FontFace* f = aRule->GetFontFace();
+  FontFace* f = mRuleFaceMap.Get(aRule);
   if (f) {
     return f->GetUserFontEntry();
   }
@@ -1268,8 +1276,12 @@ FontFaceSet::DoRebuildUserFontSet()
 FontFace*
 FontFaceSet::FontFaceForRule(nsCSSFontFaceRule* aRule)
 {
-  FontFace* f = aRule->GetFontFace();
+  MOZ_ASSERT(aRule);
+
+  FontFace* f = mRuleFaceMap.Get(aRule);
   if (f) {
+    MOZ_ASSERT(f->GetFontFaceSet() == this,
+               "existing FontFace is from another FontFaceSet?");
     return f;
   }
 
@@ -1278,7 +1290,9 @@ FontFaceSet::FontFaceForRule(nsCSSFontFaceRule* aRule)
   gfxUserFontEntry* entry = FindUserFontEntryForRule(aRule);
   nsRefPtr<FontFace> newFontFace =
     FontFace::CreateForRule(GetParentObject(), mPresContext, aRule, entry);
-  aRule->SetFontFace(newFontFace);
+  MOZ_ASSERT(newFontFace->GetFontFaceSet() == this,
+             "new FontFace is from another FontFaceSet?");
+  mRuleFaceMap.Put(aRule, newFontFace);
   return newFontFace;
 }
 
diff --git a/layout/style/FontFaceSet.h b/layout/style/FontFaceSet.h
index 7490adbc6fac..816104dbb82f 100644
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -196,6 +196,12 @@ private:
    */
   void Disconnect();
 
+  /**
+   * Calls DisconnectFromRule on the given FontFace and removes its entry from
+   * mRuleFaceMap.
+   */
+  void DisconnectFromRule(FontFace* aFontFace);
+
   /**
    * Returns whether there might be any pending font loads, which should cause
    * the mReady Promise not to be resolved yet.
@@ -305,6 +311,13 @@ private:
   // this FontFaceSet.
   nsTArray<FontFace*> mUnavailableFaces;
 
+  // Map of nsCSSFontFaceRule objects to FontFace objects.  We hold a weak
+  // reference to both; for actively used FontFaces, mRuleFaces will hold
+  // a strong reference to the FontFace and the FontFace will hold on to
+  // the nsCSSFontFaceRule.  FontFaceSet::DisconnectFromRule will ensure its
+  // entry in this array will be removed.
+  nsDataHashtable<nsPtrHashKey<nsCSSFontFaceRule>, FontFace*> mRuleFaceMap;
+
   // The overall status of the loading or loaded fonts in the FontFaceSet.
   mozilla::dom::FontFaceSetLoadStatus mStatus;