Manual bisection of layout/reftests/reftest.list leads to that reftest.list. Failure is TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1344 bytes during test execution TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 42 instances of gfxASurface with size 32 bytes each (1344 bytes total) == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 32527 |<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->| Per-Inst Leaked Total Rem Mean StdDev Total Rem Mean StdDev 0 TOTAL 66 896 1432093 28 ( 1764.22 +/- 3366.63) 2728269 0 ( 2086.50 +/- 4060.57) 114 gfxASurface 32 896 1002 28 ( 38.42 +/- 9.66) 0 0 ( 0.00 +/- 0.00) Need sleep, will fix tomorrow. Might block fennec, depending on the underlying problem.

More general problem that was only appearing on linux-desktop-ff when we used canvases, because by default there we use gfxXlibSurface for thebeslayer. (Shadow canvas layers currently always use image surfaces.) Can be repro'd on any suite by setting MOZ_LAYERS_FORCE_SHMEM_SURFACES=1. The gfxSharedImageSurfaces themselves are being used correctly. I verified that they're being destroyed when they should be. Still unsure about what's going on here. Looking more.

I tracked down what's happening here: when we do nsRefPtr<gfxSharedImageSurface> s = new gfxSharedImageSurface(); s->Init(...); what ends up happening is, |s| gets a "floating ref" on its first AddRef(), since it doesn't have a valid surface (yet). Then on Init(), we create the cairo_image_surface, which starts out with refcnt=1. The surface ends up with two refs for its one AddRef() --- the floating ref, and the cairo_surface's ref. gfxASurface expects the floating ref to be toasted on the next AddRef(), but there's no guarantee there'll be another AddRef(), when there's not we leak the gfxSharedImageSurface and its cairo surface. (The other ctor that takes an existing Shmem works correctly.) This isn't a trivial leak, but also not a showstopper because we're still cleaning up the shmem segment correctly. It affects fennec and desktop FF because of the mac OOPP code. (I guess we're not testing the Shmem drawing path on tinderbox? :( ) I started on a patch that manually killed the extra floating ref when necessary, but while digging through the gfxASurface code, I realized that its model seems to be that gfxASurface::Init() is always called from a subclass's ctor. gfxSharedImageSurface has broken that model, and in so doing made Init() unsafe in a way I don't want to attempt to fix. So instead, I'll throw out my WIP patch and fix gfxSharedImageSurface. (gfx guys: is the above correct?)

Meat of the patch is obviously in gfx/thebes. The rest is just mechanical rewrites of callers. The big changes are, replacing direct instantiation with factory functions, Create() and Open(). I went with Open() instead of an overloaded Create() for clarity at call sites, since Create() places a burden on the caller to free the Shmem whereas Open() doesn't impose any burden. This naming convention doesn't quite match up with gfxXlibSurface, e.g.

Comment on attachment 497017 [details] [diff] [review] Fix leaking gfxSharedImageSurfaces Yes, this is much better. Exposing Init was a bad idea :-)