Bug 1514869 - patch 8 - Make the SetCharacterMap message async, and use the unshared gfxCharacterMap in the content process until the shared one is in place. r=jwatt,jld
authorJonathan Kew <jkew@mozilla.com>
Mon, 29 Apr 2019 14:39:05 +0000
changeset 471760 7de7d6a0be86d400ee23ca1ac806eb358555b28d
parent 471759 b3fbab6d325ae88011a9589d6398353f5f64377b
child 471761 4ad266f5412e40c438b572e8e414c3a468bb9776
push id35934
push usershindli@mozilla.com
push dateMon, 29 Apr 2019 21:53:38 +0000
treeherdermozilla-central@f6766ba4ac77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt, jld
bugs1514869
milestone68.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 1514869 - patch 8 - Make the SetCharacterMap message async, and use the unshared gfxCharacterMap in the content process until the shared one is in place. r=jwatt,jld Differential Revision: https://phabricator.services.mozilla.com/D24137
dom/ipc/PContent.ipdl
gfx/thebes/SharedFontList.cpp
gfx/thebes/gfxFontEntry.cpp
gfx/thebes/gfxFontEntry.h
ipc/ipdl/sync-messages.ini
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -1244,24 +1244,18 @@ parent:
      *   Font-list shared-memory "pointer" to the Face record to be updated.
      *   A Pointer is a record of a shared-memory block index and an offset
      *   within that block, which each process that maps the block can convert
      *   into a real pointer in its address space.
      * @param aMap
      *   The character coverage map of the face. (This will be stored as a
      *   SharedBitSet record within the shared font list, and the Face record
      *   will be updated to reference it.)
-     *
-     * This is initially a sync message because the content process needs the
-     * Face record to be updated with its character-map reference before it can
-     * proceed with its font-matching work.
-     *
-     * NOTE: a later patch in the series makes this async.
      */
-    sync SetCharacterMap(uint32_t aGeneration, Pointer aFacePtr, gfxSparseBitSet aMap);
+    async SetCharacterMap(uint32_t aGeneration, Pointer aFacePtr, gfxSparseBitSet aMap);
 
     /**
      * Ask the parent to set up the merged charmap for a family, to accelerate
      * future fallback searches.
      * aFamilyPtr may refer to an element of either the Families() or AliasFamilies().
      */
     async SetupFamilyCharMap(uint32_t aGeneration, Pointer aFamilyPtr);
 
--- a/gfx/thebes/SharedFontList.cpp
+++ b/gfx/thebes/SharedFontList.cpp
@@ -318,35 +318,34 @@ void Family::SearchAllFontsForChar(FontL
     MOZ_ASSERT(face->HasValidDescriptor());
     // Get the face's character map, if available (may be null!)
     charmap =
         static_cast<const SharedBitSet*>(face->mCharacterMap.ToPtr(aList));
     if (charmap) {
       ++charMapsLoaded;
     }
     // Check style distance if the char is supported, or if charmap not known
+    // (so that we don't trigger cmap-loading for faces that would be a worse
+    // match than what we've already found).
     if (!charmap || charmap->test(aMatchData->mCh)) {
       double distance = WSSDistance(face, aMatchData->mStyle);
       if (distance < aMatchData->mMatchDistance) {
-        // It's a better style match: if we didn't already check charmap,
-        // load it and do that now
-        if (!charmap) {
-          aList->LoadCharMapFor(*face, this);
-          charmap = static_cast<const SharedBitSet*>(
-              face->mCharacterMap.ToPtr(aList));
-          if (charmap) {
-            ++charMapsLoaded;
-          }
-          if (!charmap || !charmap->test(aMatchData->mCh)) {
-            continue;
-          }
+        // It's a better style match: get a fontEntry, and if we haven't
+        // already checked character coverage, do it now (note that
+        // HasCharacter() will trigger loading the fontEntry's cmap, if
+        // needed).
+        RefPtr<gfxFontEntry> fe = gfxPlatformFontList::PlatformFontList()
+            ->GetOrCreateFontEntry(face, this);
+        if (!fe) {
+          continue;
         }
-        aMatchData->mBestMatch =
-            gfxPlatformFontList::PlatformFontList()->GetOrCreateFontEntry(face,
-                                                                          this);
+        if (!charmap && !fe->HasCharacter(aMatchData->mCh)) {
+          continue;
+        }
+        aMatchData->mBestMatch = fe;
         aMatchData->mMatchDistance = distance;
         aMatchData->mMatchedSharedFamily = this;
       }
     }
   }
   if (mCharacterMap.IsNull() && charMapsLoaded == numFaces) {
     SetupFamilyCharMap(aList);
   }
@@ -600,25 +599,16 @@ Pointer FontList::Alloc(uint32_t aSize) 
     // We've found a block; allocate space from it, and return
     mBlocks[blockIndex]->Allocated() = curAlloc + aSize;
     break;
   }
 
   return Pointer(blockIndex, curAlloc);
 }
 
