295639 - Assert painting SVG image: "trying to get data on an immutable frame"

Status: RESOLVED DUPLICATE of bug 362008

(Core :: SVG, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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

Comment 1

[tor]

Attachment: set flag so we don't assert

Comment 2

[tor]

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

Comment 3

[Jonathan Watt [:jwatt]]

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 4

[Stuart Parmenter]

Comment on attachment 184627 [details] [diff] [review]
set flag so we don't assert

that really shouldn't work...

Comment 5

[Jonathan Watt [:jwatt]]

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.

Comment 6

[Stuart Parmenter]

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.

Comment 7

[Jonathan Watt [:jwatt]]

Great. Thanks for clarifying!

Comment 8

[tor]

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

Comment 9

[Martijn Wargers (dead)]

So should the patch be checked in?

Comment 10

[Mats Palmgren (inactive)]

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

Comment 11

[Boris Zbarsky [:bzbarsky]]

That method hands back a pointer the caller can write to.

Comment 12

[Mats Palmgren (inactive)]

Ok, but do we have consumers that actually writes to it?
Maybe make the return type 'const' and then hunt down the bad callers?

Comment 13

[Boris Zbarsky [:bzbarsky]]

> 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).

Comment 14

[Stuart Parmenter]

ignore the assertion, i'm going to remove it soonish.

Comment 15

[tor]

This will go away when bug 362008 goes in.


*** This bug has been marked as a duplicate of 362008 ***