Bug 1524246 - Cancel font loads from unlinking. r=jfkthame
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 05 Feb 2019 14:41:57 +0000
changeset 457342 a89ff647fa824059fa9dc4f536a0bc4e22f710ae
parent 457341 241a3659869bd944111ee48233ba27ceb2040af7
child 457343 8993106d37a62df5e66c61b0e7d56d7d818d23a5
push id35506
push useropoprus@mozilla.com
push dateWed, 06 Feb 2019 09:47:29 +0000
treeherdermozilla-central@2853480ed90d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1524246
milestone67.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 1524246 - Cancel font loads from unlinking. r=jfkthame 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);
 }