-void FontList::LoadCharMapFor(Face& aFace, const Family* aFamily) {
-  RefPtr<gfxFontEntry> fe =
-      gfxPlatformFontList::PlatformFontList()->GetOrCreateFontEntry(&aFace,
-                                                                    aFamily);
-  if (fe) {
-    fe->ReadCMAP();
-  }
-}
-
 void FontList::SetFamilyNames(const nsTArray<Family::InitData>& aFamilies) {
   // Only the parent process should ever assign the list of families.
   MOZ_ASSERT(XRE_IsParentProcess());
 
   Header& header = GetHeader();
   MOZ_ASSERT(!header.mFamilyCount);
 
   size_t count = aFamilies.Length();
--- a/gfx/thebes/gfxFontEntry.cpp
+++ b/gfx/thebes/gfxFontEntry.cpp
@@ -135,16 +135,25 @@ gfxFontEntry::~gfxFontEntry() {
 
   // By the time the entry is destroyed, all font instances that were
   // using it should already have been deleted, and so the HB and/or Gr
   // face objects should have been released.
   MOZ_ASSERT(!mHBFace);
   MOZ_ASSERT(!mGrFaceInitialized);
 }
 
+bool gfxFontEntry::TrySetShmemCharacterMap()
+{
+  MOZ_ASSERT(mShmemFace);
+  auto list = gfxPlatformFontList::PlatformFontList()->SharedFontList();
+  mShmemCharacterMap =
+      static_cast<const SharedBitSet*>(mShmemFace->mCharacterMap.ToPtr(list));
+  return mShmemCharacterMap != nullptr;
+}
+
 bool gfxFontEntry::TestCharacterMap(uint32_t aCh) {
   if (!mCharacterMap && !mShmemCharacterMap) {
     ReadCMAP();
     MOZ_ASSERT(mCharacterMap || mShmemCharacterMap,
                "failed to initialize character map");
   }
   return mShmemCharacterMap ? mShmemCharacterMap->test(aCh)
                             : mCharacterMap->test(aCh);
--- a/gfx/thebes/gfxFontEntry.h
+++ b/gfx/thebes/gfxFontEntry.h
@@ -208,18 +208,25 @@ class gfxFontEntry {
     }
     return mHasCmapTable;
   }
 
   inline bool HasCharacter(uint32_t ch) {
     if (mShmemCharacterMap) {
       return mShmemCharacterMap->test(ch);
     }
-    if (mCharacterMap && mCharacterMap->test(ch)) {
-      return true;
+    if (mCharacterMap) {
+      if (mShmemFace && TrySetShmemCharacterMap()) {
+        // Forget our temporary local copy, now we can use the shared cmap
+        mCharacterMap = nullptr;
+        return mShmemCharacterMap->test(ch);
+      }
+      if (mCharacterMap->test(ch)) {
+        return true;
+      }
     }
     return TestCharacterMap(ch);
   }
 
   virtual bool SkipDuringSystemFallback() { return false; }
   nsresult InitializeUVSMap();
   uint16_t GetUVSGlyph(uint32_t aCh, uint32_t aVS);
 
@@ -522,16 +529,21 @@ class gfxFontEntry {
 
   // lookup the cmap in cached font data
   virtual already_AddRefed<gfxCharacterMap> GetCMAPFromFontInfo(
       FontInfoData* aFontInfoData, uint32_t& aUVSOffset);
 
   // helper for HasCharacter(), which is what client code should call
   virtual bool TestCharacterMap(uint32_t aCh);
 
+  // Try to set mShmemCharacterMap, based on the char map in mShmemFace;
+  // return true if successful, false if it remains null (maybe the parent
+  // hasn't handled our SetCharacterMap message yet).
+  bool TrySetShmemCharacterMap();
+
   // Shaper-specific face objects, shared by all instantiations of the same
   // physical font, regardless of size.
   // Usually, only one of these will actually be created for any given font
   // entry, depending on the font tables that are present.
 
   // hb_face_t is refcounted internally, so each shaper that's using it will
   // bump the ref count when it acquires the face, and "destroy" (release) it
   // in its destructor. The font entry has only this non-owning reference to
--- a/ipc/ipdl/sync-messages.ini
+++ b/ipc/ipdl/sync-messages.ini
@@ -885,18 +885,16 @@ description =
 [PContent::GetGraphicsDeviceInitData]
 description =
 [PContent::GetFontListShmBlock]
 description = for bug 1514869 - layout code needs synchronous access to font list, but this is used only once per block, after which content directly reads the shared memory
 [PContent::InitializeFamily]
 description = for bug 1514869 - layout is blocked on needing sync access to a specific font family - used once per family, then the data is cached in shared memory
 [PContent::InitOtherFamilyNames]
 description = for bug 1514869 - layout is blocked on font lookup, needs complete family-name information - not used after loading is complete
-[PContent::SetCharacterMap]
-description = for bug 1514869 - changed to async later in patch series
 [PContent::UngrabPointer]
 description =
 [PContent::RemovePermission]
 description =
 [PContent::GetA11yContentId]
 description =
 [PGMP::StartPlugin]
 description =