Bug 102192 - [Qt] Crash in PasteboardQt.cpp Pasteboard::writeSelection
Summary: [Qt] Crash in PasteboardQt.cpp Pasteboard::writeSelection
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Allan Sandfeld Jensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-14 01:06 PST by Fabien Vallée
Modified: 2013-01-22 07:04 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.70 KB, patch)
2013-01-21 09:25 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (2.30 KB, patch)
2013-01-22 05:40 PST, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabien Vallée 2012-11-14 01:06:29 PST
Tested with Qt 4.8. 
Running layoutTests with child-processes=4, pasteboard related tests (e.g. editing/pasteboard/4944770-2.html) crash (randomly) in Pasteboard::writeSelection.
(it happens when several tests access simultaneously the clipboard).

The crash is at the line :
md->setData(QLatin1String("application/vnd.qtwebkit.smartpaste"), QByteArray());   ( Source/WebCore/platform/qt/PasteboardQt.cpp@134566 )


I believe there is a bug in  Pasteboard::writeSelection. The function logic is :

1) creates a new QMimeData object (raw pointer)
2) set its content (setHtml, ...)
3) gives the QMimeData raw pointer to the QGuiApplication::clipboard() ( QGuiApplication::clipboard()->setMimeData).
4) set the QMimeData data for smartpaste on the raw pointer

Pasteboard::writeSelection lost the ownership on the QMimeData object in step 3), therefore it should not access the object in 4).
Step 4) shall be done before step 3)

The following fix should be enough to fix the issue :

--- a/Source/WebCore/platform/qt/PasteboardQt.cpp
+++ b/Source/WebCore/platform/qt/PasteboardQt.cpp
@@ -74,11 +74,11 @@ void Pasteboard::writeSelection(Range* selectedRange, bool canSmartCopyOrDelete,
     md->setHtml(markup);
 #endif
 
+    if (canSmartCopyOrDelete)
+        md->setData(QLatin1String("application/vnd.qtwebkit.smartpaste"), QByteArray());
 #ifndef QT_NO_CLIPBOARD
     QGuiApplication::clipboard()->setMimeData(md, m_selectionMode ? QClipboard::Selection : QClipboard::Clipboard);
 #endif
-    if (canSmartCopyOrDelete)
-        md->setData(QLatin1String("application/vnd.qtwebkit.smartpaste"), QByteArray());
 }
Comment 1 Allan Sandfeld Jensen 2013-01-21 09:19:38 PST
I can not trigger the crash, but we have transfered ownership, so the current order must be wrong.
Comment 2 Allan Sandfeld Jensen 2013-01-21 09:25:36 PST
Created attachment 183797 [details]
Patch
Comment 3 Jocelyn Turcotte 2013-01-22 03:26:40 PST
Comment on attachment 183797 [details]
Patch

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

Could you do the same modification in Pasteboard::writePlainText as well?

> Source/WebCore/ChangeLog:9
> +        QClipboard::setData transfers ownership of QMimeData to the clipboard,

QClipboard::setMimeData
Comment 4 Allan Sandfeld Jensen 2013-01-22 05:40:33 PST
Created attachment 183972 [details]
Patch
Comment 5 Jocelyn Turcotte 2013-01-22 06:28:12 PST
Comment on attachment 183972 [details]
Patch

Overall comments
Comment 6 WebKit Review Bot 2013-01-22 07:04:43 PST
Comment on attachment 183972 [details]
Patch

Clearing flags on attachment: 183972

Committed r140423: <http://trac.webkit.org/changeset/140423>
Comment 7 WebKit Review Bot 2013-01-22 07:04:49 PST
All reviewed patches have been landed.  Closing bug.