25050 - assert in nsViewManager :: Refresh

Status: VERIFIED FIXED

(Core Graveyard :: GFX, defect, P3)

Description

[John Bandhauer]

[sorry, I'm clueless about what 'Component' to use for the bug]

Running demo "#8 Forms" in mozilla.exe asserts at:
NS_ASSERTION(!(PR_TRUE == mPainting), "recursive painting not permitted");
And continues to do so if I ignore the assert

I see that there is a similar old (fixed) bug:
http://bugzilla.mozilla.org/show_bug.cgi?id=4363

Also... It bothers me a bit that there are some return paths in this method 
which come after mPainting is set to but before it is cleared.

Comment 1

[Patrick C. Beard]

*** Bug 25130 has been marked as a duplicate of this bug. ***

Comment 2

[Patrick C. Beard]

I'd need a stack crawl to identify the offender, but this is most likely a 
problem with layout.

Comment 3

[troy]

I don't hit an assert, and without a stack trace there's not much I can do with 
it

Comment 4

[John Bandhauer]

Hey, I'm sorry I didn't capture a stack trace at the time. It is not currently 
aserting for me either.

I'm reopening to point out that the code in the two method with this name still 
look broken to me...
http://lxr.mozilla.org/seamonkey/search?string=nsViewManager+%3A%3A+Refresh

They use a flag to protect from rentrance. But, after setting the flag there are 
code paths that exit without clearing the flag. Is this not broken? I don't 
understand this exit condition. The code claims that it is a resonable place 
to return, but says nothing about the state of the mPainting flag. I'll attach a 
proposed patch. Apply it if you think you should. Or reclose the bug. Whatever.

Comment 5

[John Bandhauer]

Attachment: patch to clear mPainting on all code paths out

Comment 6

[troy]

Changing component back to Compositor, because the patch is for a view manager 
file

Comment 7

[Patrick C. Beard]

Patches are in my tree, waiting for tree opening.

Comment 8

[Patrick C. Beard]

Waiting for kevin to check in latest nsViewManager2 files.

Comment 9

[Patrick C. Beard]

Checked in fix for nsViewManager.cpp.

Comment 10

[Chris Petersen]

Marking verified per last comments.