1527951 - Assertion failure: mFormat == SurfaceFormat::B8G8R8A8 || src[3] == 0xFF

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[Bob Clary [:bc] (inactive)]

1. https://www.goodsearch.org/

2. Assertion failure: mFormat == SurfaceFormat::B8G8R8A8 || src[3] == 0xFF, at z:/build/build/src/image/decoders/nsWebPDecoder.cpp:492
   #01: mozilla::image::nsWebPDecoder::ReadHeader(WebPDemuxer *,bool) [image/decoders/nsWebPDecoder.cpp:399]
   #02: mozilla::image::nsWebPDecoder::ReadData() [image/decoders/nsWebPDecoder.cpp:0]
   #03: mozilla::image::nsWebPDecoder::UpdateBuffer(mozilla::image::SourceBufferIterator &,mozilla::image::SourceBufferIterator::State) [image/decoders/nsWebPDecoder.cpp:0]
   #04: mozilla::image::nsWebPDecoder::DoDecode(mozilla::image::SourceBufferIterator &,mozilla::image::IResumable *) [image/decoders/nsWebPDecoder.cpp:119]
   #05: mozilla::image::Decoder::Decode(mozilla::image::IResumable *) [image/Decoder.cpp:125]
   #06: mozilla::image::DecodedSurfaceProvider::Run() [image/DecodedSurfaceProvider.cpp:125]
   #07: mozilla::image::DecodePoolWorker::Run() [image/DecodePool.cpp:266]
   #08: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1149]

Windows/Linux. From the original webp landing. Doesn't crash opt from what I can tell.

Comment 1

[Bob Clary [:bc] (inactive)]

This image reproduces the assertion:

https://res.cloudinary.com/goodsearch/image/upload/c_pad,f_auto,h_70,q_auto,w_70/v1452552640/hi_resolution_merchant_logos/ashley-furniture_coupons.jpg

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

The assertion itself is probably harmless, but I think we can trigger release asserts in skia because of this. We should just ignore the alpha channel from the decoder if the surface is BGRX.

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1527951 - Ignore WebP image's alpha channel when a frame is marked as opaque.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Andrew Osmond [:aosmond] from comment #2)

The assertion itself is probably harmless, but I think we can trigger release asserts in skia because of this. We should just ignore the alpha channel from the decoder if the surface is BGRX.

With WebRender, it causes us to be completely transparent on Linux -- it shows the windows/desktop underneath the image. Very confusing!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1c887a569595
Ignore WebP image's alpha channel when a frame is marked as opaque. r=tnikkel

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/1c887a569595

Comment 7

[Ryan VanderMeulen [:RyanVM]]

I'm assuming this can ride the trains, but feel free to nominate for Beta approval if there's a user impact which justifies consideration.

Comment 8

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9044293 [details]
Bug 1527951 - Ignore WebP image's alpha channel when a frame is marked as opaque.

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1294490
• User impact if declined: May experience rare (release assertion) crash or visual distortion with malformed WebP images.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is not risky because we now explicitly set the alpha channel to be opaque instead of relying upon libwebp to give us a sane value. It produces more restrictive output than before, and is thus more predictable. I also added tests which explicitly test this condition.
• String changes made/needed: N/A

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

I can verify this works as expected in nightly since the patch landed. Images like:

https://hg.mozilla.org/integration/mozilla-inbound/raw-file/1c887a569595/image/test/gtest/transparent-no-alpha-header.webp

should be all black now, which it is.

Comment 10

[Liz Henry (:lizzard)]

Comment on attachment 9044293 [details]
Bug 1527951 - Ignore WebP image's alpha channel when a frame is marked as opaque.

Fix verified in nightly, has test coverage.
OK for uplift for beta 12.

Comment 11

[Razvan Maries]

https://hg.mozilla.org/releases/mozilla-beta/rev/aecc839fd41c