D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-23 Thread Martin Flöser
graesslin abandoned this revision. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7078 To: graesslin, #kwin, #plasma, #frameworks Cc: cfeck, anthonyfieroni, davidedmundson, plasma-devel, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-23 Thread David Edmundson
davidedmundson added a comment. I'm still against it. Passing unexpectedly invalid pointers about all over the place and then trying to guard them afterwards is an anti pattern that leads to future problems. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7078

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-23 Thread Martin Flöser
graesslin added a comment. I'm still in favor of pushing this one too. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7078 To: graesslin, #kwin, #plasma, #frameworks Cc: cfeck, anthonyfieroni, davidedmundson, plasma-devel, leezu, ZrenBot, progwolff, lesliezhai,

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-22 Thread Christoph Feck
cfeck added a comment. https://phabricator.kde.org/D7316 has been committed, and the referenced bug marked as fixed. Reading above comments, this patch can/should be committed, too. Please check if this is still true, and either approve this patch, or discard it. REPOSITORY R127

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-14 Thread Martin Flöser
graesslin added a comment. > If your patch works, then mine would too. It's checking the exact same pointer, just one frame earlier. But then it's an easy thing: let's do both. Your patch and mine. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7078 To:

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-14 Thread David Edmundson
davidedmundson added a comment. > of course we do that :-) We're not. Kwin does not check if a client has a selection. It merely checks if the client has an interface from a selection can be fetched, it does not check that device actually has a valid selection. > you think

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-13 Thread Anthony Fieroni
anthonyfieroni added a comment. But what is point to have a DataOfferInterface without valid source? REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7078 To: graesslin, #kwin, #plasma, #frameworks Cc: anthonyfieroni, davidedmundson, plasma-devel, leezu, ZrenBot,

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-13 Thread Martin Flöser
graesslin added a comment. In https://phabricator.kde.org/D7078#135036, @davidedmundson wrote: > > So from KWayland Server side we have a selection > > from KWayland server side we have a Seat:selection, we don't check if that has a DDI::selection > > > > If a client

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-12 Thread David Edmundson
davidedmundson added a comment. > So from KWayland Server side we have a selection from KWayland server side we have a Seat:selection, we don't check if that has a DDI::selection If a client had called wl_set_selection(dataDevice, nulltptr) the server would still have a

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-09 Thread Martin Flöser
graesslin added a comment. > the send_selection spec says we should be passing a null resource if we have no selection, whereas this version creates a data offer with nothing in it. But we have a selection, that's the point. We just don't have a datasource on the selection.

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-09 Thread David Edmundson
davidedmundson added a comment. I don't think we did go in there because we have a selection, otherwise other->selection() wouldn't be null. If we want DDI::sendSelection(DDI *other) to always keep the client in sync we should do: auto selection = other->selection(); if

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-09 Thread Martin Flöser
graesslin added a comment. In https://phabricator.kde.org/D7078#134111, @davidedmundson wrote: > Isn't the real bug from that trace here: > > datadevice_interface.cpp:204 > void DataDeviceInterface::sendSelection(DataDeviceInterface *other) >

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-09 Thread David Edmundson
davidedmundson added a comment. Isn't the real bug from that trace here: datadevice_interface.cpp:204 void DataDeviceInterface::sendSelection(DataDeviceInterface *other) d->createDataOffer(other->selection()); it's valid for other->selection() to be null, so this should be

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-09 Thread Martin Flöser
graesslin added a comment. ping! This is a crash fix. REPOSITORY R127 KWayland REVISION DETAIL https://phabricator.kde.org/D7078 To: graesslin, #kwin, #plasma, #frameworks Cc: plasma-devel, leezu, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol,

D7078: [server] Fix crash when sending selection to a DDI without a DataSource

2017-08-02 Thread Martin Flöser
graesslin created this revision. Restricted Application added projects: Plasma on Wayland, Frameworks. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY This crash got triggered in KWin when sending the selection to XWayland. While the test handles condition of not yet