Bug 114356 - REGRESSION(r148034): IconDatabase::updatIconRecord crashes with null iconData and iconBitmap.
Summary: REGRESSION(r148034): IconDatabase::updatIconRecord crashes with null iconData...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Raphael Kubo da Costa (:rakuco)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-10 06:53 PDT by Raphael Kubo da Costa (:rakuco)
Modified: 2013-04-11 02:16 PDT (History)
4 users (show)

See Also:

Attachments
Patch (2.04 KB, patch)
2013-04-10 06:57 PDT, Raphael Kubo da Costa (:rakuco) 		thorton: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Raphael Kubo da Costa (:rakuco) 2013-04-10 06:53:59 PDT 
REGRESSION(r148034): IconDatabase::updatIconRecord crashes with null iconData and iconBitmap.
Comment 1 Raphael Kubo da Costa (:rakuco) 2013-04-10 06:57:37 PDT 
Created attachment 197262 [details]
Patch
Comment 2 Tim Horton 2013-04-10 08:45:53 PDT 
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?
Comment 3 Raphael Kubo da Costa (:rakuco) 2013-04-10 09:05:55 PDT 
(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.
Comment 4 Raphael Kubo da Costa (:rakuco) 2013-04-10 09:12:21 PDT 
Committed r148097: <http://trac.webkit.org/changeset/148097>
Comment 5 Darin Adler 2013-04-10 09:42:32 PDT 
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.
Comment 6 Raphael Kubo da Costa (:rakuco) 2013-04-11 02:16:28 PDT 
(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