Bug 113126 - [Qt] editing/pasteboard/can-read-in-dragstart-event.html and /can-read-in-copy-and-cut-events.html are crashing
Summary: [Qt] editing/pasteboard/can-read-in-dragstart-event.html and /can-read-in-cop...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Cheng
URL:
Keywords:
Depends on:
Blocks: 79666
  Show dependency treegraph
 
Reported: 2013-03-22 21:54 PDT by Ryosuke Niwa
Modified: 2013-06-13 06:52 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.61 KB, patch)
2013-03-23 18:07 PDT, Daniel Cheng
no flags Details | Formatted Diff | Diff
Patch (7.77 KB, patch)
2013-03-25 03:52 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (8.23 KB, patch)
2013-03-26 09:52 PDT, Allan Sandfeld Jensen
no flags Details | Formatted Diff | Diff
Patch (8.23 KB, patch)
2013-03-26 10:37 PDT, Allan Sandfeld Jensen
jturcotte: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Ryosuke Niwa 2013-03-22 21:57:17 PDT
Committed r146713: <http://trac.webkit.org/changeset/146713>
Comment 2 Daniel Cheng 2013-03-22 23:19:02 PDT
Is there any way to get symbolized backtraces from the bots?
Comment 3 Ryosuke Niwa 2013-03-22 23:23:31 PDT
The best one I can give you is:

http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Debug/r146711%20(28239)/results.html

crash log for DumpRenderTree (pid 10543):
STDOUT: <empty>
STDERR: ASSERTION FAILED: m_readableData
STDERR: /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/Source/WebCore/platform/qt/ClipboardQt.cpp(190) : virtual WTF::ListHashSet<WTF::String> WebCore::ClipboardQt::types() const
STDERR: 1   0x7f8a4bff549f /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x11f649f) [0x7f8a4bff549f]
STDERR: 2   0x7f8a4b3a7b74 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x5a8b74) [0x7f8a4b3a7b74]
STDERR: 3   0x7f8a4c415c73 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x1616c73) [0x7f8a4c415c73]
STDERR: 4   0x7f8a4b384651 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x585651) [0x7f8a4b384651]
STDERR: 5   0x7f8a4b3f215d /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x5f315d) [0x7f8a4b3f215d]
STDERR: 6   0x7f8a4cb49f34 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x1d4af34) [0x7f8a4cb49f34]
STDERR: 7   0x7f8a4cb53791 /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/lib/libQt5WebKit.so.5(+0x1d54791) [0x7f8a4cb53791]
Comment 4 Ryosuke Niwa 2013-03-22 23:23:52 PDT
Maybe Ossy can help you.
Comment 5 Daniel Cheng 2013-03-23 10:52:45 PDT
Oh I see... thanks for the symbolized trace!

It's because the Qt clipboard expects to have a m_readableData always when it attempts to read, but this is not set when a mutable ClipboardQt is created (in dragstart/copy/cut events).

The proper fix here is to just combine m_readableData/m_writableData into m_mimeData and just use that for all operations. Until then, a crash is the expected result due to the null dereference (and assert).
Comment 6 Ryosuke Niwa 2013-03-23 12:11:10 PDT
(In reply to comment #5)
> The proper fix here is to just combine m_readableData/m_writableData into m_mimeData and just use that for all operations. Until then, a crash is the expected result due to the null dereference (and assert).

That doesn't sound right. A program crash should NEVER be an expected behavior. Did this crash exist your patch? If not, this is a regression.
Comment 7 Ryosuke Niwa 2013-03-23 12:21:10 PDT
That doesn't sound right. A program crash should NEVER be an expected behavior. Did this crash exist *before* your patch? If not, this is a regression.
Comment 8 Daniel Cheng 2013-03-23 17:05:52 PDT
Hmm, you're right. As a temporary workaround until someone more familiar can implement the work, I will change the ASSERTs to early returns.
Comment 9 Ryosuke Niwa 2013-03-23 17:21:46 PDT
(In reply to comment #8)
> Hmm, you're right. As a temporary workaround until someone more familiar can implement the work, I will change the ASSERTs to early returns.

Great. Thanks. Meanwhile, we need to find a Qt port contributor who can address issue...
Comment 10 Daniel Cheng 2013-03-23 18:07:56 PDT
Created attachment 194724 [details]
Patch
Comment 11 Ryosuke Niwa 2013-03-23 18:20:02 PDT
Comment on attachment 194724 [details]
Patch

Thanks!
Comment 12 Daniel Cheng 2013-03-23 18:27:04 PDT
Committed r146723: <http://trac.webkit.org/changeset/146723>
Comment 13 Allan Sandfeld Jensen 2013-03-24 08:23:55 PDT
I will take a look.
Comment 14 Allan Sandfeld Jensen 2013-03-25 03:52:52 PDT
Created attachment 194820 [details]
Patch
Comment 15 Allan Sandfeld Jensen 2013-03-26 09:52:15 PDT
Created attachment 195102 [details]
Patch
Comment 16 Jocelyn Turcotte 2013-03-26 10:00:27 PDT
Comment on attachment 195102 [details]
Patch

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

> Source/WebCore/platform/qt/ClipboardQt.cpp:146
>      if (!canReadData())
>          return String();

This can be merged into the below if (!data).

> Source/WebCore/platform/qt/ClipboardQt.cpp:187
>      if (!canReadTypes())
>          return ListHashSet<String>();

ditto

> Source/WebCore/platform/qt/ClipboardQt.cpp:203
> +    if (!canReadData())
> +        return FileList::create();

ditto

> Source/WebCore/platform/qt/ClipboardQt.cpp:347
> +const QMimeData* ClipboardQt::readClipboardData() const

Nit: I'm not sure about the name, it sounds like it's actively fetching data from the clipboard.
Comment 17 Allan Sandfeld Jensen 2013-03-26 10:37:35 PDT
Created attachment 195112 [details]
Patch
Comment 18 Allan Sandfeld Jensen 2013-03-26 11:12:39 PDT
Committed r146910: <http://trac.webkit.org/changeset/146910>
Comment 19 Gábor Ábrahám 2013-06-13 06:20:24 PDT
editing/pasteboard/can-read-in-dragstart-event.html test timeout on wk1.
Comment 20 Zoltan Arvai 2013-06-13 06:52:31 PDT
(In reply to comment #19)
> editing/pasteboard/can-read-in-dragstart-event.html test timeout on wk1.

Yeah, it is skipped and moved to "missing drag-and-drop support" section in
http://trac.webkit.org/changeset/146910/trunk/LayoutTests/platform/qt/TestExpectations