Bug 717178 - Part 3 alternative: don't put Win32 cairo_font_face_ts into the font-face cache if they were created with an explicit HFONT. r=jrmuizel, a=bajaj
authorRobert O'Callahan <robert@ocallahan.org>
Wed, 02 Jan 2013 19:18:53 +1300
changeset 127103 64b4d4cb1f06caf1822b34b385b5fcdea1419082
parent 127102 0f5e486ffd77e003f13945c9b736d8d12e3f6e3f
child 127104 c947094aa28b5f50d00835562042fe6a918c4f72
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, bajaj
bugs717178
milestone20.0a2
Bug 717178 - Part 3 alternative: don't put Win32 cairo_font_face_ts into the font-face cache if they were created with an explicit HFONT. r=jrmuizel, a=bajaj
gfx/cairo/README
gfx/cairo/cairo/src/cairo-win32-font.c
gfx/cairo/win32-gdi-font-cache-no-HFONT.patch
--- a/gfx/cairo/README
+++ b/gfx/cairo/README
@@ -195,16 +195,18 @@ d2d-gradient-ensure-stops.patch: bug 792
 setlcdfilter_in_tree.patch: bug 790139; force cairo to use FT_Library_SetLcdFilter from our in tree library rather than picking it up from the system
 
 dwrite-font-match-robustness.patch: bug 717178, don't crash when _name_tables_match is passed a nil scaled-font
 
 handle-multi-path-clip.patch: bug 813124, handle multiple clip paths correctly
 
 win32-gdi-font-cache.patch: Bug 717178, cache GDI font faces to reduce usage of GDI resources
 
+win32-gdi-font-cache-no-HFONT.patch: Bug 717178, don't cache GDI font faces when an HFONT belonging to the caller is passed in
+
 ==== pixman patches ====
 
 pixman-android-cpu-detect.patch: Add CPU detection support for Android, where we can't reliably access /proc/self/auxv.
 
 pixman-rename-and-endian.patch: include cairo-platform.h for renaming of external symbols and endian macros
 
 NOTE: we previously supported ARM assembler on MSVC, this has been removed because of the maintenance burden
 
--- a/gfx/cairo/cairo/src/cairo-win32-font.c
+++ b/gfx/cairo/cairo/src/cairo-win32-font.c
@@ -1941,16 +1941,21 @@ const cairo_font_face_backend_t _cairo_w
  * The primary purpose of this mapping is to provide unique
  * #cairo_font_face_t values so that our cache and mapping from
  * #cairo_font_face_t => #cairo_scaled_font_t works. Once the
  * corresponding #cairo_font_face_t objects fall out of downstream
  * caches, we don't need them in this hash table anymore.
  *
  * Modifications to this hash table are protected by
  * _cairo_win32_font_face_mutex.
+ *
+ * Only #cairo_font_face_t values with null 'hfont' (no
+ * HFONT preallocated by caller) are stored in this table. We rely
+ * on callers to manage the lifetime of the HFONT, and they can't
+ * do that if we share #cairo_font_face_t values with other callers.
  */
 
 static cairo_hash_table_t *cairo_win32_font_face_hash_table = NULL;
 
 static int
 _cairo_win32_font_face_keys_equal (const void *key_a,
 				   const void *key_b);
 
