1368999 - remove or fix WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: toolkit/components/places/nsFaviconService.cpp, line 776

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P3)

Description

[Honza Bambas (:mayhemer)]

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: toolkit/components/places/nsFaviconService.cpp, line 776
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: toolkit/components/places/nsFaviconService.cpp, line 472
WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: toolkit/components/places/nsFaviconService.cpp, line 582

Comment 1

[Honza Bambas (:mayhemer)]

m-c@39d5cc0fda5e

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I was looking into this the other day, I need to understand which icon causes them and I couldn't reproduce it consistently enough yet. It's possible we have an invalid icon in the product code, since the warnings come from imgTools being unable to convert a payload.
These warnings are positive, they are showing an issue, so we should fix the issue rather than removing them.  Indeed I didn't rush a conversion of Places from NS_ENSURE_SUCCESS yet, since in the last years we got far more bugs fixed thanks to these warnings than false positives.

I'll try to have a look at this once release priorities are gone.

Comment 3

[Honza Bambas (:mayhemer)]

I was able to repro on windows with a debug build with:

./mach test toolkit/components/mozprotocol/tests/browser_mozprotocol.js

and a local patch applied (not sure you need it to repro).  If you can't find it, I can give some more info.

Comment 4

[Honza Bambas (:mayhemer)]

I think just running the test infra triggers these NS_ENSURE failures.  I can see them before the first TEST-START.  No local patches.

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

LOL, the problem is STILL the icon I thought to have fixed in bug 1346736!
While I fixed a typo in the mimeType, there's more there, the payload is not an ico at all, it's a png.
Changing that to image/png solves the warnings as well. Since the icon lies to us, we try to decode it as an ico, not a png, and the decoding fails.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Should now be fixed.

Comment 7

[Honza Bambas (:mayhemer)]

Thanks!