Bug 1557962 - Buffer OTS messages and report them once we're back on the main thread. r=jfkthame
authorCameron McCormack <cam@mcc.id.au>
Tue, 11 Jun 2019 15:56:31 +0000
changeset 478251 4df5efc6f719436f1055b9c28e736d9d9a785534
parent 478250 cb05bf1bcc54d28e12587b8cf8d02b4b3b7f4a48
child 478252 7d7a497d0d97b0cade710a11eb6b9161c3e89bfc
push id36138
push userrgurzau@mozilla.com
push dateTue, 11 Jun 2019 21:33:19 +0000
treeherdermozilla-central@ee33e076cbcb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1557962
milestone69.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 1557962 - Buffer OTS messages and report them once we're back on the main thread. r=jfkthame And turn OMT OTS back on. Differential Revision: https://phabricator.services.mozilla.com/D34439
gfx/thebes/gfxUserFontSet.cpp
gfx/thebes/gfxUserFontSet.h
layout/style/FontFaceSet.cpp
modules/libpref/init/StaticPrefList.h
--- a/gfx/thebes/gfxUserFontSet.cpp
+++ b/gfx/thebes/gfxUserFontSet.cpp
@@ -178,26 +178,29 @@ gfxFont* gfxUserFontEntry::CreateFontIns
       " with an actual platform font entry");
 
   // userfont entry is a container, can't create font from the container
   return nullptr;
 }
 
 class MOZ_STACK_CLASS gfxOTSContext : public ots::OTSContext {
  public:
-  explicit gfxOTSContext(gfxUserFontEntry* aUserFontEntry)
-      : mUserFontEntry(aUserFontEntry) {
+  gfxOTSContext() {
     // Whether to apply OTS validation to OpenType Layout tables
     mCheckOTLTables = StaticPrefs::ValidateOTLTables();
     // Whether to preserve Variation tables in downloaded fonts
     mCheckVariationTables = StaticPrefs::ValidateVariationTables();
     // Whether to preserve color bitmap glyphs
     mKeepColorBitmaps = StaticPrefs::KeepColorBitmaps();
   }
 
+  virtual ~gfxOTSContext() {
+    MOZ_ASSERT(mMessages.IsEmpty(), "should have called TakeMessages");
+  }
+
   virtual ots::TableAction GetTableAction(uint32_t aTag) override {
     // Preserve Graphite, color glyph and SVG tables,
     // and possibly OTL and Variation tables (depending on prefs)
     if ((!mCheckOTLTables && (aTag == TRUETYPE_TAG('G', 'D', 'E', 'F') ||
                               aTag == TRUETYPE_TAG('G', 'P', 'O', 'S') ||
                               aTag == TRUETYPE_TAG('G', 'S', 'U', 'B'))) ||
         (!mCheckVariationTables &&
          (aTag == TRUETYPE_TAG('a', 'v', 'a', 'r') ||
@@ -233,32 +236,41 @@ class MOZ_STACK_CLASS gfxOTSContext : pu
       // For warnings (rather than errors that cause the font to fail),
       // we only report the first instance of any given message.
       if (mWarningsIssued.Contains(msg)) {
         return;
       }
       mWarningsIssued.PutEntry(msg);
     }
 
-    mUserFontEntry->mFontSet->LogMessage(mUserFontEntry, msg.get());
+    mMessages.AppendElement(msg);
   }
 
+  bool Process(ots::OTSStream* aOutput, const uint8_t* aInput, size_t aLength,
+               nsTArray<nsCString>& aMessages) {
+    bool ok = ots::OTSContext::Process(aOutput, aInput, aLength);
+    aMessages = TakeMessages();
+    return ok;
+  }
+
+  nsTArray<nsCString>&& TakeMessages() { return std::move(mMessages); }
+
  private:
-  gfxUserFontEntry* mUserFontEntry;
   nsTHashtable<nsCStringHashKey> mWarningsIssued;
+  nsTArray<nsCString> mMessages;
   bool mCheckOTLTables;
   bool mCheckVariationTables;
   bool mKeepColorBitmaps;
 };
 
 // Call the OTS library to sanitize an sfnt before attempting to use it.
 // Returns a newly-allocated block, or nullptr in case of fatal errors.
 const uint8_t* gfxUserFontEntry::SanitizeOpenTypeData(
     const uint8_t* aData, uint32_t aLength, uint32_t& aSaneLength,
-    gfxUserFontType& aFontType) {
+    gfxUserFontType& aFontType, nsTArray<nsCString>& aMessages) {
   aFontType = gfxFontUtils::DetermineFontDataType(aData, aLength);
   Telemetry::Accumulate(Telemetry::WEBFONT_FONTTYPE, uint32_t(aFontType));
 
   if (aFontType == GFX_USERFONT_UNKNOWN) {
     aSaneLength = 0;
     return nullptr;
   }
 
@@ -267,18 +279,18 @@ const uint8_t* gfxUserFontEntry::Sanitiz
     lengthHint *= 2;
   } else if (aFontType == GFX_USERFONT_WOFF2) {
     lengthHint *= 3;
   }
 
   // limit output/expansion to 256MB
   ExpandingMemoryStream output(lengthHint, 1024 * 1024 * 256);
 
-  gfxOTSContext otsContext(this);
-  if (!otsContext.Process(&output, aData, aLength)) {
+  gfxOTSContext otsContext;
+  if (!otsContext.Process(&output, aData, aLength, aMessages)) {
     // Failed to decode/sanitize the font, so discard it.
     aSaneLength = 0;
     return nullptr;
   }
 
   aSaneLength = output.Tell();
   return static_cast<const uint8_t*>(output.forget());
 }
@@ -677,50 +689,58 @@ bool gfxUserFontEntry::LoadPlatformFontS
 
   // Unwrap/decompress/sanitize or otherwise munge the downloaded data
   // to make a usable sfnt structure.
 
   // Call the OTS sanitizer; this will also decode WOFF to sfnt
   // if necessary. The original data in aFontData is left unchanged.
   uint32_t saneLen;
   gfxUserFontType fontType;
+  nsTArray<nsCString> messages;
   const uint8_t* saneData =
-      SanitizeOpenTypeData(aFontData, aLength, saneLen, fontType);
+      SanitizeOpenTypeData(aFontData, aLength, saneLen, fontType, messages);
 
-  return LoadPlatformFont(aFontData, aLength, fontType, saneData, saneLen);
+  return LoadPlatformFont(aFontData, aLength, fontType, saneData, saneLen,
+                          std::move(messages));
 }
 
 void gfxUserFontEntry::StartPlatformFontLoadOnWorkerThread(
     const uint8_t* aFontData, uint32_t aLength,
     nsMainThreadPtrHandle<nsIFontLoadCompleteCallback> aCallback) {
   MOZ_ASSERT(sFontLoadingThread);
   MOZ_ASSERT(sFontLoadingThread->IsOnCurrentThread());
 
   uint32_t saneLen;
   gfxUserFontType fontType;
+  nsTArray<nsCString> messages;
   const uint8_t* saneData =
-      SanitizeOpenTypeData(aFontData, aLength, saneLen, fontType);
+      SanitizeOpenTypeData(aFontData, aLength, saneLen, fontType, messages);
 
   nsCOMPtr<nsIRunnable> event =
       NewRunnableMethod<const uint8_t*, uint32_t, gfxUserFontType,
-                        const uint8_t*, uint32_t,
+                        const uint8_t*, uint32_t, nsTArray<nsCString>&&,
                         nsMainThreadPtrHandle<nsIFontLoadCompleteCallback>>(
           "gfxUserFontEntry::ContinuePlatformFontLoadOnMainThread", this,
           &gfxUserFontEntry::ContinuePlatformFontLoadOnMainThread, aFontData,
-          aLength, fontType, saneData, saneLen, aCallback);
+          aLength, fontType, saneData, saneLen, std::move(messages), aCallback);
   NS_DispatchToMainThread(event.forget());
 }
 
 bool gfxUserFontEntry::LoadPlatformFont(const uint8_t* aOriginalFontData,
                                         uint32_t aOriginalLength,
                                         gfxUserFontType aFontType,
                                         const uint8_t* aSanitizedFontData,
-                                        uint32_t aSanitizedLength) {
+                                        uint32_t aSanitizedLength,
+                                        nsTArray<nsCString>&& aMessages) {
   MOZ_ASSERT(NS_IsMainThread());
 
+  for (const auto& msg : aMessages) {
+    mFontSet->LogMessage(this, msg.get());
+  }
+
   if (!aSanitizedFontData) {
     mFontSet->LogMessage(this, "rejected by sanitizer");
   } else {
     // Check whether aSanitizedFontData is a known OpenType format; it might be
     // a TrueType Collection, which OTS would accept but we don't yet
     // know how to handle. If so, discard.
     if (gfxFontUtils::DetermineFontDataType(
             aSanitizedFontData, aSanitizedLength) != GFX_USERFONT_OPENTYPE) {
@@ -914,22 +934,23 @@ void gfxUserFontEntry::LoadPlatformFontA
           aLength, cb);
   MOZ_ALWAYS_SUCCEEDS(
       sFontLoadingThread->Dispatch(event.forget(), NS_DISPATCH_NORMAL));
 }
 
 void gfxUserFontEntry::ContinuePlatformFontLoadOnMainThread(
     const uint8_t* aOriginalFontData, uint32_t aOriginalLength,
     gfxUserFontType aFontType, const uint8_t* aSanitizedFontData,
-    uint32_t aSanitizedLength,
+    uint32_t aSanitizedLength, nsTArray<nsCString>&& aMessages,
     nsMainThreadPtrHandle<nsIFontLoadCompleteCallback> aCallback) {
   MOZ_ASSERT(NS_IsMainThread());
 
   bool loaded = LoadPlatformFont(aOriginalFontData, aOriginalLength, aFontType,
-                                 aSanitizedFontData, aSanitizedLength);
+                                 aSanitizedFontData, aSanitizedLength,
+                                 std::move(aMessages));
   aOriginalFontData = nullptr;
   aSanitizedFontData = nullptr;
 
   if (loaded) {
     IncrementGeneration();
     aCallback->FontLoadComplete();
     return;
   }
--- a/gfx/thebes/gfxUserFontSet.h
+++ b/gfx/thebes/gfxUserFontSet.h
@@ -639,17 +639,18 @@ class gfxUserFontEntry : public gfxFontE
     MOZ_ASSERT_UNREACHABLE("not meaningful for a userfont placeholder");
   }
 
   static void Shutdown() { sFontLoadingThread = nullptr; }
 
  protected:
   const uint8_t* SanitizeOpenTypeData(const uint8_t* aData, uint32_t aLength,
                                       uint32_t& aSaneLength,
-                                      gfxUserFontType& aFontType);
+                                      gfxUserFontType& aFontType,
+                                      nsTArray<nsCString>& aMessages);
 
   // 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);
