Bug 1665196 - Use the main-thread idle queue rather than a repeating …

…timer to finalize font loading. r=lsalzman

Differential Revision: https://phabricator.services.mozilla.com/D92302

dom/events/test/mochitest.ini

@@ -1,7 +1,11 @@
 [DEFAULT]
 # Skip migration work in BG__migrateUI for browser_startup.js since it increases
 # the occurrence of the leak reported in bug 1398563 with test_bug1327798.html.
-prefs = browser.migration.version=9999999
+# Run the font-loader eagerly to minimize the risk that font list finalization
+# may disrupt the events received or result in a timeout.
+prefs =
+  browser.migration.version=9999999
+  gfx.font_loader.delay=0
 skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM
 support-files =
   bug226361_iframe.xhtml

gfx/thebes/gfxFontInfoLoader.cpp

@@ -128,9 +128,7 @@ gfxFontInfoLoader::ShutdownObserver::Observe(nsISupports* aSubject,
   return NS_OK;
 }
 
-void gfxFontInfoLoader::StartLoader(uint32_t aDelay, uint32_t aInterval) {
-  mInterval = aInterval;
-
+void gfxFontInfoLoader::StartLoader(uint32_t aDelay) {
   NS_ASSERTION(!mFontInfo, "fontinfo should be null when starting font loader");
 
   // sanity check
@@ -183,6 +181,12 @@ void gfxFontInfoLoader::StartLoader(uint32_t aDelay, uint32_t aInterval) {
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
+
+  PRThread* prThread;
+  if (NS_SUCCEEDED(mFontLoaderThread->GetPRThread(&prThread))) {
+    PR_SetThreadPriority(prThread, PR_PRIORITY_LOW);
+  }
+
   mState = stateAsyncLoad;
 
   nsCOMPtr<nsIRunnable> loadEvent = new AsyncFontInfoLoader(mFontInfo);
@@ -195,6 +199,32 @@ void gfxFontInfoLoader::StartLoader(uint32_t aDelay, uint32_t aInterval) {
   }
 }
 
+class FinalizeLoaderRunnable : public Runnable {
+  virtual ~FinalizeLoaderRunnable() = default;
+
+ public:
+  NS_INLINE_DECL_REFCOUNTING_INHERITED(FinalizeLoaderRunnable, Runnable)
+
+  explicit FinalizeLoaderRunnable(gfxFontInfoLoader* aLoader)
+      : mozilla::Runnable("FinalizeLoaderRunnable"), mLoader(aLoader) {}
+
+  NS_IMETHOD Run() override {
+    nsresult rv;
+    if (mLoader->LoadFontInfo()) {
+      mLoader->CancelLoader();
+      rv = NS_OK;
+    } else {
+      nsCOMPtr<nsIRunnable> runnable = this;
+      rv = NS_DispatchToCurrentThreadQueue(
+          runnable.forget(), PR_INTERVAL_NO_TIMEOUT, EventQueuePriority::Idle);
+    }
+    return rv;
+  }
+
+ private:
+  gfxFontInfoLoader* mLoader;
+};
+
 void gfxFontInfoLoader::FinalizeLoader(FontInfoData* aFontInfo) {
   // Avoid loading data if loader has already been canceled.
   // This should mean that CancelLoader() ran and the Load
@@ -207,24 +237,13 @@ void gfxFontInfoLoader::FinalizeLoader(FontInfoData* aFontInfo) {
 
   mLoadTime = mFontInfo->mLoadTime;
 
-  // try to load all font data immediately
-  if (LoadFontInfo()) {
-    CancelLoader();
-    return;
+  MOZ_ASSERT(NS_IsMainThread());
+  nsCOMPtr<nsIRunnable> runnable = new FinalizeLoaderRunnable(this);
+  if (NS_FAILED(NS_DispatchToCurrentThreadQueue(runnable.forget(),
+                                                PR_INTERVAL_NO_TIMEOUT,
+                                                EventQueuePriority::Idle))) {
+    NS_WARNING("Failed to finalize async font info");
   }
-
-  NS_ASSERTION(!sFontLoaderShutdownObserved,
-               "Bug 1508626 - Finalize with interval timer for font loader "
-               "after shutdown observed");
-  NS_ASSERTION(!gXPCOMThreadsShutDown,
-               "Bug 1508626 - Finalize with interval timer for font loader "
-               "after shutdown but before observer");
-
-  // not all work completed ==> run load on interval
-  mState = stateTimerOnInterval;
-  mTimer->InitWithNamedFuncCallback(LoadFontInfoCallback, this, mInterval,
-                                    nsITimer::TYPE_REPEATING_SLACK,
-                                    "gfxFontInfoLoader::FinalizeLoader");
 }
 
 void gfxFontInfoLoader::CancelLoader() {
@@ -246,25 +265,6 @@ void gfxFontInfoLoader::CancelLoader() {
   CleanupLoader();
 }
 
-void gfxFontInfoLoader::LoadFontInfoTimerFire() {
-  if (mState == stateTimerOnDelay) {
-    NS_ASSERTION(!sFontLoaderShutdownObserved,
-                 "Bug 1508626 - Setting interval timer for font loader after "
-                 "shutdown observed");
-    NS_ASSERTION(!gXPCOMThreadsShutDown,
-                 "Bug 1508626 - Setting interval timer for font loader after "
-                 "shutdown but before observer");
-
-    mState = stateTimerOnInterval;
-    mTimer->SetDelay(mInterval);
-  }
-
-  bool done = LoadFontInfo();
-  if (done) {
-    CancelLoader();
-  }
-}
-
 gfxFontInfoLoader::~gfxFontInfoLoader() {
   RemoveShutdownObserver();
   MOZ_COUNT_DTOR(gfxFontInfoLoader);

gfx/thebes/gfxFontInfoLoader.h

@@ -125,46 +125,43 @@ class FontInfoData {
 // (e.g. localized names, face names, cmaps) are loaded async.
 
 // helper class for loading in font info on a separate async thread
-// once async thread completes, completion process is run on regular
-// intervals to prevent tying up the main thread
+// once async thread completes, completion process is run on the main
+// thread's idle queue in short slices
 
 class gfxFontInfoLoader {
  public:
   // state transitions:
   //   initial ---StartLoader with delay---> timer on delay
-  //   initial ---StartLoader without delay---> timer on interval
-  //   timer on delay ---LoaderTimerFire---> timer on interval
+  //   initial ---StartLoader without delay---> timer off
+  //   timer on delay ---LoaderTimerFire---> timer off
   //   timer on delay ---CancelLoader---> timer off
-  //   timer on interval ---CancelLoader---> timer off
   //   timer off ---StartLoader with delay---> timer on delay
-  //   timer off ---StartLoader without delay---> timer on interval
+  //   timer off ---StartLoader without delay---> timer off
   typedef enum {
     stateInitial,
     stateTimerOnDelay,
     stateAsyncLoad,
-    stateTimerOnInterval,
     stateTimerOff
   } TimerState;
 
-  gfxFontInfoLoader() : mInterval(0), mState(stateInitial) {
+  gfxFontInfoLoader() : mState(stateInitial) {
     MOZ_COUNT_CTOR(gfxFontInfoLoader);
   }
 
   virtual ~gfxFontInfoLoader();
 
-  // start timer with an initial delay, then call Run method at regular
-  // intervals
-  void StartLoader(uint32_t aDelay, uint32_t aInterval);
+  // start timer with an initial delay
+  void StartLoader(uint32_t aDelay);
 
-  // Finalize - async load complete, transfer data (on intervals if necessary)
+  // Finalize - async load complete, transfer data (on idle)
   virtual void FinalizeLoader(FontInfoData* aFontInfo);
 
   // cancel the timer and cleanup
   void CancelLoader();
 
-  uint32_t GetInterval() { return mInterval; }
-
  protected:
+  friend class FinalizeLoaderRunnable;
+
   class ShutdownObserver : public nsIObserver {
    public:
     NS_DECL_ISUPPORTS
@@ -194,15 +191,9 @@ class gfxFontInfoLoader {
   // Cleanup - finish and cleanup after done, including possible reflows
   virtual void CleanupLoader() { mFontInfo = nullptr; }
 
-  // Timer interval callbacks
-  static void LoadFontInfoCallback(nsITimer* aTimer, void* aThis) {
-    gfxFontInfoLoader* loader = static_cast<gfxFontInfoLoader*>(aThis);
-    loader->LoadFontInfoTimerFire();
-  }
-
   static void DelayedStartCallback(nsITimer* aTimer, void* aThis) {
     gfxFontInfoLoader* loader = static_cast<gfxFontInfoLoader*>(aThis);
-    loader->StartLoader(0, loader->GetInterval());
+    loader->StartLoader(0);
   }
 
   void LoadFontInfoTimerFire();
@@ -213,7 +204,6 @@ class gfxFontInfoLoader {
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsIObserver> mObserver;
   nsCOMPtr<nsIThread> mFontLoaderThread;
-  uint32_t mInterval;
   TimerState mState;
 
   // after async font loader completes, data is stored here

gfx/thebes/gfxPlatformFontList.cpp

@@ -127,10 +127,6 @@ const gfxFontEntry::ScriptRange gfxPlatformFontList::sComplexScriptRanges[] = {
     {0, 0, 0, {0, 0, 0}}  // terminator
 };
 
-// prefs for the font info loader
-#define FONT_LOADER_DELAY_PREF "gfx.font_loader.delay"
-#define FONT_LOADER_INTERVAL_PREF "gfx.font_loader.interval"
-
 static const char* kObservedPrefs[] = {"font.",
                                        "font.name-list.",
                                        "intl.accept_languages",  // hmmmm...
@@ -638,7 +634,7 @@ void gfxPlatformFontList::InitOtherFamilyNames(
   // (This is used so we can reliably run reftests that depend on localized
   // font-family names being available.)
   if (aDeferOtherFamilyNamesLoading &&
-      Preferences::GetUint(FONT_LOADER_DELAY_PREF) > 0) {
+      StaticPrefs::gfx_font_loader_delay_AtStartup() > 0) {
     if (!mPendingOtherFamilyNameTask) {
       RefPtr<mozilla::CancelableRunnable> task =
           new InitOtherFamilyNamesRunnable();
@@ -2104,7 +2100,7 @@ void gfxPlatformFontList::InitLoader() {
 }
 
 #define FONT_LOADER_MAX_TIMESLICE \
-  100  // max time for one pass through RunLoader = 100ms
+  20  // max time for one pass through RunLoader = 20ms
 
 bool gfxPlatformFontList::LoadFontInfo() {
   TimeStamp start = TimeStamp::Now();
@@ -2142,12 +2138,16 @@ bool gfxPlatformFontList::LoadFontInfo() {
       }
     }
 
-    // limit the time spent reading fonts in one pass
-    TimeDuration elapsed = TimeStamp::Now() - start;
-    if (elapsed.ToMilliseconds() > FONT_LOADER_MAX_TIMESLICE &&
-        i + 1 != endIndex) {
-      endIndex = i + 1;
-      break;
+    // Limit the time spent reading fonts in one pass, unless the font-loader
+    // delay was set to zero, in which case we run to completion even if it
+    // causes some jank.
+    if (StaticPrefs::gfx_font_loader_delay_AtStartup() > 0) {
+      TimeDuration elapsed = TimeStamp::Now() - start;
+      if (elapsed.ToMilliseconds() > FONT_LOADER_MAX_TIMESLICE &&
+          i + 1 != endIndex) {
+        endIndex = i + 1;
+        break;
+      }
     }
   }
 
@@ -2216,11 +2216,8 @@ void gfxPlatformFontList::CleanupLoader() {
 }
 
 void gfxPlatformFontList::GetPrefsAndStartLoader() {
-  uint32_t delay = std::max(1u, Preferences::GetUint(FONT_LOADER_DELAY_PREF));
-  uint32_t interval =
-      std::max(1u, Preferences::GetUint(FONT_LOADER_INTERVAL_PREF));
-
-  StartLoader(delay, interval);
+  uint32_t delay = std::max(1u, StaticPrefs::gfx_font_loader_delay_AtStartup());
+  StartLoader(delay);
 }
 
 void gfxPlatformFontList::ForceGlobalReflow() {

modules/libpref/init/StaticPrefList.yaml

@@ -4199,6 +4199,15 @@
   value: @IS_NIGHTLY_BUILD@
   mirror: once
 
+- name: gfx.font_loader.delay
+  type: uint32_t
+#if defined(XP_WIN)
+  value: 60000
+#else
+  value: 8000
+#endif
+  mirror: once
+
 # Disable antialiasing of Ahem, for use in tests.
 - name: gfx.font_rendering.ahem_antialias_none
   type: RelaxedAtomicBool

modules/libpref/init/all.js

@@ -612,15 +612,6 @@ pref("gfx.downloadable_fonts.disable_cache", false);
 // whether to try and do something about it (e.g. download additional fonts)?
 pref("gfx.missing_fonts.notify", false);
 
-// prefs controlling the font (name/cmap) loader that runs shortly after startup
-#ifdef XP_WIN
-  pref("gfx.font_loader.delay", 120000);       // 2 minutes after startup
-  pref("gfx.font_loader.interval", 1000);      // every 1 second until complete
-#else
-  pref("gfx.font_loader.delay", 8000);         // 8 secs after startup
-  pref("gfx.font_loader.interval", 50);        // run every 50 ms
-#endif
-
 // whether to always search all font cmaps during system font fallback
 pref("gfx.font_rendering.fallback.always_use_cmaps", false);