Bug 114356 – REGRESSION(r148034): IconDatabase::updatIconRecord crashes with null iconData and iconBitmap.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Raphael Kubo da Costa (:rakuco)]

REGRESSION(r148034): IconDatabase::updatIconRecord crashes with null iconData and iconBitmap.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 197262 [details]
Patch

[Tim Horton]

Comment on attachment 197262 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=197262&action=review

> Source/WebCore/loader/icon/IconDatabase.cpp:541
> +    ASSERT(!(!!iconData && !!iconBitmap));

!! is a bit bizarre but there are other instances of it around. But I wonder why && alone isn't enough?

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #2)
> (From update of attachment 197262 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=197262&action=review
> 
> > Source/WebCore/loader/icon/IconDatabase.cpp:541
> > +    ASSERT(!(!!iconData && !!iconBitmap));
> 
> !! is a bit bizarre but there are other instances of it around. But I wonder why && alone isn't enough?

I was just being extra-cautious here; PassRefPtr::operator!() was casting them into bools, and I'm adding an extra ! to get the "original" boolean value.

[Raphael Kubo da Costa (:rakuco)]

Committed r148097: <http://trac.webkit.org/changeset/148097>

[Darin Adler]

Comment on attachment 197262 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=197262&action=review

>>> Source/WebCore/loader/icon/IconDatabase.cpp:541
>>> +    ASSERT(!(!!iconData && !!iconBitmap));
>> 
>> !! is a bit bizarre but there are other instances of it around. But I wonder why && alone isn't enough?
> 
> I was just being extra-cautious here; PassRefPtr::operator!() was casting them into bools, and I'm adding an extra ! to get the "original" boolean value.

No, those !! are not needed unless there is a bug in PassRefPtr. The && operator should work fine with two PassRefPtr values without requiring the ! at all.

It would be nice to remove those.

[Raphael Kubo da Costa (:rakuco)]

(In reply to comment #5)
> (From update of attachment 197262 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=197262&action=review
> 
> >>> Source/WebCore/loader/icon/IconDatabase.cpp:541
> >>> +    ASSERT(!(!!iconData && !!iconBitmap));
> >> 
> >> !! is a bit bizarre but there are other instances of it around. But I wonder why && alone isn't enough?
> > 
> > I was just being extra-cautious here; PassRefPtr::operator!() was casting them into bools, and I'm adding an extra ! to get the "original" boolean value.
> 
> No, those !! are not needed unless there is a bug in PassRefPtr. The && operator should work fine with two PassRefPtr values without requiring the ! at all.
> 
> It would be nice to remove those.

https://bugs.webkit.org/show_bug.cgi?id=114425