Bug 1356103 - Part 9: Use a PostTraversalTask to deal with downloadable fonts in gfxUserFontSet. r=bholley,jfkthame
☠☠ backed out by 8f8cb62a8749 ☠ ☠
authorCameron McCormack <cam@mcc.id.au>
Sun, 30 Apr 2017 14:57:25 +0800
changeset 573113 2f383d89184b02533e9afbfa4d614db064e0b2c6
parent 573112 a03112e1c9d5597e91b48d94a49d62890a33d298
child 573114 7bc3a4861a3995638bc2dada3cc8f2257b887342
push id57306
push userbmo:emilio+bugs@crisal.io
push dateFri, 05 May 2017 10:08:55 +0000
reviewersbholley, jfkthame
bugs1356103
milestone55.0a1
Bug 1356103 - Part 9: Use a PostTraversalTask to deal with downloadable fonts in gfxUserFontSet. r=bholley,jfkthame Here we add a new UserFontLoadState value, STATUS_LOAD_PENDING, which represents the state just after a gfxUserFontEntry's url()-valued source would being loading, except that we can't start the load due to being on a Servo style worker thread. In that case, we defer the work of initiating the load until just after the Servo traversal is finished. URLs that can normally be loaded synchronously, such as data: URLs and script-implemented protocols marked as synchronous, must be handled asynchronously when encountered during Servo traversal, since various main-thread only work (in FontFaceSet::SyncLoadFontData) must happen. This is a user visible change from stock Gecko, but should only happen when font metrics for a data: URL font are requested due to ch/ex unit resolution when layout hasn't previously requested the font load. Hopefully nobody relies on synchronous resolution of ch/ex units with data: URLs. We unfortunately also can't pick gfxUserFontEntry objects out of the UserFontCache during Servo traversal, since validating the cache entry involves doing content policy checking, which is not thread-safe (due in part to taking strong references to nsIPrincipals). Platform fonts and ArrayBuffer-backed DOM FontFace objects continue to be handled synchronously. The PostTraversalTask does not take a strong reference to the gfxUserFontEntry object, since it is held on to by the DOM FontFace object, which itself won't go away before the PostTraversalTask is run. MozReview-Commit-ID: J9ODLsusrNV
gfx/thebes/gfxTextRun.cpp
gfx/thebes/gfxUserFontSet.cpp
gfx/thebes/gfxUserFontSet.h
layout/style/FontFace.cpp
layout/style/PostTraversalTask.cpp
layout/style/PostTraversalTask.h
--- a/gfx/thebes/gfxTextRun.cpp
+++ b/gfx/thebes/gfxTextRun.cpp
@@ -1952,16 +1952,17 @@ gfxFontGroup::GetFontAt(int32_t i, uint3
 void
 gfxFontGroup::FamilyFace::CheckState(bool& aSkipDrawing)
 {
     gfxFontEntry* fe = FontEntry();
     if (fe->mIsUserFontContainer) {
         gfxUserFontEntry* ufe = static_cast<gfxUserFontEntry*>(fe);
         gfxUserFontEntry::UserFontLoadState state = ufe->LoadState();
         switch (state) {
+            case gfxUserFontEntry::STATUS_LOAD_PENDING:
             case gfxUserFontEntry::STATUS_LOADING:
                 SetLoading(true);
                 break;
             case gfxUserFontEntry::STATUS_FAILED:
                 SetInvalid();
                 // fall-thru to the default case
                 MOZ_FALLTHROUGH;
             default:
--- a/gfx/thebes/gfxUserFontSet.cpp
+++ b/gfx/thebes/gfxUserFontSet.cpp
@@ -16,16 +16,18 @@
 #include "nsIPrincipal.h"
 #include "nsIZipReader.h"
 #include "gfxFontConstants.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/Services.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/gfx/2D.h"
 #include "gfxPlatformFontList.h"
+#include "mozilla/ServoStyleSet.h"
+#include "mozilla/PostTraversalTask.h"
 
 #include "opentype-sanitiser.h"
 #include "ots-memory-stream.h"
 
 using namespace mozilla;
 
 mozilla::LogModule*
 gfxUserFontSet::GetUserFontsLog()
@@ -136,16 +138,20 @@ gfxUserFontEntry::gfxUserFontEntry(gfxUs
 
     if (aUnicodeRanges) {
         mCharacterMap = new gfxCharacterMap(*aUnicodeRanges);
     }
 }
 
 gfxUserFontEntry::~gfxUserFontEntry()
 {
+    // Assert that we don't drop any gfxUserFontEntry objects during a Servo
+    // traversal, since PostTraversalTask objects can hold raw pointers to
+    // gfxUserFontEntry objects.
+    MOZ_ASSERT(!ServoStyleSet::IsInServoTraversal());
 }
 
 bool
 gfxUserFontEntry::Matches(const nsTArray<gfxFontFaceSrc>& aFontFaceSrcList,
                           uint32_t aWeight,
                           int32_t aStretch,
                           uint8_t aStyle,
                           const nsTArray<gfxFontFeature>& aFeatureSettings,
@@ -424,36 +430,52 @@ CopyWOFFMetadata(const uint8_t* aFontDat
     }
     memcpy(aMetadata->Elements(), aFontData + metaOffset, metaCompLen);
     *aMetaOrigLen = woff->metaOrigLen;
 }
 
 void
 gfxUserFontEntry::LoadNextSrc()
 {
-    uint32_t numSrc = mSrcList.Length();
-
-    NS_ASSERTION(mSrcIndex < numSrc,
+    NS_ASSERTION(mSrcIndex < mSrcList.Length(),
                  "already at the end of the src list for user font");
     NS_ASSERTION((mUserFontLoadState == STATUS_NOT_LOADED ||
+                  mUserFontLoadState == STATUS_LOAD_PENDING ||
                   mUserFontLoadState == STATUS_LOADING) &&
                  mFontDataLoadingState < LOADING_FAILED,
                  "attempting to load a font that has either completed or failed");
 
     if (mUserFontLoadState == STATUS_NOT_LOADED) {
         SetLoadState(STATUS_LOADING);
         mFontDataLoadingState = LOADING_STARTED;
         mUnsupportedFormat = false;
     } else {
         // we were already loading; move to the next source,
         // but don't reset state - if we've already timed out,
         // that counts against the new download
         mSrcIndex++;
     }
 
+    DoLoadNextSrc(false);
+}
+
+void
+gfxUserFontEntry::ContinueLoad()
+{
+    MOZ_ASSERT(mUserFontLoadState == STATUS_LOAD_PENDING);
+    MOZ_ASSERT(mSrcList[mSrcIndex].mSourceType == gfxFontFaceSrc::eSourceType_URL);
+
+    DoLoadNextSrc(true);
+}
+
+void
+gfxUserFontEntry::DoLoadNextSrc(bool aForceAsync)
+{
+    uint32_t numSrc = mSrcList.Length();
+
     // load each src entry in turn, until a local face is found
     // or a download begins successfully
     while (mSrcIndex < numSrc) {
         gfxFontFaceSrc& currSrc = mSrcList[mSrcIndex];
 
         // src local ==> lookup and load immediately
 
         if (currSrc.mSourceType == gfxFontFaceSrc::eSourceType_Local) {
@@ -499,16 +521,22 @@ gfxUserFontEntry::LoadNextSrc()
             }
         }
 
         // src url ==> start the load process
         else if (currSrc.mSourceType == gfxFontFaceSrc::eSourceType_URL) {
             if (gfxPlatform::GetPlatform()->IsFontFormatSupported(currSrc.mURI,
                     currSrc.mFormatFlags)) {
 
+                if (ServoStyleSet* set = ServoStyleSet::Current()) {
+                    set->AppendTask(PostTraversalTask::LoadFontEntry(this));
+                    SetLoadState(STATUS_LOAD_PENDING);
+                    return;
+                }
+
                 nsIPrincipal* principal = nullptr;
                 bool bypassCache;
                 nsresult rv = mFontSet->CheckFontLoad(&currSrc, &principal,
                                                       &bypassCache);
 
                 if (NS_SUCCEEDED(rv) && principal != nullptr) {
                     if (!bypassCache) {
                         // see if we have an existing entry for this source
@@ -532,19 +560,21 @@ gfxUserFontEntry::LoadNextSrc()
                     }
 
                     // record the principal returned by CheckFontLoad,
                     // for use when creating a channel
                     // and when caching the loaded entry
                     mPrincipal = principal;
 
                     bool loadDoesntSpin = false;
-                    rv = NS_URIChainHasFlags(currSrc.mURI,
-                           nsIProtocolHandler::URI_SYNC_LOAD_IS_OK,
-                           &loadDoesntSpin);
+                    if (!aForceAsync) {
+                        rv = NS_URIChainHasFlags(currSrc.mURI,
+                               nsIProtocolHandler::URI_SYNC_LOAD_IS_OK,
+                               &loadDoesntSpin);
+                    }
 
                     if (NS_SUCCEEDED(rv) && loadDoesntSpin) {
                         uint8_t* buffer = nullptr;
                         uint32_t bufferLength = 0;
 
                         // sync load font immediately
                         rv = mFontSet->SyncLoadFontData(this, &currSrc, buffer,
                                                         bufferLength);
@@ -639,16 +669,17 @@ gfxUserFontEntry::SetLoadState(UserFontL
 }
 
 MOZ_DEFINE_MALLOC_SIZE_OF_ON_ALLOC(UserFontMallocSizeOfOnAlloc)
 
 bool
 gfxUserFontEntry::LoadPlatformFont(const uint8_t* aFontData, uint32_t& aLength)
 {
     NS_ASSERTION((mUserFontLoadState == STATUS_NOT_LOADED ||
+                  mUserFontLoadState == STATUS_LOAD_PENDING ||
                   mUserFontLoadState == STATUS_LOADING) &&
                  mFontDataLoadingState < LOADING_FAILED,
                  "attempting to load a font that has either completed or failed");
 
     gfxFontEntry* fe = nullptr;
 
     gfxUserFontType fontType =
         gfxFontUtils::DetermineFontDataType(aFontData, aLength);
--- a/gfx/thebes/gfxUserFontSet.h
+++ b/gfx/thebes/gfxUserFontSet.h
@@ -12,16 +12,19 @@
 #include "nsCOMPtr.h"
 #include "nsIURI.h"
 #include "nsIPrincipal.h"
 #include "nsIScriptError.h"
 #include "nsURIHashKey.h"
 #include "mozilla/net/ReferrerPolicy.h"
 #include "gfxFontConstants.h"
 
+namespace mozilla {
+class PostTraversalTask;
+} // namespace mozilla
 class nsFontFaceLoader;
 
 //#define DEBUG_USERFONT_CACHE
 
 class gfxFontFaceBufferSource
 {
   NS_INLINE_DECL_REFCOUNTING(gfxFontFaceBufferSource)
 public:
@@ -543,24 +546,26 @@ protected:
     // performance stats
     uint32_t mDownloadCount;
     uint64_t mDownloadSize;
 };
 
 // acts a placeholder until the real font is downloaded
 
 class gfxUserFontEntry : public gfxFontEntry {
+    friend class mozilla::PostTraversalTask;
     friend class gfxUserFontSet;
     friend class nsUserFontSet;
     friend class nsFontFaceLoader;
     friend class gfxOTSContext;
 
 public:
     enum UserFontLoadState {
         STATUS_NOT_LOADED = 0,
+        STATUS_LOAD_PENDING,
         STATUS_LOADING,
         STATUS_LOADED,
         STATUS_FAILED
     };
 
     gfxUserFontEntry(gfxUserFontSet* aFontSet,
                      const nsTArray<gfxFontFaceSrc>& aFontFaceSrcList,
                      uint32_t aWeight,
@@ -588,17 +593,18 @@ public:
 
     gfxFontEntry* GetPlatformFontEntry() const { return mPlatformFontEntry; }
 
     // is the font loading or loaded, or did it fail?
     UserFontLoadState LoadState() const { return mUserFontLoadState; }
 
     // whether to wait before using fallback font or not
     bool WaitForUserFont() const {
-        return mUserFontLoadState == STATUS_LOADING &&
+        return (mUserFontLoadState == STATUS_LOAD_PENDING ||
+                mUserFontLoadState == STATUS_LOADING) &&
                mFontDataLoadingState < LOADING_SLOWLY;
     }
 
     // for userfonts, cmap is used to store the unicode range data
     // no cmap ==> all codepoints permitted
     bool CharacterInUnicodeRange(uint32_t ch) const {
         if (mCharacterMap) {
             return mCharacterMap->test(ch);
@@ -628,16 +634,18 @@ public:
 protected:
     const uint8_t* SanitizeOpenTypeData(const uint8_t* aData,
                                         uint32_t aLength,
                                         uint32_t& aSaneLength,
                                         gfxUserFontType aFontType);
 
     // attempt to load the next resource in the src list.
     void LoadNextSrc();
+    void ContinueLoad();
+    void DoLoadNextSrc(bool aForceAsync);
 
     // change the load state
     virtual void SetLoadState(UserFontLoadState aLoadState);
 
     // when download has been completed, pass back data here
     // aDownloadStatus == NS_OK ==> download succeeded, error otherwise
     // returns true if platform font creation sucessful (or local()
     // reference was next in line)
--- a/layout/style/FontFace.cpp
+++ b/layout/style/FontFace.cpp
@@ -132,16 +132,17 @@ FontFace::WrapObject(JSContext* aCx, JS:
 }
 
 static FontFaceLoadStatus
 LoadStateToStatus(gfxUserFontEntry::UserFontLoadState aLoadState)
 {
   switch (aLoadState) {
     case gfxUserFontEntry::UserFontLoadState::STATUS_NOT_LOADED:
       return FontFaceLoadStatus::Unloaded;
+    case gfxUserFontEntry::UserFontLoadState::STATUS_LOAD_PENDING:
     case gfxUserFontEntry::UserFontLoadState::STATUS_LOADING:
       return FontFaceLoadStatus::Loading;
     case gfxUserFontEntry::UserFontLoadState::STATUS_LOADED:
       return FontFaceLoadStatus::Loaded;
     case gfxUserFontEntry::UserFontLoadState::STATUS_FAILED:
       return FontFaceLoadStatus::Error;
   }
   NS_NOTREACHED("invalid aLoadState value");
--- a/layout/style/PostTraversalTask.cpp
+++ b/layout/style/PostTraversalTask.cpp
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "PostTraversalTask.h"
 
 #include "mozilla/dom/FontFace.h"
 #include "mozilla/dom/FontFaceSet.h"
+#include "gfxUserFontSet.h"
 
 namespace mozilla {
 
 using namespace dom;
 
 void
 PostTraversalTask::Run()
 {
@@ -29,12 +30,16 @@ PostTraversalTask::Run()
       static_cast<FontFaceSet*>(mTarget)->
         DispatchLoadingEventAndReplaceReadyPromise();
       break;
 
     case Type::DispatchFontFaceSetCheckLoadingFinishedAfterDelay:
       static_cast<FontFaceSet*>(mTarget)->
         DispatchCheckLoadingFinishedAfterDelay();
       break;
+
+    case Type::LoadFontEntry:
+      static_cast<gfxUserFontEntry*>(mTarget)->ContinueLoad();
+      break;
   }
 }
 
 } // namespace mozilla
--- a/layout/style/PostTraversalTask.h
+++ b/layout/style/PostTraversalTask.h
@@ -10,16 +10,17 @@
 /* a task to be performed immediately after a Servo traversal */
 
 namespace mozilla {
 namespace dom {
 class FontFace;
 class FontFaceSet;
 } // namespace dom
 } // namespace mozilla
+class gfxUserFontEntry;
 
 namespace mozilla {
 
 /**
  * A PostTraversalTask is a task to be performed immediately after a Servo
  * traversal.  There are just a few tasks we need to perform, so we use this
  * class rather than Runnables, to avoid virtual calls and some allocations.
  *
@@ -58,16 +59,23 @@ public:
     dom::FontFaceSet* aFontFaceSet)
   {
     auto task =
       PostTraversalTask(Type::DispatchFontFaceSetCheckLoadingFinishedAfterDelay);
     task.mTarget = aFontFaceSet;
     return task;
   }
 
+  static PostTraversalTask LoadFontEntry(gfxUserFontEntry* aFontEntry)
+  {
+    auto task = PostTraversalTask(Type::LoadFontEntry);
+    task.mTarget = aFontEntry;
+    return task;
+  }
+
   void Run();
 
 private:
   // For any new raw pointer type that we need to store in a PostTraversalTask,
   // please add an assertion that class' destructor that we are not in a Servo
   // traversal, to protect against the possibility of having dangling pointers.
   enum class Type
   {
@@ -78,16 +86,19 @@ private:
     // mResult
     RejectFontFaceLoadedPromise,
 
     // mTarget (FontFaceSet*)
     DispatchLoadingEventAndReplaceReadyPromise,
 
     // mTarget (FontFaceSet*)
     DispatchFontFaceSetCheckLoadingFinishedAfterDelay,
+
+    // mTarget (gfxUserFontEntry*)
+    LoadFontEntry,
   };
 
   PostTraversalTask(Type aType)
     : mType(aType)
     , mTarget(nullptr)
     , mResult(NS_OK)
   {
   }