Bug 1145937 - Don't set FontFaceSet status to Loaded between a font download completion and the document reflow. r=jdaggett
authorCameron McCormack <cam@mcc.id.au>
Fri, 27 Mar 2015 21:05:22 +1100
changeset 264944 447a722809972ac41d5465ffdfcbb6584076fdc2
parent 264943 7a7b97047c76a98b40049fcd8280809da2820521
child 264945 346bbcaeff387faadd2cc493f1a6fd77b2fcea14
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdaggett
bugs1145937
milestone39.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 1145937 - Don't set FontFaceSet status to Loaded between a font download completion and the document reflow. r=jdaggett
layout/style/FontFaceSet.cpp
layout/style/FontFaceSet.h
layout/style/test/test_font_loading_api.html
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -77,16 +77,17 @@ NS_INTERFACE_MAP_END_INHERITING(DOMEvent
 FontFaceSet::FontFaceSet(nsPIDOMWindow* aWindow, nsPresContext* aPresContext)
   : DOMEventTargetHelper(aWindow)
   , mPresContext(aPresContext)
   , mDocument(aPresContext->Document())
   , mStatus(FontFaceSetLoadStatus::Loaded)
   , mNonRuleFacesDirty(false)
   , mHasLoadingFontFaces(false)
   , mHasLoadingFontFacesIsDirty(false)
+  , mDelayedLoadCheck(false)
 {
   MOZ_COUNT_CTOR(FontFaceSet);
 
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aWindow);
 
   // If the pref is not set, don't create the Promise (which the page wouldn't
   // be able to get to anyway) as it causes the window.FontFaceSet constructor
   // to be created.
@@ -1340,27 +1341,46 @@ FontFaceSet::OnFontFaceStatusChanged(Fon
 
   mHasLoadingFontFacesIsDirty = true;
 
   if (aFontFace->Status() == FontFaceLoadStatus::Loading) {
     CheckLoadingStarted();
   } else {
     MOZ_ASSERT(aFontFace->Status() == FontFaceLoadStatus::Loaded ||
                aFontFace->Status() == FontFaceLoadStatus::Error);
-    CheckLoadingFinished();
+    // When a font finishes downloading, nsPresContext::UserFontSetUpdated
+    // will be called immediately afterwards to request a reflow of the
+    // relevant elements in the document.  We want to wait until the reflow
+    // request has been done before the FontFaceSet is marked as Loaded so
+    // that we don't briefly set the FontFaceSet to Loaded and then Loading
+    // again once the reflow is pending.  So we go around the event loop
+    // and call CheckLoadingFinished() after the reflow has been queued.
+    if (!mDelayedLoadCheck) {
+      mDelayedLoadCheck = true;
+      nsCOMPtr<nsIRunnable> checkTask =
+        NS_NewRunnableMethod(this, &FontFaceSet::CheckLoadingFinishedAfterDelay);
+      NS_DispatchToMainThread(checkTask);
+    }
   }
 }
 
 void
 FontFaceSet::DidRefresh()
 {
   CheckLoadingFinished();
 }
 
 void
+FontFaceSet::CheckLoadingFinishedAfterDelay()
+{
+  mDelayedLoadCheck = false;
+  CheckLoadingFinished();
+}
+
+void
 FontFaceSet::CheckLoadingStarted()
 {
   if (!HasLoadingFontFaces()) {
     return;
   }
 
   if (mStatus == FontFaceSetLoadStatus::Loading) {
     // We have already dispatched a loading event and replaced mReady
@@ -1442,16 +1462,21 @@ FontFaceSet::MightHavePendingFontLoads()
   }
 
   return false;
 }
 
 void
 FontFaceSet::CheckLoadingFinished()
 {
+  if (mDelayedLoadCheck) {
+    // Wait until the runnable posted in OnFontFaceStatusChanged calls us.
+    return;
+  }
+
   if (mStatus == FontFaceSetLoadStatus::Loaded) {
     // We've already resolved mReady and dispatched the loadingdone/loadingerror
     // events.
     return;
   }
 
   if (MightHavePendingFontLoads()) {
     // We're not finished loading yet.
--- a/layout/style/FontFaceSet.h
+++ b/layout/style/FontFaceSet.h
@@ -215,16 +215,22 @@ private:
 
   /**
    * Checks to see whether it is time to resolve mReady and dispatch any
    * "loadingdone" and "loadingerror" events.
    */
   void CheckLoadingFinished();
 
   /**
+   * Callback for invoking CheckLoadingFinished after going through the
+   * event loop.  See OnFontFaceStatusChanged.
+   */
+  void CheckLoadingFinishedAfterDelay();
+
+  /**
    * Dispatches a CSSFontFaceLoadEvent to this object.
    */
   void DispatchLoadingFinishedEvent(
                                 const nsAString& aType,
                                 const nsTArray<FontFace*>& aFontFaces);
 
   // Note: if you add new cycle collected objects to FontFaceRecord,
   // make sure to update FontFaceSet's cycle collection macros
@@ -318,14 +324,18 @@ private:
 
   // Whether any FontFace objects in mRuleFaces or mNonRuleFaces are
   // loading.  Only valid when mHasLoadingFontFacesIsDirty is false.  Don't use
   // this variable directly; call the HasLoadingFontFaces method instead.
   bool mHasLoadingFontFaces;
 
   // This variable is only valid when mLoadingDirty is false.
   bool mHasLoadingFontFacesIsDirty;
+
+  // Whether CheckLoadingFinished calls should be ignored.  See comment in
+  // OnFontFaceStatusChanged.
+  bool mDelayedLoadCheck;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // !defined(mozilla_dom_FontFaceSet_h)
--- a/layout/style/test/test_font_loading_api.html
+++ b/layout/style/test/test_font_loading_api.html
@@ -943,27 +943,27 @@ function runTest() {
 
     is(document.fonts.status, "loaded", "document.fonts.status should have status \"loaded\" (TEST 37)");
 
     face = new FontFace("test", "url(BitPattern.woff)");
     face.load();
     document.fonts.add(face);
     is(document.fonts.status, "loading", "document.fonts.status should have status \"loading\" after first font added (TEST 37)");
 
-    return face.loaded
+    return document.fonts.ready
       .then(function() {
         is(document.fonts.status, "loaded", "document.fonts.status should have status \"loaded\" after first font loaded (TEST 37)");
         is(face.status, "loaded", "first FontFace should have status \"loaded\" (TEST 37)");
 
         face2 = new FontFace("test2", "url(BitPattern.woff)");
         face2.load();
         document.fonts.add(face2);
         is(document.fonts.status, "loading", "document.fonts.status should have status \"loading\" after second font added (TEST 37)");
 
-        return face2.loaded;
+        return document.fonts.ready;
       }).then(function() {
         is(document.fonts.status, "loaded", "document.fonts.status should have status \"loaded\" after second font loaded (TEST 37)");
         is(face2.status, "loaded", "second FontFace should have status \"loaded\" (TEST 37)");
 
         is(events.length, 2, "should receive two events (TEST 37)");
 
         is(events[0].type, "loadingdone", "first event should be \"loadingdone\" (TEST 37)");
         is(events[0].fontfaces.length, 1, "first event should have 1 FontFace (TEST 37)");
@@ -995,32 +995,28 @@ function runTest() {
 
     is(document.fonts.status, "loaded", "document.fonts.status should have status \"loaded\" (TEST 38)");
 
     face = new FontFace("test", "url(x)");
     face.load();
     document.fonts.add(face);
     is(document.fonts.status, "loading", "document.fonts.status should have status \"loading\" after first font added (TEST 38)");
 
-    return face.loaded
+    return document.fonts.ready
       .then(function() {
-        ok(false, "first FontFace should not load (TEST 38)");
-      }, function() {
         is(document.fonts.status, "loaded", "document.fonts.status should have status \"loaded\" after first font failed to load (TEST 38)");
         is(face.status, "error", "first FontFace should have status \"error\" (TEST 38)");
 
         face2 = new FontFace("test2", "url(x)");
         face2.load();
         document.fonts.add(face2);
         is(document.fonts.status, "loading", "document.fonts.status should have status \"loading\" after second font added (TEST 38)");
 
-        return face2.loaded;
+        return document.fonts.ready;
       }).then(function() {
-        ok(false, "second FontFace should not load (TEST 38)");
-      }, function() {
         is(document.fonts.status, "loaded", "document.fonts.status should have status \"loaded\" after second font failed to load (TEST 38)");
         is(face2.status, "error", "second FontFace should have status \"error\" (TEST 38)");
 
         is(events.length, 4, "should receive four events (TEST 38)");
 
         is(events[0].type, "loadingdone", "first event should be \"loadingdone\" (TEST 38)");
         is(events[0].fontfaces.length, 0, "first event should have no FontFaces (TEST 38)");