BUILD: current trunk Firefox build (GTK2, cairo SVG). STEPS TO REPRODUCE: 1) Load http://fuchsia-design.com/CanvaSVG/Tests/Test.xml ACTUAL RESULTS: ###!!! ASSERTION: trying to get data on an immutable frame: 'mMutable', file ../../../../mozilla/gfx/src/shared/gfxImageFrame.cpp, line 274 EXPECTED RESULTS: No assert. Stack to assert: #0 0xb736347d in gfxImageFrame::GetImageData (this=0x8abf900, aData=0xbfffdbbc, length=0xbfffdbc4) at ../../../../mozilla/gfx/src/shared/gfxImageFrame.cpp:274 #1 0xb6f925e6 in nsSVGImageFrame::ConvertFrame (this=0x87ee078, aNewFrame=0x8abf900) at ../../../../../mozilla/layout/svg/base/src/nsSVGImageFrame.cpp:496 #2 0xb6f91b3b in nsSVGImageFrame::PaintSVG (this=0x87ee078, canvas=0x87ef6b8, dirtyRectTwips=@0xbfffdee0) at ../../../../../mozilla/layout/svg/base/src/nsSVGImageFrame.cpp:329 #3 0xb6f9ea88 in nsSVGOuterSVGFrame::Paint (this=0x88a7450, aPresContext=0x8a89330, aRenderingContext=@0x8a7f790, aDirtyRect=@0xbfffdee0, aWhichLayer=eFramePaintLayer_Overlay, aFlags=0) at ../../../../../mozilla/layout/svg/base/src/nsSVGOuterSVGFrame.cpp:915
Created attachment 184627 [details] [diff] [review] set flag so we don't assert
*** Bug 329469 has been marked as a duplicate of this bug. ***
pav, can you review this please? It's a one line change, and it's been up for review for almost a year. This assertion is getting on my nerves.
Comment on attachment 184627 [details] [diff] [review] set flag so we don't assert that really shouldn't work...
Do you mean you wouldn't expect this to prevent the assertion, or that it should work, but we should change things so it doesn't? Or do you mean this is the wrong thing to do? I didn't mean to ask you to allow in something that you don't think we should do, just to ask that this get some comment one way or another.
well, SetMutable was supposed to be 1-way. That said, it can do proper 2-way with cairo, so i think it is ok. i'm going to revisit the image apis at some point soonish anyways and fix the issues.
Great. Thanks for clarifying!
*** Bug 346439 has been marked as a duplicate of this bug. ***
So should the patch be checked in?
Same assertion occurs for some of the (non-SVG) cursor images here: http://biesi.damowmow.com/cursor2.html Why do we assert on *reading* the data from an immutable object anyway? Shouldn't we just remove these assertions: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/src/shared/gfxImageFrame.cpp&rev=1.36&root=/cvsroot&mark=276,420#271
That method hands back a pointer the caller can write to.
Ok, but do we have consumers that actually writes to it? Maybe make the return type 'const' and then hunt down the bad callers?
> Ok, but do we have consumers that actually writes to it? Yes. The image decoders. In any case, you can't just change the type, since this is an IDL-defined method (sorta; it's not like it's following IDL ownership stuff).
ignore the assertion, i'm going to remove it soonish.
This will go away when bug 362008 goes in. *** This bug has been marked as a duplicate of 362008 ***