Bug 1028497 - Part 19: Support disconnecting FontFaces that reflect @font-face rules. r=jdaggett,bzbarsky
authorCameron McCormack <cam@mcc.id.au>
Thu, 02 Oct 2014 12:32:08 +1000
changeset 231547 6585686da666f99414f32bee20cf38983e282edc
parent 231546 e37382cb217343afd8efd35268eab571932be209
child 231548 19897140f7f2e1480fee807164d1a56d9c6cef08
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdaggett, bzbarsky
bugs1028497
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 1028497 - Part 19: Support disconnecting FontFaces that reflect @font-face rules. r=jdaggett,bzbarsky This adds support for a CSS-connected FontFace to be disconnected from its rule. This causes it to get its own copy of the descriptors on the nsCSSFontFaceStyleDecl, and for the pointers between the FontFace and the nsCSSFontFaceRule to be nulled out. We start tracking now whether a given FontFace is in the FontFaceSet (in the sense that it will appear on the DOM FontFaceSet object if we inspect it with script). All FontFace objects created though, whether they are currently "in" the FontFaceSet or not, are still tracked by the FontFaceSet. We use the new mUnavailableFaces array on the FontFaceSet for that. We need to track these FontFaces that aren't in the FontFaceSet as that's where we store their user font entry -- important if we call load() on a FontFace before adding it to the FontFaceSet.
layout/style/FontFace.cpp
layout/style/FontFace.h
layout/style/FontFaceSet.cpp
layout/style/FontFaceSet.h
layout/style/nsCSSRules.h
--- a/layout/style/FontFace.cpp
+++ b/layout/style/FontFace.cpp
@@ -10,50 +10,77 @@
 #include "mozilla/dom/Promise.h"
 #include "nsCSSParser.h"
 #include "nsCSSRules.h"
 #include "nsIDocument.h"
 #include "nsStyleUtil.h"
 
 using namespace mozilla::dom;
 
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(FontFace, mParent, mLoaded, mRule)
+NS_IMPL_CYCLE_COLLECTION_CLASS(FontFace)
+
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(FontFace)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mLoaded)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRule)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFontFaceSet)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS
+NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
+
+NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(FontFace)
+  if (!tmp->IsInFontFaceSet()) {
+    tmp->mFontFaceSet->RemoveUnavailableFontFace(tmp);
+  }
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoaded)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mRule)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK(mFontFaceSet)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
+NS_IMPL_CYCLE_COLLECTION_UNLINK_END
+
+NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(FontFace)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
+NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(FontFace)
   NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
   NS_INTERFACE_MAP_ENTRY(nsISupports)
 NS_INTERFACE_MAP_END
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(FontFace)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(FontFace)
 
 FontFace::FontFace(nsISupports* aParent, nsPresContext* aPresContext)
   : mParent(aParent)
   , mPresContext(aPresContext)
   , mStatus(FontFaceLoadStatus::Unloaded)
+  , mFontFaceSet(aPresContext->Fonts())
+  , mInFontFaceSet(false)
 {
   MOZ_COUNT_CTOR(FontFace);
 
-  MOZ_ASSERT(mPresContext);
-
   SetIsDOMBinding();
 
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aParent);
 
   if (global) {
     ErrorResult rv;
     mLoaded = Promise::Create(global, rv);
   }
 }
 
 FontFace::~FontFace()
 {
   MOZ_COUNT_DTOR(FontFace);
 
   SetUserFontEntry(nullptr);
+
+  if (mFontFaceSet && !IsInFontFaceSet()) {
+    mFontFaceSet->RemoveUnavailableFontFace(this);
+  }
 }
 
 JSObject*
 FontFace::WrapObject(JSContext* aCx)
 {
   return FontFaceBinding::Wrap(aCx, this);
 }
 
