Bug 61314 - [Qt] QtWebKit crashes when dragging not loaded images
Summary: [Qt] QtWebKit crashes when dragging not loaded images
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Critical
Assignee: Igor Trindade Oliveira
Keywords: Qt, QtTriaged
Depends on:
Reported: 2011-05-23 15:21 PDT by Igor Trindade Oliveira
Modified: 2011-05-26 08:56 PDT (History)
9 users (show)

See Also:

Test Case (184 bytes, text/html)
2011-05-23 15:22 PDT, Igor Trindade Oliveira
no flags Details
Patch (1.75 KB, patch)
2011-05-23 15:27 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (1.75 KB, patch)
2011-05-23 15:43 PDT, Igor Trindade Oliveira
kling: review-
Details | Formatted Diff | Diff
Patch (4.44 KB, patch)
2011-05-24 15:19 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (4.43 KB, patch)
2011-05-25 07:05 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Igor Trindade Oliveira 2011-05-23 15:21:04 PDT
QtWebKit crashes when dragging not loaded images.
Comment 1 Igor Trindade Oliveira 2011-05-23 15:22:17 PDT
Created attachment 94500 [details]
Test Case

To reproduce the bug try to drag the image when the page is loading.
Comment 2 Igor Trindade Oliveira 2011-05-23 15:27:27 PDT
Created attachment 94502 [details]

Proposed patch
Comment 3 WebKit Review Bot 2011-05-23 15:32:23 PDT
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.
Comment 4 Igor Trindade Oliveira 2011-05-23 15:43:50 PDT
Created attachment 94507 [details]

Proposed patch.
Comment 5 Alexis Menard (darktears) 2011-05-23 16:10:51 PDT
Comment on attachment 94507 [details]

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?
Comment 6 Andreas Kling 2011-05-24 04:30:09 PDT
Comment on attachment 94507 [details]

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());
Comment 7 Igor Trindade Oliveira 2011-05-24 15:19:34 PDT
Created attachment 94702 [details]

Proposed patch. Add test.
Comment 8 Alexis Menard (darktears) 2011-05-25 07:01:10 PDT
Comment on attachment 94702 [details]

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?
Comment 9 Igor Trindade Oliveira 2011-05-25 07:05:40 PDT
Created attachment 94778 [details]

Proposed patch.
Comment 10 WebKit Commit Bot 2011-05-25 09:34:09 PDT
Comment on attachment 94778 [details]

Clearing flags on attachment: 94778

Committed r87298: <http://trac.webkit.org/changeset/87298>
Comment 11 WebKit Commit Bot 2011-05-25 09:34:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Simon Fraser (smfr) 2011-05-25 20:33:22 PDT
This test is crashing on Mac too. Seems like other platforms need a similar fix.
Comment 13 Simon Fraser (smfr) 2011-05-25 20:34:20 PDT
I filed bug 61499.
Comment 14 Antonio Gomes 2011-05-25 20:57:27 PDT
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?
Comment 15 Philippe Normand 2011-05-26 02:30:15 PDT
Filed bug 61513 for GTK
Comment 16 Igor Trindade Oliveira 2011-05-26 03:07:21 PDT
After http://trac.webkit.org/changeset/87366 , mac is not failing anymore.
Comment 17 Ademar Reis 2011-05-26 08:56:47 PDT
Revision r87298 cherry-picked into qtwebkit-2.2 with commit 8cfca23 <http://gitorious.org/webkit/qtwebkit/commit/8cfca23>