Loading the testcase and quitting makes trace-refcnt report leaked nsStyleContext and gfxTextRun objects. I think this bug appeared within the last few days on mozilla-central.

hg bisect says this is a regression from bug 441782.

What's particularly odd is that I'm seeing a style context leaked because an nsTextFrame takes a reference to it and never releases it. But we're not leaking any frames or content nodes... Is this a matter of us just releasing some things too late during shutdown?

I get the below assertion when loading the test case in a debug build: ASSERTION: Some objects allocated with AllocateFrame were not freed: 'mFrameCount == 0', file c:/mozroot/hgmoz/layout/base/nsPresShell.cpp, line 675 I'm no expert here, but it seems like some parts of a frame are not being freed. Does this help in tracking this down? Where should I move from here?

That assertion failure is just a side effect of leaking nsStyleContexts, since nsStyleContexts are allocated from the same arena as frames (see "nsStyleContext::operator new"). To debug this, you'll have to track down the nsTextFrame leak (but see comment 3).

I tried generating the shutdown log using the instructions at <https://developer.mozilla.org/en/Debugging_memory_leaks#Analyzing_the_shutdown_log>, but the output of this log shows nothing relevant. There is no occurrence of gfxTextRun in the log.

I believe this is caused by a mistake in the patch for bug 441782: >diff --git a/gfx/thebes/public/gfxTextRunWordCache.h b/gfx/thebes/public/gfxTextRunWordCache.h >--- a/gfx/thebes/public/gfxTextRunWordCache.h >+++ b/gfx/thebes/public/gfxTextRunWordCache.h >@@ -46,7 +46,15 @@ > */ > class THEBES_API gfxTextRunWordCache { > public: >- enum { TEXT_IN_CACHE = 0x10000000 }; >+ enum { >+ TEXT_IN_CACHE = 0x10000000, >+ /** >+ * When set, the previous character for this textrun was an Arabic >+ * character. This is used for the context detection necessary for >+ * bidi.numeral implementation. >+ */ >+ TEXT_INCOMING_ARABICCHAR = 0x10000000 >+ }; > > /** > * Create a textrun using cached words. This uses the same bit in the flags for both TEXT_IN_CACHE and TEXT_INCOMING_ARABICCHAR, which is invalid. We should reorganize the flag bits slightly, as we're running out of available "user" bits; I will attach a patch after some further testing here.

See bug 471389.

This patch moves the TEXT_{INCOMING,TRAILING}_ARABICCHAR flags in order to avoid overlapping the TEXT_IN_CACHE flag. This appears to resolve the reported leakage.

This seems to fix the leak. Johnathan, are you going to ask review on your patch?

Yes, I'll request review shortly, just wanted to let my tryserver build finish to check that nothing breaks there.

Comment on attachment 354952 [details] [diff] [review] resolve conflict in text-run flags causing leakage >diff --git a/gfx/thebes/public/gfxTextRunWordCache.h b/gfx/thebes/public/gfxTextRunWordCache.h >- /** > >- * When set, the previous character for this textrun was an Arabic > >- * character. This is used for the context detection necessary for > >- * bidi.numeral implementation. > >- */ Any special reason to remove this comment?

There was no reason to delete that comment, it was inadvertently left out during various rearrangements of the code. I've updated the patch to restore it, as it's a helpful note about what's going on.

Comment on attachment 354968 [details] [diff] [review] updated patch to restore comment (see comment #12) BTW, I don't think I'm eligible to give reviews officially, so you still need roc's review on this as well. :-)

AFAIK, this is ready to land, but I can't edit the Keywords to add "checkin-needed" (I guess that's because it isn't one of "my bugs"?) Can someone either take care of this or assign it to me?

Landed on mozilla-central: <http://hg.mozilla.org/mozilla-central/rev/00f1c4de50b8> Thanks for your patch, Jonathan!

Comment on attachment 354968 [details] [diff] [review] updated patch to restore comment (see comment #12) We'll want this on the 1.9.1 branch after it bakes.

(In reply to comment #17) > (From update of attachment 354968 [details] [diff] [review]) > We'll want this on the 1.9.1 branch after it bakes. This is already a blocker; should it need explicit approval?

Comment on attachment 354968 [details] [diff] [review] updated patch to restore comment (see comment #12) No, my fault not noticing that it was a blocker.

verified FIXED using the attached test case (no assertion) on debug builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090608 Shiretoko/3.5pre ID:20090608122057 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090608 Minefield/3.6a1pre ID:20090608122028