Bug 1412545 - avoid race condition when setting Cairo ft font user data. r=jrmuizel
authorLee Salzman <lsalzman@mozilla.com>
Tue, 07 Nov 2017 14:10:31 -0500
changeset 443853 b80f6e02f7571660442822b2c8800a6031a609f7
parent 443852 af56f51a55b1492f2fc22b2bf3a61cf69cc89d57
child 443854 1c6eac3c74d516b2e76ebee8b07a663341133fda
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1412545
milestone58.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 1412545 - avoid race condition when setting Cairo ft font user data. r=jrmuizel MozReview-Commit-ID: 4aqfj2xlCc2
gfx/2d/ScaledFontFontconfig.cpp
gfx/webrender_bindings/Moz2DImageRenderer.cpp
gfx/webrender_bindings/moz.build
--- a/gfx/2d/ScaledFontFontconfig.cpp
+++ b/gfx/2d/ScaledFontFontconfig.cpp
@@ -394,18 +394,19 @@ ScaledFontFontconfig::CreateFromInstance
                                              Float aSize,
                                              NativeFontResource* aNativeFontResource)
 {
   FcPattern* pattern = FcPatternCreate();
   if (!pattern) {
     gfxWarning() << "Failing initializing Fontconfig pattern for scaled font";
     return nullptr;
   }
-  if (aUnscaledFont->GetFace()) {
-    FcPatternAddFTFace(pattern, FC_FT_FACE, aUnscaledFont->GetFace());
+  FT_Face face = aUnscaledFont->GetFace();
+  if (face) {
+    FcPatternAddFTFace(pattern, FC_FT_FACE, face);
   } else {
     FcPatternAddString(pattern, FC_FILE, reinterpret_cast<const FcChar8*>(aUnscaledFont->GetFile()));
     FcPatternAddInteger(pattern, FC_INDEX, aUnscaledFont->GetIndex());
   }
   FcPatternAddDouble(pattern, FC_PIXEL_SIZE, aSize);
   aInstanceData.SetupPattern(pattern);
 
   cairo_font_face_t* font = cairo_ft_font_face_create_for_pattern(pattern);
@@ -415,20 +416,27 @@ ScaledFontFontconfig::CreateFromInstance
     return nullptr;
   }
 
   if (aNativeFontResource) {
     // Bug 1362117 - Cairo may keep the font face alive after the owning NativeFontResource
     // was freed. To prevent this, we must bind the NativeFontResource to the font face so that
     // it stays alive at least as long as the font face.
     aNativeFontResource->AddRef();
-    if (cairo_font_face_set_user_data(font,
-                                      &sNativeFontResourceKey,
-                                      aNativeFontResource,
-                                      ReleaseNativeFontResource) != CAIRO_STATUS_SUCCESS) {
+    // Bug 1412545 - Setting Cairo font user data is not thread-safe. If Fontconfig patterns match,
+    // cairo_ft_font_face_create_for_pattern may share Cairo faces. We need to lock setting user data
+    // to prevent races if multiple threads are thus sharing the same Cairo face.
+    FT_Library library = face ? face->glyph->library : Factory::GetFTLibrary();
+    Factory::LockFTLibrary(library);
+    cairo_status_t err = cairo_font_face_set_user_data(font,
+                                                       &sNativeFontResourceKey,
+                                                       aNativeFontResource,
+                                                       ReleaseNativeFontResource);
+    Factory::UnlockFTLibrary(library);
+    if (err != CAIRO_STATUS_SUCCESS) {
       gfxWarning() << "Failed binding NativeFontResource to Cairo font face";
       aNativeFontResource->Release();
       cairo_font_face_destroy(font);
       FcPatternDestroy(pattern);
       return nullptr;
     }
   }
 
--- a/gfx/webrender_bindings/Moz2DImageRenderer.cpp
+++ b/gfx/webrender_bindings/Moz2DImageRenderer.cpp
@@ -14,18 +14,17 @@
 
 #include <iostream>
 #include <unordered_map>
 
 #ifdef XP_MACOSX
 #include "mozilla/gfx/UnscaledFontMac.h"
 #elif defined(XP_WIN)
 #include "mozilla/gfx/UnscaledFontDWrite.h"
-#elif defined(MOZ_ENABLE_FREETYPE)
-#include "mozilla/ThreadLocal.h"
+#else
 #include "mozilla/gfx/UnscaledFontFreeType.h"
 #endif
 
 namespace std {
   template <>
     struct hash<mozilla::wr::FontKey>{
       public :
         size_t operator()(const mozilla::wr::FontKey &key ) const
@@ -38,29 +37,24 @@ namespace std {
 
 
 namespace mozilla {
 
 using namespace gfx;
 
 namespace wr {
 
-#ifdef MOZ_ENABLE_FREETYPE
-static MOZ_THREAD_LOCAL(FT_Library) sFTLibrary;
-#endif
-
 struct FontTemplate {
   const uint8_t *mData;
   size_t mSize;
   uint32_t mIndex;
   const VecU8 *mVec;
   RefPtr<UnscaledFont> mUnscaledFont;
 };
 
-// we need to do special things for linux so that we have fonts per backend
 std::unordered_map<FontKey, FontTemplate> sFontDataTable;
 
 extern "C" {
 void
 AddFontData(WrFontKey aKey, const uint8_t *aData, size_t aSize, uint32_t aIndex, const ArcVecU8 *aVec) {
   auto i = sFontDataTable.find(aKey);
   if (i == sFontDataTable.end()) {
     FontTemplate font;
@@ -152,31 +146,16 @@ static bool Moz2DRenderCallback(const Ra
   }
 
   auto stride = aSize.width * gfx::BytesPerPixel(aFormat);
 
   if (aOutput.length() < static_cast<size_t>(aSize.height * stride)) {
     return false;
   }
 
-  void* fontContext = nullptr;
-#ifdef MOZ_ENABLE_FREETYPE
-  if (!sFTLibrary.init()) {
-    return false;
-  }
-  if (!sFTLibrary.get()) {
-    FT_Library library = gfx::Factory::NewFTLibrary();
-    if (!library) {
-      return false;
-    }
-    sFTLibrary.set(library);
-  }
-  fontContext = sFTLibrary.get();
-#endif
-
   // In bindings.rs we allocate a buffer filled with opaque white.
   bool uninitialized = false;
 
   RefPtr<gfx::DrawTarget> dt = gfx::Factory::CreateDrawTargetForData(
     gfx::BackendType::SKIA,
     aOutput.begin().get(),
     aSize,
     stride,
@@ -220,17 +199,17 @@ static bool Moz2DRenderCallback(const Ra
   Reader reader(aBlob.begin().get()+indexOffset, aBlob.length()-sizeof(size_t)-indexOffset);
 
   bool ret;
   size_t offset = 0;
   while (reader.pos < reader.len) {
     size_t end = reader.ReadSize();
     size_t extra_end = reader.ReadSize();
 
-    gfx::InlineTranslator translator(dt, fontContext);
+    gfx::InlineTranslator translator(dt);
 
     size_t count = *(size_t*)(aBlob.begin().get() + end);
     for (size_t i = 0; i < count; i++) {
       wr::FontKey key = *(wr::FontKey*)(aBlob.begin() + end + sizeof(count) + sizeof(wr::FontKey)*i).get();
       RefPtr<UnscaledFont> font = GetUnscaledFont(&translator, key);
       translator.AddUnscaledFont(0, font);
     }
     Range<const uint8_t> blob(aBlob.begin() + offset, aBlob.begin() + end);
--- a/gfx/webrender_bindings/moz.build
+++ b/gfx/webrender_bindings/moz.build
@@ -44,15 +44,14 @@ if CONFIG['MOZ_ENABLE_D3D10_LAYER']:
     EXPORTS.mozilla.webrender += [
         'RenderD3D11TextureHostOGL.h',
     ]
     UNIFIED_SOURCES += [
         'RenderD3D11TextureHostOGL.cpp',
     ]
 
 if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('android', 'gtk2', 'gtk3'):
-    DEFINES['MOZ_ENABLE_FREETYPE'] = True
     CXXFLAGS += CONFIG['MOZ_CAIRO_CFLAGS']
     CXXFLAGS += CONFIG['CAIRO_FT_CFLAGS']
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
 FINAL_LIBRARY = 'xul'