@@ -79,16 +106,17 @@ FontFace::CreateForRule(nsISupports* aGl
                         nsPresContext* aPresContext,
                         nsCSSFontFaceRule* aRule,
                         gfxUserFontEntry* aUserFontEntry)
 {
   nsCOMPtr<nsIGlobalObject> globalObject = do_QueryInterface(aGlobal);
 
   nsRefPtr<FontFace> obj = new FontFace(aGlobal, aPresContext);
   obj->mRule = aRule;
+  obj->mInFontFaceSet = true;
   obj->SetUserFontEntry(aUserFontEntry);
   return obj.forget();
 }
 
 already_AddRefed<FontFace>
 FontFace::Constructor(const GlobalObject& aGlobal,
                       const nsAString& aFamily,
                       const StringOrArrayBufferOrArrayBufferView& aSource,
@@ -111,16 +139,17 @@ FontFace::Constructor(const GlobalObject
 
   nsPresContext* presContext = shell->GetPresContext();
   if (!presContext) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   nsRefPtr<FontFace> obj = new FontFace(global, presContext);
+  obj->mFontFaceSet->AddUnavailableFontFace(obj);
   return obj.forget();
 }
 
 void
 FontFace::GetFamily(nsString& aResult)
 {
   mPresContext->FlushUserFontSet();
 
@@ -428,16 +457,30 @@ FontFace::GetFamilyName(nsString& aResul
     nsString familyname;
     value.GetStringValue(familyname);
     aResult.Append(familyname);
   }
 
   return !aResult.IsEmpty();
 }
 
+void
+FontFace::DisconnectFromRule()
+{
+  MOZ_ASSERT(IsConnected());
+
+  // Make a copy of the descriptors.
+  mDescriptors = new CSSFontFaceDescriptors;
+  mRule->GetDescriptors(*mDescriptors);
+
+  mRule->SetFontFace(nullptr);
+  mRule = nullptr;
+  mInFontFaceSet = false;
+}
+
 // -- FontFace::Entry --------------------------------------------------------
 
 /* virtual */ void
 FontFace::Entry::SetLoadState(UserFontLoadState aLoadState)
 {
   gfxUserFontEntry::SetLoadState(aLoadState);
 
   for (size_t i = 0; i < mFontFaces.Length(); i++) {
--- a/layout/style/FontFace.h
+++ b/layout/style/FontFace.h
@@ -14,16 +14,17 @@
 
 class nsCSSFontFaceRule;
 class nsPresContext;
 
 namespace mozilla {
 struct CSSFontFaceDescriptors;
 namespace dom {
 struct FontFaceDescriptors;
+class FontFaceSet;
 class Promise;
 class StringOrArrayBufferOrArrayBufferView;
 }
 }
 
 namespace mozilla {
 namespace dom {
 
@@ -73,23 +74,36 @@ public:
 
   nsCSSFontFaceRule* GetRule() { return mRule; }
 
   void GetDesc(nsCSSFontDesc aDescID, nsCSSValue& aResult) const;
 
   gfxUserFontEntry* GetUserFontEntry() const { return mUserFontEntry; }
   void SetUserFontEntry(gfxUserFontEntry* aEntry);
 
+  bool IsInFontFaceSet() { return mInFontFaceSet; }
+
   /**
    * Gets the family name of the FontFace as a raw string (such as 'Times', as
    * opposed to GetFamily, which returns a CSS-escaped string, such as
    * '"Times"').  Returns whether a valid family name was available.
    */
   bool GetFamilyName(nsString& aResult);
 
+  /**
+   * Returns whether this object is CSS-connected, i.e. reflecting an
+   * @font-face rule.
+   */
+  bool IsConnected() const { return mRule; }
+
+  /**
+   * Breaks the connection between this FontFace and its @font-face rule.
+   */
+  void DisconnectFromRule();
+
   // Web IDL
   static already_AddRefed<FontFace>
   Constructor(const GlobalObject& aGlobal,
               const nsAString& aFamily,
               const mozilla::dom::StringOrArrayBufferOrArrayBufferView& aSource,
               const mozilla::dom::FontFaceDescriptors& aDescriptors,
               ErrorResult& aRV);
 
@@ -157,14 +171,21 @@ private:
   // status, since the spec sometimes requires us to go through the event
   // loop before updating the status, rather than doing it immediately.
   mozilla::dom::FontFaceLoadStatus mStatus;
 
   // The values corresponding to the font face descriptors, if we are not
   // a CSS-connected FontFace object.  For CSS-connected objects, we use
   // the descriptors stored in mRule.
   nsAutoPtr<mozilla::CSSFontFaceDescriptors> mDescriptors;
+
+  // The FontFaceSet this FontFace is associated with, regardless of whether
+  // it is currently "in" the set.
+  nsRefPtr<FontFaceSet> mFontFaceSet;
+
+  // Whether this FontFace appears in the FontFaceSet.
+  bool mInFontFaceSet;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // !defined(mozilla_dom_FontFace_h)
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -203,19 +203,24 @@ static PLDHashOperator DestroyIterator(n
 }
 
 void
 FontFaceSet::DestroyUserFontSet()
 {
   mPresContext = nullptr;
   mLoaders.EnumerateEntries(DestroyIterator, nullptr);
   for (size_t i = 0; i < mConnectedFaces.Length(); i++) {
+    mConnectedFaces[i].mFontFace->DisconnectFromRule();
     mConnectedFaces[i].mFontFace->SetUserFontEntry(nullptr);
   }
+  for (size_t i = 0; i < mUnavailableFaces.Length(); i++) {
+    mUnavailableFaces[i]->SetUserFontEntry(nullptr);
+  }
   mConnectedFaces.Clear();
+  mUnavailableFaces.Clear();
   mReady = nullptr;
   mUserFontSet = nullptr;
 }
 
 void
 FontFaceSet::RemoveLoader(nsFontFaceLoader* aLoader)
 {
   mLoaders.RemoveEntry(aLoader);
@@ -396,25 +401,32 @@ FontFaceSet::UpdateRules(const nsTArray<
     // will keep their "orphaned" font entries alive until they complete,
     // even after the oldRules array is deleted.
     //
     // XXX Now that it is possible for the author to hold on to a CSS-connected
     // FontFace object, we shouldn't cancel loading here; instead we should do
     // it when the FontFace is GCed, if we can detect that.
     size_t count = oldRecords.Length();
     for (size_t i = 0; i < count; ++i) {
-      gfxUserFontEntry* userFontEntry =
-        oldRecords[i].mFontFace->GetUserFontEntry();
+      nsRefPtr<FontFace> f = oldRecords[i].mFontFace;
+      gfxUserFontEntry* userFontEntry = f->GetUserFontEntry();
       if (userFontEntry) {
         nsFontFaceLoader* loader = userFontEntry->GetLoader();
         if (loader) {
           loader->Cancel();
           RemoveLoader(loader);
         }
       }
+
+      // Any left over FontFace objects should also cease being CSS-connected.
+      MOZ_ASSERT(!mUnavailableFaces.Contains(f),
+                 "FontFace should not occur in mUnavailableFaces twice");
+
+      mUnavailableFaces.AppendElement(f);
+      f->DisconnectFromRule();
     }
   }
 
   if (modified) {
     IncrementGeneration(true);
   }
 
   // local rules have been rebuilt, so clear the flag
@@ -450,32 +462,39 @@ FontFaceSet::InsertConnectedFontFace(
 {
   nsAutoString fontfamily;
   if (!aFontFace->GetFamilyName(fontfamily)) {
     // If there is no family name, this rule cannot contribute a
     // usable font, so there is no point in processing it further.
     return;
   }
 
+  bool remove = false;
+  size_t removeIndex;
+
   // This is a CSS-connected FontFace.  First, we check in aOldRecords; if
   // the FontFace for the rule exists there, just move it to the new record
   // list, and put the entry into the appropriate family.
   for (size_t i = 0; i < aOldRecords.Length(); ++i) {
     FontFaceRecord& rec = aOldRecords[i];
 
     if (rec.mFontFace == aFontFace &&
         rec.mSheetType == aSheetType) {
 
       // if local rules were used, don't use the old font entry
       // for rules containing src local usage
       if (mUserFontSet->mLocalRulesUsed) {
         nsCSSValue val;
         aFontFace->GetDesc(eCSSFontDesc_Src, val);
         nsCSSUnit unit = val.GetUnit();
         if (unit == eCSSUnit_Array && HasLocalSrc(val.GetArrayValue())) {
+          // Remove the old record, but wait to see if we successfully create a
+          // new user font entry below.
+          remove = true;
+          removeIndex = i;
           break;
         }
       }
 
       gfxUserFontEntry* entry = rec.mFontFace->GetUserFontEntry();
       MOZ_ASSERT(entry, "FontFace should have a gfxUserFontEntry by now");
 
       mUserFontSet->AddUserFontEntry(fontfamily, entry);
@@ -497,16 +516,26 @@ FontFaceSet::InsertConnectedFontFace(
   // this is a new rule:
   nsRefPtr<gfxUserFontEntry> entry =
     FindOrCreateUserFontEntryFromFontFace(fontfamily, aFontFace, aSheetType);
 
   if (!entry) {
     return;
   }
 
+  if (remove) {
+    // Although we broke out of the aOldRecords loop above, since we found
+    // src local usage, and we're not using the old user font entry, we still
+    // are adding a record to mConnectedFaces with the same FontFace object.
+    // Remove the old record so that we don't have the same FontFace listed
+    // in both mConnectedFaces and oldRecords, which would cause us to call
+    // DisconnectFromRule on a FontFace that should still be connected.
+    aOldRecords.RemoveElementAt(removeIndex);
+  }
+
   FontFaceRecord rec;
   rec.mFontFace = aFontFace;
   rec.mSheetType = aSheetType;
 
   aFontFace->SetUserFontEntry(entry);
 
   MOZ_ASSERT(!HasConnectedFontFace(aFontFace),
              "FontFace should not occur in mConnectedFaces twice");
@@ -1012,16 +1041,39 @@ FontFaceSet::FontFaceForRule(nsCSSFontFa
   // 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);
   return newFontFace;
 }
 
+void
+FontFaceSet::AddUnavailableFontFace(FontFace* aFontFace)
+{
+  MOZ_ASSERT(!aFontFace->IsInFontFaceSet());
+  MOZ_ASSERT(!mUnavailableFaces.Contains(aFontFace));
+
+  mUnavailableFaces.AppendElement(aFontFace);
+}
+
+void
+FontFaceSet::RemoveUnavailableFontFace(FontFace* aFontFace)
+{
+  MOZ_ASSERT(!aFontFace->IsConnected());
+  MOZ_ASSERT(!aFontFace->IsInFontFaceSet());
+
+  // We might not actually find the FontFace in mUnavailableFaces, since we
+  // might be shutting down the document and had DestroyUserFontSet called
+  // on us, which clears out mUnavailableFaces.
+  mUnavailableFaces.RemoveElement(aFontFace);
+
+  MOZ_ASSERT(!mUnavailableFaces.Contains(aFontFace));
+}
+
 // -- FontFaceSet::UserFontSet ------------------------------------------------
 
 /* virtual */ nsresult
 FontFaceSet::UserFontSet::CheckFontLoad(const gfxFontFaceSrc* aFontFaceSrc,
                                         nsIPrincipal** aPrincipal,
                                         bool* aBypassCache)
 {
   if (!mFontFaceSet) {
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -107,16 +107,28 @@ public:
 
   nsPresContext* GetPresContext() { return mPresContext; }
 
   // search for @font-face rule that matches a platform font entry
   nsCSSFontFaceRule* FindRuleForEntry(gfxFontEntry* aFontEntry);
 
   void IncrementGeneration(bool aIsRebuild = false);
 
+  /**
+   * Adds the specified FontFace to the mUnavailableFaces array.  This is called
+   * when a new FontFace object has just been created in JS by the author.
+   */
+  void AddUnavailableFontFace(FontFace* aFontFace);
+
+  /**
+   * Removes the specified FontFace from the mUnavailableFaces array.  This
+   * is called when a FontFace object is about be destroyed.
+   */
+  void RemoveUnavailableFontFace(FontFace* aFontFace);
+
   // -- Web IDL --------------------------------------------------------------
 
   IMPL_EVENT_HANDLER(loading)
   IMPL_EVENT_HANDLER(loadingdone)
   IMPL_EVENT_HANDLER(loadingerror)
   already_AddRefed<mozilla::dom::Promise> Load(const nsAString& aFont,
                                                const nsAString& aText,
                                                mozilla::ErrorResult& aRv);
@@ -188,14 +200,18 @@ private:
 
   // Set of all loaders pointing to us. These are not strong pointers,
   // but that's OK because nsFontFaceLoader always calls RemoveLoader on
   // us before it dies (unless we die first).
   nsTHashtable< nsPtrHashKey<nsFontFaceLoader> > mLoaders;
 
   // The CSS-connected FontFace objects in the FontFaceSet.
   nsTArray<FontFaceRecord> mConnectedFaces;
+
+  // The unconnected FontFace objects that have not been added to
+  // this FontFaceSet.
+  nsTArray<FontFace*> mUnavailableFaces;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // !defined(mozilla_dom_FontFaceSet_h)
--- a/layout/style/nsCSSRules.h
+++ b/layout/style/nsCSSRules.h
@@ -275,16 +275,19 @@ public:
   void SetDesc(nsCSSFontDesc aDescID, nsCSSValue const & aValue);
   void GetDesc(nsCSSFontDesc aDescID, nsCSSValue & aValue);
 
   virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const MOZ_OVERRIDE;
 
   mozilla::dom::FontFace* GetFontFace() const { return mFontFace; }
   void SetFontFace(mozilla::dom::FontFace* aFontFace) { mFontFace = aFontFace; }
 
+  void GetDescriptors(mozilla::CSSFontFaceDescriptors& aDescriptors) const
+    { aDescriptors = mDecl.mDescriptors; }
+
 protected:
   ~nsCSSFontFaceRule() {}
 
   // The CSS-connected FontFace object reflecting this @font-face rule, if one
   // has been created.
   nsRefPtr<mozilla::dom::FontFace> mFontFace;
 
   friend class nsCSSFontFaceStyleDecl;