454730 - Move shutdown of GTK's fontmap from gfxPlatformGtk to nsAppRunner

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[Karl Tomlinson (:karlt)]

GTK's fontmap should not be shutdown until after we have finished with GTK.
Thebes will use a different fontmap after bug 385263.

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: patch [pushed]

This also removes our dependencies on pangocairo and Xft (which we haven't
used for some time).

Comment 2

[Benjamin Smedberg]

Comment on attachment 338030 [details] [diff] [review]
patch [pushed]

I really don't feel comfortable having this kind of cleanup code only run debug-or-memory-logging... what's the reason we can't just turn it on in general?

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> what's the reason we can't just turn it on in general?

cairo_debug_reset_static_data() hits a fatal assertion if there are active
objects remaining.

http://www.cairographics.org/manual/cairo-Error-Handling.html#cairo-debug-reset-static-data

pango_fc_font_map_shutdown can cause a different fatal assertion to be hit in
cairo with some cairo/Pango versions if there are active Pango objects
remaining (bug 399556).

gtk loads different modules according to user/distro settings and so it is
hard or impossible for us to know which ones may leak cairo or Pango objects.

In debug builds this cleanup is useful because it asserts when there are
objects leaked.  In NS_TRACE_MALLOC builds it is useful for cleaning up
shutdown leaks.  I'm not so confident of the usefulness for
NS_BUILD_REFCNT_LOGGING-only builds.  Pango and cairo objects can have
shutdown functions that might release refcnted objects, but we've already shut
down XPCOM here.  However it would test that we'd removed our shutdown functions before/when shutting down XPCOM.

In release builds I see little advantage in having these enabled.
The one thing that they could achieve is crashes on shutdown and restart.

(I think closing the display is different, as I think perhaps the display
should be synchronously closed before calling exec and opening another
connection, but I still don't completely understand  Bug 246313.)

When Thebes uses its own fontmap (bug 385263), we should always shutdown the
fontmap as we have control over cairo and the fontmap and it is only
accessible through Thebes.  I'll change my patch in that bug accordingly.

Comment 4

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/71004cbe3d8a

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

This broke debug build on linux (fedora-7 based)
nsAppRunner.cpp
In file included from ../../dist/include/system_wrappers/ft2build.h:3,
                 from /usr/include/pango-1.0/pango/pangofc-font.h:25,
                 from ../../dist/include/system_wrappers/pango/pangofc-font.h:3,
                 from /usr/include/pango-1.0/pango/pangofc-decoder.h:25,
                 from ../../dist/include/system_wrappers/pango/pangofc-decoder.h:3,
                 from /usr/include/pango-1.0/pango/pangofc-fontmap.h:27,
                 from ../../dist/include/system_wrappers/pango/pangofc-fontmap.h:3,
                 from /home/smaug/mozilla/mozilla_cvs/hg/mozilla/toolkit/xre/nsAppRunner.cpp:280:
/usr/include/ft2build.h:56:38: error: freetype/config/ftheader.h: No such file or directory
In file included from ../../dist/include/system_wrappers/pango/pangofc-font.h:3,
                 from /usr/include/pango-1.0/pango/pangofc-decoder.h:25,
                 from ../../dist/include/system_wrappers/pango/pangofc-decoder.h:3,
                 from /usr/include/pango-1.0/pango/pangofc-fontmap.h:27,
                 from ../../dist/include/system_wrappers/pango/pangofc-fontmap.h:3,
                 from /home/smaug/mozilla/mozilla_cvs/hg/mozilla/toolkit/xre/nsAppRunner.cpp:280:
/usr/include/pango-1.0/pango/pangofc-font.h:26:10: error: #include expects "FILENAME" or <FILENAME>
/usr/include/pango-1.0/pango/pangofc-font.h:112: error: expected identifier before ‘*’ token
/usr/include/pango-1.0/pango/pangofc-font.h:112: error: ‘FT_Face’ declared as function returning a function
/usr/include/pango-1.0/pango/pangofc-font.h:147: error: ‘FT_Face’ does not name a type

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: add MOZ_PANGO_CFLAGS [pushed]

to address comment 5.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 340539 [details] [diff] [review]
add MOZ_PANGO_CFLAGS [pushed]

This works for me

Comment 8

[Karl Tomlinson (:karlt)]

Comment on attachment 340539 [details] [diff] [review]
add MOZ_PANGO_CFLAGS [pushed]

http://hg.mozilla.org/mozilla-central/rev/03e2e5e8dad7