Bug 113126

Summary: [Qt] editing/pasteboard/can-read-in-dragstart-event.html and /can-read-in-copy-and-cut-events.html are crashing
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Daniel Cheng <dcheng>
Status: RESOLVED FIXED    
Severity: Normal CC: abrhm, allan.jensen, dcheng, kadam, ossy, tony, zarvai
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79666    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch jturcotte: review+

Attachments
Patch (4.61 KB, patch)
2013-03-23 18:07 PDT, Daniel Cheng 		no flags
DetailsFormatted DiffDiff
Patch (7.77 KB, patch)
2013-03-25 03:52 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (8.23 KB, patch)
2013-03-26 09:52 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (8.23 KB, patch)
2013-03-26 10:37 PDT, Allan Sandfeld Jensen 		jturcotte: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2013-03-22 21:57:17 PDT
Committed r146713: <http://trac.webkit.org/changeset/146713>
Daniel Cheng
Comment 2 2013-03-22 23:19:02 PDT
Is there any way to get symbolized backtraces from the bots?
Ryosuke Niwa
Comment 3 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]
Ryosuke Niwa
Comment 4 2013-03-22 23:23:52 PDT
Maybe Ossy can help you.
Daniel Cheng
Comment 5 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).
Ryosuke Niwa
Comment 6 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.
Ryosuke Niwa
Comment 7 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.
Daniel Cheng
Comment 8 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.
Ryosuke Niwa
Comment 9 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...
Daniel Cheng
Comment 10 2013-03-23 18:07:56 PDT
Created attachment 194724 [details] Patch
Ryosuke Niwa
Comment 11 2013-03-23 18:20:02 PDT
Comment on attachment 194724 [details] Patch Thanks!
Daniel Cheng
Comment 12 2013-03-23 18:27:04 PDT
Committed r146723: <http://trac.webkit.org/changeset/146723>
Allan Sandfeld Jensen
Comment 13 2013-03-24 08:23:55 PDT
I will take a look.
Allan Sandfeld Jensen
Comment 14 2013-03-25 03:52:52 PDT
Created attachment 194820 [details] Patch
Allan Sandfeld Jensen
Comment 15 2013-03-26 09:52:15 PDT
Created attachment 195102 [details] Patch
Jocelyn Turcotte
Comment 16 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.
Allan Sandfeld Jensen
Comment 17 2013-03-26 10:37:35 PDT
Created attachment 195112 [details] Patch
Allan Sandfeld Jensen
Comment 18 2013-03-26 11:12:39 PDT
Committed r146910: <http://trac.webkit.org/changeset/146910>
Gábor Ábrahám
Comment 19 2013-06-13 06:20:24 PDT
editing/pasteboard/can-read-in-dragstart-event.html test timeout on wk1.
Zoltan Arvai
Comment 20 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
Note You need to log in before you can comment on or make changes to this bug.