[Qt] QtWebKit crashes when dragging not loaded images

[Igor Trindade Oliveira]

QtWebKit crashes when dragging not loaded images.

[Igor Trindade Oliveira]

Created attachment 94500 [details]
Test Case

To reproduce the bug try to drag the image when the page is loading.

[Igor Trindade Oliveira]

Created attachment 94502 [details]
Patch

Proposed patch

[WebKit Review Bot]

Attachment 94502 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/qt/DragImageQt.cpp:66:  Declaration has space between type name and * in QPixmap *dragImage  [whitespace/declaration] [3]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Igor Trindade Oliveira]

Created attachment 94507 [details]
Patch

Proposed patch.

[Alexis Menard (darktears)]

Comment on attachment 94507 [details]
Patch

Looks good but can you do a layout test? I know drag and drop are tricky but is there any existing drag and drop tests you could reuse?

[Andreas Kling]

Comment on attachment 94507 [details]
Patch

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

Good catch! Needs a test though.

> Source/WebCore/platform/qt/DragImageQt.cpp:70
> -    if (!image)
> -        return 0;
> +    QPixmap* dragImage = 0;
> +    if (image && image->nativeImageForCurrentFrame())
> +        dragImage = new QPixmap(*image->nativeImageForCurrentFrame());
>  
> -    return new QPixmap(*image->nativeImageForCurrentFrame());
> +    return dragImage;

This would read better as:

if (!image || !image->nativeImageForCurrentFrame())
    return 0;
return new QPixmap(*image->nativeImageForCurrentFrame());

[Igor Trindade Oliveira]

Created attachment 94702 [details]
Patch

Proposed patch. Add test.

[Alexis Menard (darktears)]

Comment on attachment 94702 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        Checks if WebKit crashes when dragging a not loaded image.

dragging an image not yet loaded. better no?

[Igor Trindade Oliveira]

Created attachment 94778 [details]
Patch

Proposed patch.

[WebKit Commit Bot]

Comment on attachment 94778 [details]
Patch

Clearing flags on attachment: 94778

Committed r87298: <http://trac.webkit.org/changeset/87298>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This test is crashing on Mac too. Seems like other platforms need a similar fix.

[Simon Fraser (smfr)]

I filed bug 61499.

[Antonio Gomes]

It was failing on Qt and no bot email/bug_comment was added to this bug, as it usually happen. Failing on Mac too (according to Simon and nothing yet from the emailer bot). Maybe there is a problem with them?

[Philippe Normand]

Filed bug 61513 for GTK

[Igor Trindade Oliveira]

After http://trac.webkit.org/changeset/87366 , mac is not failing anymore.

[Ademar Reis]

Revision r87298 cherry-picked into qtwebkit-2.2 with commit 8cfca23 <http://gitorious.org/webkit/qtwebkit/commit/8cfca23>