Bug 1383767 - guarantee FreeType thread-safety by holding Cairo per-face lock and locking down rasterization. r=jrmuizel

MozReview-Commit-ID: DuPRIUBgw4W

gfx/2d/2D.h

@@ -1645,6 +1645,8 @@ public:
 
   static FT_Library NewFTLibrary();
   static void ReleaseFTLibrary(FT_Library aFTLibrary);
+  static void LockFTLibrary(FT_Library aFTLibrary);
+  static void UnlockFTLibrary(FT_Library aFTLibrary);
 
   static FT_Face NewFTFace(FT_Library aFTLibrary, const char* aFileName, int aFaceIndex);
   static FT_Face NewFTFaceFromData(FT_Library aFTLibrary, const uint8_t* aData, size_t aDataSize, int aFaceIndex);

gfx/2d/Factory.cpp

@@ -186,6 +186,18 @@ mozilla_ReleaseFTFace(FT_Face aFace)
   mozilla::gfx::Factory::ReleaseFTFace(aFace);
 }
 
+void
+mozilla_LockFTLibrary(FT_Library aFTLibrary)
+{
+  mozilla::gfx::Factory::LockFTLibrary(aFTLibrary);
+}
+
+void
+mozilla_UnlockFTLibrary(FT_Library aFTLibrary)
+{
+  mozilla::gfx::Factory::UnlockFTLibrary(aFTLibrary);
+}
+
 }
 #endif
 
@@ -667,6 +679,20 @@ Factory::ReleaseFTLibrary(FT_Library aFTLibrary)
   FT_Done_FreeType(aFTLibrary);
 }
 
+void
+Factory::LockFTLibrary(FT_Library aFTLibrary)
+{
+  MOZ_ASSERT(mFTLock);
+  mFTLock->Lock();
+}
+
+void
+Factory::UnlockFTLibrary(FT_Library aFTLibrary)
+{
+  MOZ_ASSERT(mFTLock);
+  mFTLock->Unlock();
+}
+
 FT_Face
 Factory::NewFTFace(FT_Library aFTLibrary, const char* aFileName, int aFaceIndex)
 {

gfx/cairo/cairo/src/cairo-ft-font.c

@@ -107,6 +107,8 @@ static setLcdFilterFunc setLcdFilter;
 extern FT_Face mozilla_NewFTFace(FT_Library aFTLibrary, const char* aFileName, int aFaceIndex);
 extern FT_Face mozilla_NewFTFaceFromData(FT_Library aFTLibrary, const uint8_t* aData, size_t aDataSize, int aFaceIndex);
 extern void mozilla_ReleaseFTFace(FT_Face aFace);
+extern void mozilla_LockFTLibrary(FT_Library aFTLibrary);
+extern void mozilla_UnlockFTLibrary(FT_Library aFTLibrary);
 
 /**
  * SECTION:cairo-ft
@@ -1453,13 +1455,21 @@ _render_glyph_outline (FT_Face                    face,
 #endif
         }
 
-	if (setLcdFilter)
-          setLcdFilter (library, lcd_filter);
+	if (setLcdFilter &&
+	    (render_mode == FT_RENDER_MODE_LCD ||
+	     render_mode == FT_RENDER_MODE_LCD_V)) {
+	    mozilla_LockFTLibrary (library);
+	    setLcdFilter (library, lcd_filter);
+	}
 
 	fterror = FT_Render_Glyph (face->glyph, render_mode);
 
-	if (setLcdFilter)
-          setLcdFilter (library, FT_LCD_FILTER_NONE);
+	if (setLcdFilter &&
+	    (render_mode == FT_RENDER_MODE_LCD ||
+	     render_mode == FT_RENDER_MODE_LCD_V)) {
+	    setLcdFilter (library, FT_LCD_FILTER_NONE);
+	    mozilla_UnlockFTLibrary (library);
+	}
 
 	if (fterror != 0)
 		return _cairo_error (CAIRO_STATUS_NO_MEMORY);
@@ -2218,7 +2228,7 @@ _cairo_ft_scaled_glyph_init (void			*abstract_font,
     cairo_ft_scaled_font_t *scaled_font = abstract_font;
     cairo_ft_unscaled_font_t *unscaled = scaled_font->unscaled;
     FT_GlyphSlot glyph;
-    FT_Face face;
+    FT_Face face = NULL;
     FT_Error error;
     int load_flags = scaled_font->ft_options.load_flags;
     FT_Glyph_Metrics *metrics;
@@ -3276,7 +3286,8 @@ cairo_ft_scaled_font_lock_face (cairo_scaled_font_t *abstract_font)
      * opportunity for creating deadlock. This is obviously unsafe,
      * but as documented, the user must add manual locking when using
      * this function. */
-     CAIRO_MUTEX_UNLOCK (scaled_font->unscaled->mutex);
+    // BEWARE: Mozilla's tree Cairo keeps the lock across calls for thread-safety.
+    // CAIRO_MUTEX_UNLOCK (scaled_font->unscaled->mutex);
 
     return face;
 }
@@ -3307,7 +3318,8 @@ cairo_ft_scaled_font_unlock_face (cairo_scaled_font_t *abstract_font)
      * cairo_ft_scaled_font_lock_face, so we have to acquire it again
      * as _cairo_ft_unscaled_font_unlock_face expects it to be held
      * when we call into it. */
-    CAIRO_MUTEX_LOCK (scaled_font->unscaled->mutex);
+    // BEWARE: Mozilla's tree Cairo keeps the lock across calls for thread-safety.
+    //CAIRO_MUTEX_LOCK (scaled_font->unscaled->mutex);
 
     _cairo_ft_unscaled_font_unlock_face (scaled_font->unscaled);
 }

gfx/skia/skia/src/ports/SkFontHost_cairo.cpp

@@ -58,6 +58,11 @@ typedef enum FT_LcdFilter_
 #define SK_FONTHOST_CAIRO_STANDALONE 1
 #endif
 
+extern "C" {
+    extern void mozilla_LockFTLibrary(FT_Library aFTLibrary);
+    extern void mozilla_UnlockFTLibrary(FT_Library aFTLibrary);
+}
+
 static cairo_user_data_key_t kSkTypefaceKey;
 
 static bool gFontHintingEnabled = true;
@@ -744,6 +749,7 @@ void SkScalerContext_CairoFT::generateImage(const SkGlyph& glyph)
         isLCD(glyph) &&
         gSetLcdFilter;
     if (useLcdFilter) {
+        mozilla_LockFTLibrary(face->glyph->library);
         gSetLcdFilter(face->glyph->library, fLcdFilter);
     }
 
@@ -758,6 +764,7 @@ void SkScalerContext_CairoFT::generateImage(const SkGlyph& glyph)
 
     if (useLcdFilter) {
         gSetLcdFilter(face->glyph->library, FT_LCD_FILTER_NONE);
+        mozilla_UnlockFTLibrary(face->glyph->library);
     }
 }