Bug 847272 - avoid userfontset updates within font stream handling methods. r=jkew
authorJohn Daggett <jdaggett@mozilla.com>
Thu, 14 Mar 2013 14:24:40 +0900
changeset 124781 06f85e10a40c8c094ae3c029472cf4e8fd4cf84d
parent 124780 e38d0a3fe054f8e55b900c48d9801237258ebc5f
child 124782 8af239e2e57c6dbe7a1e013201f8b6089887891b
push id24601
push userjdaggett@mozilla.com
push dateThu, 14 Mar 2013 05:25:19 +0000
treeherdermozilla-inbound@06f85e10a40c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjkew
bugs847272
milestone22.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 847272 - avoid userfontset updates within font stream handling methods. r=jkew
gfx/thebes/gfxUserFontSet.h
layout/style/nsFontFaceLoader.cpp
--- a/gfx/thebes/gfxUserFontSet.h
+++ b/gfx/thebes/gfxUserFontSet.h
@@ -81,26 +81,28 @@ public:
         mAvailableFonts.AppendElement(fe);
         aFontEntry->mFamilyName = Name();
         ResetCharacterMap();
     }
 
     void ReplaceFontEntry(gfxFontEntry *aOldFontEntry,
                           gfxFontEntry *aNewFontEntry) {
         uint32_t numFonts = mAvailableFonts.Length();
-        for (uint32_t i = 0; i < numFonts; i++) {
+        uint32_t i;
+        for (i = 0; i < numFonts; i++) {
             gfxFontEntry *fe = mAvailableFonts[i];
             if (fe == aOldFontEntry) {
                 // note that this may delete aOldFontEntry, if there's no
                 // other reference to it except from its family
                 aNewFontEntry->mFamilyName = Name();
                 mAvailableFonts[i] = aNewFontEntry;
                 break;
             }
         }
+        NS_ASSERTION(i < numFonts, "font entry not found in family!");
         ResetCharacterMap();
     }
 
     void RemoveFontEntry(gfxFontEntry *aFontEntry) {
         uint32_t numFonts = mAvailableFonts.Length();
         for (uint32_t i = 0; i < numFonts; i++) {
             gfxFontEntry *fe = mAvailableFonts[i];
             if (fe == aFontEntry) {
--- a/layout/style/nsFontFaceLoader.cpp
+++ b/layout/style/nsFontFaceLoader.cpp
@@ -97,16 +97,21 @@ nsFontFaceLoader::StartedLoading(nsIStre
   mStreamLoader = aStreamLoader;
 }
 
 void
 nsFontFaceLoader::LoadTimerCallback(nsITimer *aTimer, void *aClosure)
 {
   nsFontFaceLoader *loader = static_cast<nsFontFaceLoader*>(aClosure);
 
+  if (!loader->mFontSet) {
+    // We've been canceled
+    return;
+  }
+
   gfxProxyFontEntry *pe = loader->mFontEntry.get();
   bool updateUserFontSet = true;
 
   // If the entry is loading, check whether it's >75% done; if so,
   // we allow another timeout period before showing a fallback font.
   if (pe->mLoadingState == gfxProxyFontEntry::LOADING_STARTED) {
     int64_t contentLength;
     uint32_t numBytesRead;
@@ -131,20 +136,20 @@ nsFontFaceLoader::LoadTimerCallback(nsIT
     }
   }
 
   // If the font is not 75% loaded, or if we've already timed out once
   // before, we mark this entry as "loading slowly", so the fallback
   // font will be used in the meantime, and tell the context to refresh.
   if (updateUserFontSet) {
     pe->mLoadingState = gfxProxyFontEntry::LOADING_SLOWLY;
+    gfxUserFontSet *fontSet = loader->mFontSet;
     nsPresContext *ctx = loader->mFontSet->GetPresContext();
-    NS_ASSERTION(ctx, "fontSet doesn't have a presContext?");
-    gfxUserFontSet *fontSet;
-    if (ctx && (fontSet = ctx->GetUserFontSet()) != nullptr) {
+    NS_ASSERTION(ctx, "userfontset doesn't have a presContext?");
+    if (ctx) {
       fontSet->IncrementGeneration();
       ctx->UserFontSetUpdated();
       LOG(("fontdownloader (%p) timeout reflow\n", loader));
     }
   }
 }
 
 NS_IMPL_ISUPPORTS1(nsFontFaceLoader, nsIStreamLoaderObserver)
@@ -176,22 +181,16 @@ nsFontFaceLoader::OnStreamComplete(nsISt
     }
   }
 #endif
 
   nsPresContext *ctx = mFontSet->GetPresContext();
   NS_ASSERTION(ctx && !ctx->PresShell()->IsDestroying(),
                "We should have been canceled already");
 
-  // whether an error occurred or not, notify the user font set of the completion
-  gfxUserFontSet *userFontSet = ctx->GetUserFontSet();
-  if (!userFontSet) {
-    return aStatus;
-  }
-
   if (NS_SUCCEEDED(aStatus)) {
     // for HTTP requests, check whether the request _actually_ succeeded;
     // the "request status" in aStatus does not necessarily indicate this,
     // because HTTP responses such as 404 (Not Found) will still result in
     // a success code and potentially an HTML error page from the server
     // as the resulting data. We don't want to use that as a font.
     nsCOMPtr<nsIRequest> request;
     nsCOMPtr<nsIHttpChannel> httpChannel;
@@ -207,29 +206,34 @@ nsFontFaceLoader::OnStreamComplete(nsISt
   }
 
   // The userFontSet is responsible for freeing the downloaded data
   // (aString) when finished with it; the pointer is no longer valid
   // after OnLoadComplete returns.
   // This is called even in the case of a failed download (HTTP 404, etc),
   // as there may still be data to be freed (e.g. an error page),
   // and we need the fontSet to initiate loading the next source.
-  bool fontUpdate = userFontSet->OnLoadComplete(mFontFamily,
-                                                mFontEntry,
-                                                aString, aStringLen,
-                                                aStatus);
+  bool fontUpdate = mFontSet->OnLoadComplete(mFontFamily, mFontEntry, aString,
+                                             aStringLen, aStatus);
 
   // when new font loaded, need to reflow
   if (fontUpdate) {
     // Update layout for the presence of the new font.  Since this is
     // asynchronous, reflows will coalesce.
     ctx->UserFontSetUpdated();
     LOG(("fontdownloader (%p) reflow\n", this));
   }
 
+  // done with font set
+  mFontSet = nullptr;
+  if (mLoadTimer) {
+    mLoadTimer->Cancel();
+    mLoadTimer = nullptr;
+  }
+
   return NS_SUCCESS_ADOPTED_DATA;
 }
 
 void
 nsFontFaceLoader::Cancel()
 {
   mFontEntry->mLoadingState = gfxProxyFontEntry::NOT_LOADING;
   mFontEntry->mLoader = nullptr;