Bug 1500126 - Flushing the font metrics cache needs to be done on the main thread. r=lsalzman
authorJonathan Kew <jkew@mozilla.com>
Fri, 19 Oct 2018 16:08:57 +0100
changeset 490443 214e786d1ca04c076de2d8f90086fcdbc151ee89
parent 490442 162dc0f0dbd53c31b50b1b1a062568b38f448078
child 490444 2b175e8c52636117d0e635c4cf9d14d16fcefbea
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewerslsalzman
bugs1500126
milestone64.0a1
Bug 1500126 - Flushing the font metrics cache needs to be done on the main thread. r=lsalzman
gfx/src/nsDeviceContext.cpp
--- a/gfx/src/nsDeviceContext.cpp
+++ b/gfx/src/nsDeviceContext.cpp
@@ -74,19 +74,44 @@ protected:
     // productive because it can then take too long to scan the cache.
     static const int32_t kMaxCacheEntries = 128;
 
     ~nsFontCache() {}
 
     nsDeviceContext*          mContext; // owner
     RefPtr<nsAtom>         mLocaleLanguage;
 
-    // We don't allow this array to grow beyond kMaxCacheEntries,
-    // so use an autoarray to avoid separate allocation.
-    AutoTArray<nsFontMetrics*,kMaxCacheEntries> mFontMetrics;
+    // We may not flush older entries immediately the array reaches
+    // kMaxCacheEntries length, because this usually happens on a stylo
+    // thread where we can't safely delete metrics objects. So we allocate an
+    // oversized autoarray buffer here, so that we're unlikely to overflow
+    // it and need separate heap allocation before the flush happens on the
+    // main thread.
+    AutoTArray<nsFontMetrics*,kMaxCacheEntries*2> mFontMetrics;
+
+    bool mFlushPending = false;
+
+    class FlushFontMetricsTask : public mozilla::Runnable
+    {
+    public:
+        explicit FlushFontMetricsTask(nsFontCache* aCache)
+            : mozilla::Runnable("FlushFontMetricsTask")
+            , mCache(aCache)
+        { }
+        NS_IMETHOD Run() override
+        {
+            // Partially flush the cache, leaving the kMaxCacheEntries/2 most
+            // recent entries.
+            mCache->Flush(mCache->mFontMetrics.Length() - kMaxCacheEntries / 2);
+            mCache->mFlushPending = false;
+            return NS_OK;
+        }
+    private:
+        RefPtr<nsFontCache> mCache;
+    };
 };
 
 NS_IMPL_ISUPPORTS(nsFontCache, nsIObserver)
 
 // The Init and Destroy methods are necessary because it's not
 // safe to call AddObserver from a constructor or RemoveObserver
 // from a destructor.  That should be fixed.
 void
@@ -145,19 +170,26 @@ nsFontCache::GetMetricsFor(const nsFont&
             }
             fm->GetThebesFontGroup()->UpdateUserFonts();
             return do_AddRef(fm);
         }
     }
 
     // It's not in the cache. Get font metrics and then cache them.
     // If the cache has reached its size limit, drop the older half of the
-    // entries.
-    if (n >= kMaxCacheEntries - 1) {
-        Flush(kMaxCacheEntries / 2);
+    // entries; but if we're on a stylo thread (the usual case), we have
+    // to post a task back to the main thread to do the flush.
+    if (n >= kMaxCacheEntries - 1 && !mFlushPending) {
+        if (NS_IsMainThread()) {
+            Flush(mFontMetrics.Length() - kMaxCacheEntries / 2);
+        } else {
+            mFlushPending = true;
+            nsCOMPtr<nsIRunnable> flushTask = new FlushFontMetricsTask(this);
+            MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(flushTask));
+        }
     }
 
     nsFontMetrics::Params params = aParams;
     params.language = language;
     RefPtr<nsFontMetrics> fm = new nsFontMetrics(aFont, params, mContext);
     // the mFontMetrics list has the "head" at the end, because append
     // is cheaper than insert
     mFontMetrics.AppendElement(do_AddRef(fm).take());