Jeremy Lea Assignee
	Description • 16 years ago

These two classes are now only used as base classes for nsThebesDeviceContext and nsThebesRendering context.  The attached patch rolls them into these two.

The only changes in behavior are to actually register the memory pressure observer and to make some of the methods of nsFontCache non-virtual, since there is no inheritance.  Well, these are the only two intended changes...  I've tested this on FreeBSD (posting from the build), but there shouldn't be problems on other platforms.

The Makefile changes might need a careful look, along with the includes.

	David Baron :dbaron:
	Comment 1 • 16 years ago

You should request review by following the "Details" link for the attachment, changing the review dropdown to "?", and entering an email address or unique substring thereof.  For this patch, I'd recommend either "vladimir@pobox" or "pavlov@pavlov".

	Jeremy Lea Assignee
	Comment 2 • 16 years ago

Updated patch which makes more of the members non-virtual, fixes the white space and some other style bits to match nsThebes* and also removes the unused mWidgetSurfaceCache member from nsThebesDeviceContext.  Tested on FreeBSD and Windows.  I've tried to trim the makefile changes to their minimum, but haven't tried to clean up gfx/src/Makefile.in and gfx/src/shared/Makefile.in.

This should give a slight code size win, a slight performance win, less data and makes the code cleaner.

	Jeremy Lea Assignee
	Comment 3 • 16 years ago

This is a refreshed patch against mozilla-central.  I'm still getting used to hg, so I hope I didn't screw it up ;-)

Only change from previous is the removal of an extra include which got added at some point.

	Jeremy Lea Assignee
	Comment 4 • 16 years ago

Update this again to tip, which changed a few things for @font-face.  I've been using this patch for a while with no ill effects (passes reftests and mochitests on all platforms).

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 5 • 16 years ago

Sorry this has been kicking around for so long -- I'm happy to have this land, but for 1.9.2; we should be branching 1.9.1 soon (week at the most) and can start doing all the cleanup on the trunk then.

	Jeremy Lea Assignee
	Comment 6 • 15 years ago

Two small fixes - set the memory pressure observer and remove an unused variable...  Probably should file separate bugs for these...

	Jeremy Lea Assignee
	Comment 7 • 15 years ago

Part 2 - Remove nsDeviceContextImpl, and make the nsFontCache class local and non-virtual.  This could be cleaned up more, but we should probably do unused methods first, then look to remove cruft.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 8 • 15 years ago

Comment on attachment 371334 [details] [diff] [review]
Part 2 - nsDeviceContextImpl

I am terrified, but let's do it!

	Jeremy Lea Assignee
	Comment 9 • 15 years ago

Part 3 - Remove nsRenderingContextImpl.  Could also do with more cleanup, but unused methods should be removed first.

I've built these three patches on all major platforms, and tested on Win/Linux.  I could run a try server run now that I have commit rights...

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)
	Comment 10 • 15 years ago

Comment on attachment 371337 [details] [diff] [review]
Part 3 - nsRenderingContextImpl

Yeah, throw all these onto the tryserver first.

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 15 years ago

This is failing leak tests at least on mac, due to triggering an assert:

  ASSERTION: device context is initialized twice!: '!mInitialized', file /builds/slave/mozilla-central-macosx-debug/build/gfx/src/thebes/nsThebesDeviceContext.cpp, line 753

I'd paste in the stack too, but it's pretty bogus due to not having been run through fix-macosx-stack.  It's at the end of http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239216265.1239218893.28973.gz if someone wants to look.

	Jeremy Lea Assignee
	Comment 12 • 15 years ago

I removed the assert in changeset 6386025d72c3, and added a comment to bug 482928.  I'm not sure if I should add a dependency...

	Jeremy Lea Assignee
	Comment 13 • 15 years ago

Pushed as:

http://hg.mozilla.org/mozilla-central/rev/c9b6282f0db7
http://hg.mozilla.org/mozilla-central/rev/38720611615c
http://hg.mozilla.org/mozilla-central/rev/5cb2c6ac45a6

and the assertion commented out in:

http://hg.mozilla.org/mozilla-central/rev/6386025d72c3