@@ -675,25 +676,26 @@ class gfxUserFontEntry : public gfxFontE
   void StartPlatformFontLoadOnWorkerThread(
       const uint8_t* aFontData, uint32_t aLength,
       nsMainThreadPtrHandle<nsIFontLoadCompleteCallback> aCallback);
 
   // helper method for LoadPlatformFontAsync; runs on the main thread
   void ContinuePlatformFontLoadOnMainThread(
       const uint8_t* aOriginalFontData, uint32_t aOriginalLength,
       gfxUserFontType aFontType, const uint8_t* aSanitizedFontData,
-      uint32_t aSanitizedLength,
+      uint32_t aSanitizedLength, nsTArray<nsCString>&& aMessages,
       nsMainThreadPtrHandle<nsIFontLoadCompleteCallback> aCallback);
 
   // helper method for LoadPlatformFontSync and
   // ContinuePlatformFontLoadOnMainThread; runs on the main thread
   bool LoadPlatformFont(const uint8_t* aOriginalFontData,
                         uint32_t aOriginalLength, gfxUserFontType aFontType,
                         const uint8_t* aSanitizedFontData,
-                        uint32_t aSanitizedLength);
+                        uint32_t aSanitizedLength,
+                        nsTArray<nsCString>&& aMessages);
 
   // helper method for FontDataDownloadComplete and
   // ContinuePlatformFontLoadOnMainThread; runs on the main thread
   void FontLoadFailed(nsIFontLoadCompleteCallback* aCallback);
 
   // store metadata and src details for current src into aFontEntry
   void StoreUserFontData(gfxFontEntry* aFontEntry, bool aPrivate,
                          const nsACString& aOriginalName,
--- a/layout/style/FontFaceSet.cpp
+++ b/layout/style/FontFaceSet.cpp
@@ -1196,16 +1196,20 @@ RawServoFontFaceRule* FontFaceSet::FindR
     }
   }
   return nullptr;
 }
 
 nsresult FontFaceSet::LogMessage(gfxUserFontEntry* aUserFontEntry,
                                  const char* aMessage, uint32_t aFlags,
                                  nsresult aStatus) {
+  MOZ_ASSERT(NS_IsMainThread(),
+             "LogMessage only works on the main thread, due to the Servo_XXX "
+             "CSSOM calls it makes");
+
   nsCOMPtr<nsIConsoleService> console(
       do_GetService(NS_CONSOLESERVICE_CONTRACTID));
   if (!console) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsAutoCString familyName;
   nsAutoCString fontURI;
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -2715,17 +2715,17 @@ VARCACHE_PREF(
   RelaxedAtomicBool, false
 )
 
 // Whether font sanitization is performed on the main thread or not.
 VARCACHE_PREF(
   Live,
   "gfx.downloadable_fonts.sanitize_omt",
   gfx_downloadable_fonts_sanitize_omt,
-  RelaxedAtomicBool, false
+  RelaxedAtomicBool, true
 )
 
 VARCACHE_PREF(
   Live,
   "gfx.downloadable_fonts.validate_variation_tables",
   ValidateVariationTables,
   RelaxedAtomicBool, true
 )