Bug 1077746 - Move FontFace pointer from nsCSSFontFaceRule to a table on FontFaceSet. r=jdaggett
authorCameron McCormack <cam@mcc.id.au>
Mon, 06 Oct 2014 15:29:35 +1100
changeset 233445 7fceb8bf84d2a79f7aafdefca064d90bf30f5c41
parent 233444 a20031e192ceafbf7ba8ed1aa4d1a80e0d3ec3c3
child 233446 ad8d4f7be999ff2205280fa37146b6d90b28456f
push id611
push userraliiev@mozilla.com
push dateMon, 05 Jan 2015 23:23:16 +0000
treeherdermozilla-release@345cd3b9c445 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdaggett
bugs1077746
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1077746 - Move FontFace pointer from nsCSSFontFaceRule to a table on FontFaceSet. r=jdaggett
layout/style/FontFaceSet.cpp
layout/style/FontFaceSet.h
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -350,24 +350,32 @@ FontFaceSet::Length()
 static PLDHashOperator DestroyIterator(nsPtrHashKey<nsFontFaceLoader>* aKey,
                                        void* aUserArg)
 {
   aKey->GetKey()->Cancel();
   return PL_DHASH_REMOVE;
 }
 
 void
+FontFaceSet::DisconnectFromRule(FontFace* aFontFace)
+{
+  nsCSSFontFaceRule* rule = aFontFace->GetRule();
+  aFontFace->DisconnectFromRule();
+  mRuleFaceMap.Remove(rule);
+}
+
+void
 FontFaceSet::DestroyUserFontSet()
 {
   Disconnect();
   mDocument = nullptr;
   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++) {
     mNonRuleFaces[i]->SetUserFontEntry(nullptr);
   }
   for (size_t i = 0; i < mUnavailableFaces.Length(); i++) {
     mUnavailableFaces[i]->SetUserFontEntry(nullptr);
   }
@@ -582,17 +590,17 @@ FontFaceSet::UpdateRules(const nsTArray<
         }
       }
 
       // Any left over FontFace objects should also cease being rule backed.
       MOZ_ASSERT(!mUnavailableFaces.Contains(f),
                  "FontFace should not occur in mUnavailableFaces twice");
 
       mUnavailableFaces.AppendElement(f);
-      f->DisconnectFromRule();
+      DisconnectFromRule(f);
     }
   }
 
   if (modified) {
     IncrementGeneration(true);
     mHasLoadingFontFacesIsDirty = true;
     CheckLoadingStarted();
     CheckLoadingFinished();
@@ -978,17 +986,17 @@ FontFaceSet::FindRuleForUserFontEntry(gf
     }
   }
   return nullptr;
 }
 
 gfxUserFontEntry*
 FontFaceSet::FindUserFontEntryForRule(nsCSSFontFaceRule* aRule)
 {
-  FontFace* f = aRule->GetFontFace();
+  FontFace* f = mRuleFaceMap.Get(aRule);
   if (f) {
     return f->GetUserFontEntry();
   }
   return nullptr;
 }
 
 nsresult
 FontFaceSet::LogMessage(gfxUserFontEntry* aUserFontEntry,
@@ -1263,27 +1271,33 @@ FontFaceSet::DoRebuildUserFontSet()
   }
 
   mPresContext->RebuildUserFontSet();
 }
 
 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;
   }
 
   // We might be creating a FontFace object for an @font-face rule that we are
   // just about to create a user font entry for, so entry might be null.
   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;
 }
 
 void
 FontFaceSet::AddUnavailableFontFace(FontFace* aFontFace)
 {
   MOZ_ASSERT(!aFontFace->HasRule());
   MOZ_ASSERT(!aFontFace->IsInFontFaceSet());
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -192,16 +192,22 @@ private:
   bool HasAvailableFontFace(FontFace* aFontFace);
 
   /**
    * Removes any listeners and observers.
    */
   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.
    */
   bool MightHavePendingFontLoads();
 
   /**
    * Checks to see whether it is time to replace mReady and dispatch a
    * "loading" event.
@@ -300,16 +306,23 @@ private:
   // The non rule backed FontFace objects that have been added to this
   // FontFaceSet and their corresponding user font entries.
   nsTArray<nsRefPtr<FontFace>> mNonRuleFaces;
 
   // The non rule backed FontFace objects that have not been added to
   // 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;
 
   // Whether mNonRuleFaces has changed since last time UpdateRules ran.
   bool mNonRuleFacesDirty;
 
   // Whether we have called MaybeResolve() on mReady.
   bool mReadyIsResolved;