Closed Bug 454730 Opened 11 years ago Closed 11 years ago

Move shutdown of GTK's fontmap from gfxPlatformGtk to nsAppRunner

Categories

(Toolkit :: Startup and Profile System, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1b1

People

(Reporter: karlt, Assigned: karlt)

References

Details

Attachments

(2 files)

GTK's fontmap should not be shutdown until after we have finished with GTK.
Thebes will use a different fontmap after bug 385263.
Attached patch patch [pushed]Splinter Review
This also removes our dependencies on pangocairo and Xft (which we haven't
used for some time).
Attachment #338030 - Flags: superreview?
Attachment #338030 - Flags: review?(roc)
Attachment #338030 - Flags: superreview? → superreview?(benjamin)
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?
(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.
Attachment #338030 - Flags: superreview?(benjamin) → superreview+
http://hg.mozilla.org/mozilla-central/rev/71004cbe3d8a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
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
to address comment 5.
Attachment #340539 - Flags: review?
Attachment #340539 - Flags: review? → review?(benjamin)
Comment on attachment 340539 [details] [diff] [review]
add MOZ_PANGO_CFLAGS [pushed]

This works for me
Attachment #340539 - Flags: review?(benjamin) → review+
Attachment #338030 - Attachment description: patch → patch [pushed]
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
Whiteboard: [attachment 340539 needs checkin]
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [attachment 340539 needs checkin]
Comment on attachment 340539 [details] [diff] [review]
add MOZ_PANGO_CFLAGS [pushed]

http://hg.mozilla.org/mozilla-central/rev/03e2e5e8dad7
Attachment #340539 - Attachment description: add MOZ_PANGO_CFLAGS → add MOZ_PANGO_CFLAGS [pushed]
You need to log in before you can comment on or make changes to this bug.