Bug 978608: Shut down pango's fontmap cleanly and shut down fontconfig, #if CLEANUP_MEMORY. r=karlt
authorL. David Baron <dbaron@dbaron.org>
Mon, 03 Mar 2014 00:54:38 -0800
changeset 188627 c66cd5bf1aadd254aefa2078ef3d3f319e006a53
parent 188626 2275e85c9ad070c25bba736fa4ea3ed3afb2bf5e
child 188628 8a72d26209182966111fcc0a6a721c65ac3160ec
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskarlt
bugs978608
milestone30.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 978608: Shut down pango's fontmap cleanly and shut down fontconfig, #if CLEANUP_MEMORY. r=karlt This bumps the minimum required pango version to 1.22, released 29 Sep 2008.
configure.in
gfx/thebes/gfxPlatform.cpp
gfx/thebes/gfxPlatformGtk.cpp
toolkit/xre/nsAppRunner.cpp
--- a/configure.in
+++ b/configure.in
@@ -55,17 +55,17 @@ MOZPNG=10609
 NSPR_VERSION=4
 NSS_VERSION=3
 
 dnl Set the minimum version of toolkit libs used by mozilla
 dnl ========================================================
 GLIB_VERSION=1.2.0
 PERL_VERSION=5.006
 CAIRO_VERSION=1.10
-PANGO_VERSION=1.14.0
+PANGO_VERSION=1.22.0
 GTK2_VERSION=2.10.0
 GTK3_VERSION=3.0.0
 WINDRES_VERSION=2.14.90
 W32API_VERSION=3.14
 GNOMEVFS_VERSION=2.0
 GNOMEUI_VERSION=2.2.0
 GCONF_VERSION=1.2.1
 GIO_VERSION=2.20
--- a/gfx/thebes/gfxPlatform.cpp
+++ b/gfx/thebes/gfxPlatform.cpp
@@ -529,24 +529,16 @@ gfxPlatform::~gfxPlatform()
     // Cairo objects e.g. through SkCairoFTTypeface
     SkGraphics::Term();
 #endif
 
 #if MOZ_TREE_CAIRO
     cairo_debug_reset_static_data();
 #endif
 #endif
-
-#if 0
-    // It would be nice to do this (although it might need to be after
-    // the cairo shutdown that happens in ~gfxPlatform).  It even looks
-    // idempotent.  But it has fatal assertions that fire if stuff is
-    // leaked, and we hit them.
-    FcFini();
-#endif
 }
 
 bool
 gfxPlatform::PreferMemoryOverShmem() const {
   MOZ_ASSERT(!CompositorParent::IsInCompositorThread());
   return mLayersPreferMemoryOverShmem;
 }
 
--- a/gfx/thebes/gfxPlatformGtk.cpp
+++ b/gfx/thebes/gfxPlatformGtk.cpp
@@ -70,24 +70,16 @@ gfxPlatformGtk::gfxPlatformGtk()
 }
 
 gfxPlatformGtk::~gfxPlatformGtk()
 {
     gfxFontconfigUtils::Shutdown();
     sFontconfigUtils = nullptr;
 
     gfxPangoFontGroup::Shutdown();
-
-#if 0
-    // It would be nice to do this (although it might need to be after
-    // the cairo shutdown that happens in ~gfxPlatform).  It even looks
-    // idempotent.  But it has fatal assertions that fire if stuff is
-    // leaked, and we hit them.
-    FcFini();
-#endif
 }
 
 already_AddRefed<gfxASurface>
 gfxPlatformGtk::CreateOffscreenSurface(const IntSize& size,
                                        gfxContentType contentType)
 {
     nsRefPtr<gfxASurface> newSurface;
     bool needsClear = true;
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -218,16 +218,17 @@ static char **gQtOnlyArgv;
 #define PANGO_ENABLE_BACKEND
 #include <pango/pangofc-fontmap.h>
 #endif
 #include <gtk/gtk.h>
 #ifdef MOZ_X11
 #include <gdk/gdkx.h>
 #endif /* MOZ_X11 */
 #include "nsGTKToolkit.h"
+#include <fontconfig/fontconfig.h>
 #endif
 #include "BinaryPath.h"
 
 #ifdef MOZ_LINKER
 extern "C" MFBT_API bool IsSignalHandlingBroken();
 #endif
 
 namespace mozilla {
@@ -2610,38 +2611,47 @@ static void MOZ_gdk_display_close(GdkDis
     // like Pango and cairo. But if cairo shutdown is buggy, we should
     // shut down cairo first otherwise it may crash because of dangling
     // references to Display objects (see bug 469831).
     if (!theme_is_qt)
       gdk_display_close(display);
   }
 
 #if CLEANUP_MEMORY
+  // Clean up PangoCairo's default fontmap.
+  // This pango_fc_font_map_shutdown call (and the associated code to
+  // get the font map) really shouldn't be needed anymore, except that
+  // it's needed to avoid having cairo_debug_reset_static_data fatally
+  // assert if we've leaked other things that hold on to the fontmap,
+  // which is something that currently happens in mochitest-plugins.
+  // Even if it didn't happen in mochitest-plugins, we probably want to
+  // avoid the crash-on-leak problem since it makes it harder to use
+  // many of our leak tools to debug leaks.
+
   // This doesn't take a reference.
   PangoFontMap *fontmap = pango_context_get_font_map(pangoContext);
   // Do some shutdown of the fontmap, which releases the fonts, clearing a
   // bunch of circular references from the fontmap through the fonts back to
   // itself.  The shutdown that this does is much less than what's done by
   // the fontmap's finalize, though.
   if (PANGO_IS_FC_FONT_MAP(fontmap))
       pango_fc_font_map_shutdown(PANGO_FC_FONT_MAP(fontmap));
   g_object_unref(pangoContext);
-  // PangoCairo still holds a reference to the fontmap.
-  // Now that we have finished with GTK and Pango, we could unref fontmap,
-  // which would allow us to call FcFini, but removing what is really
-  // Pango's ref feels a bit evil.  Pango-1.22 will have support for
-  // pango_cairo_font_map_set_default(nullptr), which would release the
-  // reference on the old fontmap.
+
+  // Tell PangoCairo to release its default fontmap.
+  pango_cairo_font_map_set_default(nullptr);
 
   // cairo_debug_reset_static_data() is prototyped through cairo.h included
   // by gtk.h.
 #ifdef cairo_debug_reset_static_data
 #error "Looks like we're including Mozilla's cairo instead of system cairo"
 #endif
   cairo_debug_reset_static_data();
+  // FIXME: Do we need to call this in non-GTK2 cases as well?
+  FcFini();
 #endif // CLEANUP_MEMORY
 
   if (buggyCairoShutdown) {
     if (!theme_is_qt)
       gdk_display_close(display);
   }
 }
 #endif // MOZ_WIDGET_GTK2