Bug 1524246 - Cancel font loads from unlinking. r=jfkthame, a=lizzard
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 05 Feb 2019 14:41:57 +0000
changeset 512922 d7e94ba9dbad1189eeb1fbb4ea1f456668b23381
parent 512921 d9506222285b1c0e08818aca83bfefa4a5c18a3f
child 512923 bbdc6607d97e55aaa2b368604e55c88a1baf5a78
push id10642
push userryanvm@gmail.com
push dateThu, 07 Feb 2019 15:12:19 +0000
treeherdermozilla-beta@d7e94ba9dbad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame, lizzard
bugs1524246
milestone66.0
Bug 1524246 - Cancel font loads from unlinking. r=jfkthame, a=lizzard When the FontFaceSet gets unlinked, we remove the strong pointer it holds to to the UserFontSet. This is not strictly necessary, since that object will no longer have any reference to any other cycle collected object. In any case, the loaders keep alive the user font entries, which _don't_ keep alive the user font set (they have a weak reference instead). So if the user font set is gone, all is bad. Ensure we cancel all loads when unlinking rather than just when the object is destroyed, and that the font face loader doesn't keep a reference to the user font entry anymore after being canceled (this shouldn't be necessary either, but it's better IMO). Differential Revision: https://phabricator.services.mozilla.com/D18256
layout/style/FontFaceSet.cpp
layout/style/nsFontFaceLoader.cpp
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -167,35 +167,38 @@ FontFaceSet::FontFaceSet(nsPIDOMWindowIn
 }
 
 FontFaceSet::~FontFaceSet() {
   // Assert that we don't drop any FontFaceSet objects during a Servo traversal,
   // since PostTraversalTask objects can hold raw pointers to FontFaceSets.
   MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal());
 
   Disconnect();
-  for (auto it = mLoaders.Iter(); !it.Done(); it.Next()) {
-    it.Get()->GetKey()->Cancel();
-  }
 }
 
 JSObject* FontFaceSet::WrapObject(JSContext* aContext,
                                   JS::Handle<JSObject*> aGivenProto) {
   return FontFaceSet_Binding::Wrap(aContext, this, aGivenProto);
 }
 
 void FontFaceSet::Disconnect() {
   RemoveDOMContentLoadedListener();
 
   if (mDocument && mDocument->CSSLoader()) {
     // We're null checking CSSLoader() since FontFaceSet::Disconnect() might be
     // being called during unlink, at which time the loader amy already have
     // been unlinked from the document.
     mDocument->CSSLoader()->RemoveObserver(this);
   }
+
+  for (auto it = mLoaders.Iter(); !it.Done(); it.Next()) {
+    it.Get()->GetKey()->Cancel();
+  }
+
+  mLoaders.Clear();
 }
 
 void FontFaceSet::RemoveDOMContentLoadedListener() {
   if (mDocument) {
     mDocument->RemoveSystemEventListener(NS_LITERAL_STRING("DOMContentLoaded"),
                                          this, false);
   }
 }
--- a/layout/style/nsFontFaceLoader.cpp
+++ b/layout/style/nsFontFaceLoader.cpp
@@ -318,16 +318,17 @@ nsFontFaceLoader::OnStopRequest(nsIReque
 }
 
 void nsFontFaceLoader::Cancel() {
   MOZ_DIAGNOSTIC_ASSERT(!mInLoadTimerCallback);
   MOZ_DIAGNOSTIC_ASSERT(!mInStreamComplete);
   MOZ_DIAGNOSTIC_ASSERT(mFontFaceSet);
 
   mUserFontEntry->LoadCanceled();
+  mUserFontEntry = nullptr;
   mFontFaceSet = nullptr;
   if (mLoadTimer) {
     mLoadTimer->Cancel();
     mLoadTimer = nullptr;
   }
   nsCOMPtr<nsIChannel> channel = mChannel.forget();
   channel->Cancel(NS_BINDING_ABORTED);
 }