333253 - two NS_WARNINGs for each file: URL icon on personal toolbar (icon decoder)

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

In cairo-gtk2 builds (as opposed to non-cairo gtk2 builds), I see the following pair of NS_WARNINGs on startup for each (non-directory?) file: URL on my personal toolbar:

WARNING: NS_ENSURE_TRUE(buf_end - data >= PRInt32(bpr + abpr) * height) failed: file /builds/trunk/mozilla/modules/libpr0n/decoders/icon/nsIconDecoder.cpp, line 146
WARNING: NS_ENSURE_TRUE(mSuspendCount > 0) failed: file /builds/trunk/mozilla/netwerk/base/src/nsInputStreamPump.cpp, line 228

The first one may need to be moved into the #ifdefs and an appropriate alternative written for the cairo codepath, but the second one really makes it seem like there's a bug somewhere as well.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

note that this seems to require a no-places build

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

the patch in bug 343599 fixes the mSuspendCount warning

Comment 3

[Stuart Parmenter]

i think the 2nd one results from the 1st...

Comment 4

[Stuart Parmenter]

Attachment: fix

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #3)
> i think the 2nd one results from the 1st...

sure, but it was also a bug in the input stream pump.

Comment 6

[Stuart Parmenter]

fixed

Comment 7

[Stuart Parmenter]

backed out real fix in progress.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: work in progress

Here's work in progress for a real fix -- in theory, it might work (except for OS/2), but I've tested it so far only on little-endian cairo-gtk2 (i686 Linux).

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

It also seems to work correctly on Mac PPC.  (Well, I'm seeing a slight compositing glitch, but the R, G, and B channels are pretty clearly the right way around, and the fully opaque and fully transparent parts are fine.)

Comment 10

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 250797 [details] [diff] [review]
work in progress

So this isn't quite right because the cairo format wants premultiplied alpha, but this isn't.

Comment 11

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: patch

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 250869 [details] [diff] [review]
patch

Yep, fixing the premultiplication fixed the glitches on Mac, and I can't tell the difference on Linux (although I was looking at very small icons to tell that it worked).

Comment 13

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Fix checked in to trunk.

Comment 14

[Alfred Kayser]

Note, the os2 icon decoder is not 'NOT_IMPLEMENTED' for Cairo, so this needs a separate bug? 

Comment 15

[Alfred Kayser]

'not NOT_IMPLEMENTED' -> 'now NOT_IMPLEMENTED'