@@ -2036,22 +2041,24 @@ static int
 }
 
 static void
 _cairo_win32_font_face_destroy (void *abstract_face)
 {
     cairo_hash_table_t *hash_table;
     cairo_win32_font_face_t *font_face = abstract_face;
 
-    hash_table = _cairo_win32_font_face_hash_table_lock ();
-    if (unlikely (hash_table == NULL)) {
-        return;
+    if (!font_face->hfont) {
+        hash_table = _cairo_win32_font_face_hash_table_lock ();
+        if (unlikely (hash_table == NULL)) {
+            return;
+        }
+        _cairo_hash_table_remove (hash_table, &font_face->base.hash_entry);
+        _cairo_win32_font_face_hash_table_unlock ();
     }
-    _cairo_hash_table_remove (hash_table, &font_face->base.hash_entry);
-    _cairo_win32_font_face_hash_table_unlock ();
 }
 
 /**
  * cairo_win32_font_face_create_for_logfontw_hfont:
  * @logfont: A #LOGFONTW structure specifying the font to use.
  *   If @font is %NULL then the lfHeight, lfWidth, lfOrientation and lfEscapement
  *   fields of this structure are ignored. Otherwise lfWidth, lfOrientation and
  *   lfEscapement must be zero.
@@ -2070,55 +2077,63 @@ static void
  **/
 cairo_font_face_t *
 cairo_win32_font_face_create_for_logfontw_hfont (LOGFONTW *logfont, HFONT font)
 {
     cairo_win32_font_face_t *font_face, key;
     cairo_hash_table_t *hash_table;
     cairo_status_t status;
 
-    hash_table = _cairo_win32_font_face_hash_table_lock ();
-    if (unlikely (hash_table == NULL)) {
-        _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
-	return (cairo_font_face_t *)&_cairo_font_face_nil;
-    }
+    if (!font) {
+        hash_table = _cairo_win32_font_face_hash_table_lock ();
+        if (unlikely (hash_table == NULL)) {
+            _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
+	    return (cairo_font_face_t *)&_cairo_font_face_nil;
+        }
 
-    _cairo_win32_font_face_init_key (&key, logfont, font);
+        _cairo_win32_font_face_init_key (&key, logfont, font);
 
-    /* Return existing unscaled font if it exists in the hash table. */
-    font_face = _cairo_hash_table_lookup (hash_table,
-					 &key.base.hash_entry);
-    if (font_face != NULL) {
-	cairo_font_face_reference (&font_face->base);
-	goto DONE;
+        /* Return existing unscaled font if it exists in the hash table. */
+        font_face = _cairo_hash_table_lookup (hash_table,
+                                              &key.base.hash_entry);
+        if (font_face != NULL) {
+	    cairo_font_face_reference (&font_face->base);
+	    goto DONE;
+        }
     }
 
     /* Otherwise create it and insert into hash table. */
     font_face = malloc (sizeof (cairo_win32_font_face_t));
     if (!font_face) {
         _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
 	goto FAIL;
     }
 
     _cairo_win32_font_face_init_key (font_face, logfont, font);
     _cairo_font_face_init (&font_face->base, &_cairo_win32_font_face_backend);
+    assert (font_face->base.hash_entry.hash == key.base.hash_entry.hash);
 
-    assert (font_face->base.hash_entry.hash == key.base.hash_entry.hash);
-    status = _cairo_hash_table_insert (hash_table,
-				       &font_face->base.hash_entry);
-    if (unlikely (status))
-	goto FAIL;
+    if (!font) {
+        status = _cairo_hash_table_insert (hash_table,
+                                           &font_face->base.hash_entry);
+        if (unlikely (status))
+	    goto FAIL;
+    }
 
 DONE:
-    _cairo_win32_font_face_hash_table_unlock ();
+    if (!font) {
+        _cairo_win32_font_face_hash_table_unlock ();
+    }
 
     return &font_face->base;
 
 FAIL:
-    _cairo_win32_font_face_hash_table_unlock ();
+    if (!font) {
+        _cairo_win32_font_face_hash_table_unlock ();
+    }
 
     return (cairo_font_face_t *)&_cairo_font_face_nil;
 }
 
 /**
  * cairo_win32_font_face_create_for_logfontw:
  * @logfont: A #LOGFONTW structure specifying the font to use.
  *   The lfHeight, lfWidth, lfOrientation and lfEscapement
new file mode 100644
--- /dev/null
+++ b/gfx/cairo/win32-gdi-font-cache-no-HFONT.patch
@@ -0,0 +1,145 @@
+# HG changeset patch
+# User Robert O'Callahan <robert@ocallahan.org>
+# Date 1357107533 -46800
+# Node ID ed54dfdd2facb11a4d4158138b460a31de45e9f7
+# Parent ab6457cc16ec14ea07386dcfc57cad6b8a9ac55d
+Bug 717178. Part 3 alternative: don't put Win32 cairo_font_face_ts into the font-face cache if they were created with an explicit HFONT. r=jrmuizel
+
+diff --git a/gfx/cairo/cairo/src/cairo-win32-font.c b/gfx/cairo/cairo/src/cairo-win32-font.c
+--- a/gfx/cairo/cairo/src/cairo-win32-font.c
++++ b/gfx/cairo/cairo/src/cairo-win32-font.c
+@@ -1941,16 +1942,21 @@ const cairo_font_face_backend_t _cairo_w
+  * The primary purpose of this mapping is to provide unique
+  * #cairo_font_face_t values so that our cache and mapping from
+  * #cairo_font_face_t => #cairo_scaled_font_t works. Once the
+  * corresponding #cairo_font_face_t objects fall out of downstream
+  * caches, we don't need them in this hash table anymore.
+  *
+  * Modifications to this hash table are protected by
+  * _cairo_win32_font_face_mutex.
++ *
++ * Only #cairo_font_face_t values with null 'hfont' (no
++ * HFONT preallocated by caller) are stored in this table. We rely
++ * on callers to manage the lifetime of the HFONT, and they can't
++ * do that if we share #cairo_font_face_t values with other callers.
+  */
+ 
+ static cairo_hash_table_t *cairo_win32_font_face_hash_table = NULL;
+ 
+ static int
+ _cairo_win32_font_face_keys_equal (const void *key_a,
+ 				   const void *key_b);
+ 
+@@ -2036,22 +2042,24 @@ static int
+ }
+ 
+ static void
+ _cairo_win32_font_face_destroy (void *abstract_face)
+ {
+     cairo_hash_table_t *hash_table;
+     cairo_win32_font_face_t *font_face = abstract_face;
+ 
+-    hash_table = _cairo_win32_font_face_hash_table_lock ();
+-    if (unlikely (hash_table == NULL)) {
+-        return;
++    if (!font_face->hfont) {
++        hash_table = _cairo_win32_font_face_hash_table_lock ();
++        if (unlikely (hash_table == NULL)) {
++            return;
++        }
++        _cairo_hash_table_remove (hash_table, &font_face->base.hash_entry);
++        _cairo_win32_font_face_hash_table_unlock ();
+     }
+-    _cairo_hash_table_remove (hash_table, &font_face->base.hash_entry);
+-    _cairo_win32_font_face_hash_table_unlock ();
+ }
+ 
+ /**
+  * cairo_win32_font_face_create_for_logfontw_hfont:
+  * @logfont: A #LOGFONTW structure specifying the font to use.
+  *   If @font is %NULL then the lfHeight, lfWidth, lfOrientation and lfEscapement
+  *   fields of this structure are ignored. Otherwise lfWidth, lfOrientation and
+  *   lfEscapement must be zero.
+@@ -2070,55 +2078,63 @@ static void
+  **/
+ cairo_font_face_t *
+ cairo_win32_font_face_create_for_logfontw_hfont (LOGFONTW *logfont, HFONT font)
+ {
+     cairo_win32_font_face_t *font_face, key;
+     cairo_hash_table_t *hash_table;
+     cairo_status_t status;
+ 
+-    hash_table = _cairo_win32_font_face_hash_table_lock ();
+-    if (unlikely (hash_table == NULL)) {
+-        _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
+-	return (cairo_font_face_t *)&_cairo_font_face_nil;
+-    }
++    if (!font) {
++        hash_table = _cairo_win32_font_face_hash_table_lock ();
++        if (unlikely (hash_table == NULL)) {
++            _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
++	    return (cairo_font_face_t *)&_cairo_font_face_nil;
++        }
+ 
+-    _cairo_win32_font_face_init_key (&key, logfont, font);
++        _cairo_win32_font_face_init_key (&key, logfont, font);
+ 
+-    /* Return existing unscaled font if it exists in the hash table. */
+-    font_face = _cairo_hash_table_lookup (hash_table,
+-					 &key.base.hash_entry);
+-    if (font_face != NULL) {
+-	cairo_font_face_reference (&font_face->base);
+-	goto DONE;
++        /* Return existing unscaled font if it exists in the hash table. */
++        font_face = _cairo_hash_table_lookup (hash_table,
++                                              &key.base.hash_entry);
++        if (font_face != NULL) {
++	    cairo_font_face_reference (&font_face->base);
++	    goto DONE;
++        }
+     }
+ 
+     /* Otherwise create it and insert into hash table. */
+     font_face = malloc (sizeof (cairo_win32_font_face_t));
+     if (!font_face) {
+         _cairo_error_throw (CAIRO_STATUS_NO_MEMORY);
+ 	goto FAIL;
+     }
+ 
+     _cairo_win32_font_face_init_key (font_face, logfont, font);
+     _cairo_font_face_init (&font_face->base, &_cairo_win32_font_face_backend);
++    assert (font_face->base.hash_entry.hash == key.base.hash_entry.hash);
+ 
+-    assert (font_face->base.hash_entry.hash == key.base.hash_entry.hash);
+-    status = _cairo_hash_table_insert (hash_table,
+-				       &font_face->base.hash_entry);
+-    if (unlikely (status))
+-	goto FAIL;
++    if (!font) {
++        status = _cairo_hash_table_insert (hash_table,
++                                           &font_face->base.hash_entry);
++        if (unlikely (status))
++	    goto FAIL;
++    }
+ 
+ DONE:
+-    _cairo_win32_font_face_hash_table_unlock ();
++    if (!font) {
++        _cairo_win32_font_face_hash_table_unlock ();
++    }
+ 
+     return &font_face->base;
+ 
+ FAIL:
+-    _cairo_win32_font_face_hash_table_unlock ();
++    if (!font) {
++        _cairo_win32_font_face_hash_table_unlock ();
++    }
+ 
+     return (cairo_font_face_t *)&_cairo_font_face_nil;
+ }
+ 
+ /**
+  * cairo_win32_font_face_create_for_logfontw:
+  * @logfont: A #LOGFONTW structure specifying the font to use.
+  *   The lfHeight, lfWidth, lfOrientation and lfEscapement