[PATCH wayland] protocol: Improve data source notification around DnD progress

[Carlos Garnacho]

Hey Michael,

On Thu, Oct 1, 2015 at 6:46 PM, Michael Catanzaro <mcatanzaro at igalia.com> wrote:
> One problem with this model is that a broken or malicious client could
> force the drag source to be leaked by [sending accept and] never
> destroying its data offer... but only once per user-initiated drag, so
> we can probably ignore that issue....

I think there is no way around that, we're in the end coordinating two
clients to send data around, AFAICS there will be always state in one
that should live as long as the other needs the data, no matter how we
lay it. The consolation is that it happens after .drop_performed, so
the source-side client will be in an state where it can start other
dnd operations, and maybe drop the previous data source.

FWIW, (I'm sure it's not flash news) XDND has a similar flaw if the
drag destination fails to send XdndFinished.

>
> On Thu, 2015-10-01 at 12:00 +0200, Carlos Garnacho wrote:
>> This is in order to be able to emit .cancelled if there is no
>> agreement between source and destination about the data to be
>> transferred. Otherwise every drop on any client with a wl_data_device
>> would seem successful.
>
> Ah, so the goal is to allow the drag source to know if the drop
> succeeded. The only solution I can think of that avoids the problem I
> mention is to delay the canceled event until the data offer is
> destroyed. Then there is no race, and the drag source must simply wait
> until drag_finished or a later drag_cancelled if it wants to indicate
> to the user if the drag succeeded or not (the drop success indication
> would be delayed until the data transfer is completed). I don't see any
> problems with such a scheme, but if you can find one, then I think we'd
> have to go with your solution and just say that my problem is a client
> bug.

Hmm, that would mean that .drop_performed would need to happen on both
.cancelled and .drag_finished cases, because at the moment of dropping
you don't know whether some or no mimetype/action will be accepted in
the end. I also wonder if postponing the success/failure indications
until the transfer is complete is a good idea, those can take an
arbitrary time (even if usually short enough).

To add up to the mess, there's one situation I can think where
feedback is different from the real outcome at wl_data_offer destroy
time, popping up a menu on drop with a "cancel" option (or dismissed
through other means), you certainly wouldn't want the UI feedback to
wait until you pick an option or close the menu.

But this is a very valid concern, if you preemptively accept() with a
random mimetype, the user quickly finishes the drag before transfers
are done, and it isn't accepted in the end, you'd get the wrong
feedback, and the wrong outcome in move operations, where the source
may think it's ok to delete the data. The other option is not
accept()ing until the first transfer is done like you do right now,
and train the user to wait long enough on huge dnd transfers, so early
drops turn out cancelled. There'd usually be at least cursor feedback
from the drag source when accept() finally happens. Both options are
somewhat disconcerting, I admit...

>
>> I know .accept has had no functional role in the DnD state machine
>> till now (besides UI feedback on the source-side, setting the
>> appropriate cursor/dnd surfaces mostly), it made sense to me to reuse
>> this though because compositor-side UI feedback and overall DnD
>> outcome would be in sync for virtually every usecase I can imagine. I
>> would expect .accept() to be called regularly on DnD motion over a
>> surface, the pointer moves across inner widgets that accept different
>> mimetypes or none.
>
> Chrome does call it regularly on DnD motion, but only beginning after
> it has received all the data.
>
>> Actually, IMO there should be checks somewhere about the mimetype
>> being one of the offered ones, in order to behave properly on wrong
>> requests, although that means keeping the mimetype list in memory
>> instead of just forwarding it.
>
> If you mean that the mime type sent in accept or receive requests
> should always be one of the offered mime types, then having the
> compositor check for that is a fine idea that can't possibly hurt
> anything.

Yeah, that's what I meant.

>
>> However, I can imagine other usecases where we don't know any
>> concrete
>> mimetype beforehand, eg. a "paste as html/plain text" popup menu on
>> .drop_performed, there'd be no other way in these cases than
>> preemptively picking one, as you say in your other mail.
>
> Deciding later which type of data to actually use doesn't require any
> interaction with the source process if the data has already been
> received (as is the case for Chrome), and if the data has not already
> been received, the receive request must specify an offered mime type.
> In fact, Chrome's DnD backend doesn't ever know anything about which
> mime type will actually be used (and in accept messages, it just sends
> back whatever the first offered mime type was, fdo#91950).
>
> Now, verifying that receive requests use a previously-accepted mime
> type would be problematic for Chrome, since we receive and then accept
> instead of vice-versa, but I don't think that's what you're proposing?

No :), receive() may happen mid-DnD as you say, and isn't indicative
of a successful drop.

>
>>  So perhaps it
>> makes sense to split into a request with a boolean after all?
>> Nonetheless, for the case of GTK+, both would be called from the same
>> place (and .accept is already invariably called with the first
>> mimetype from the list, hmm...)
>
> Can you clarify your idea? I am a bit lost now. :)

I meant, adding a separate .set_droppable() request with a boolean
parameter that indicates success, so .accept()'s role remains
unchanged. There's the downside that you decouple source-side feedback
from the rest of the DnD operation outcome (compositor-side feedback
and events emitted). Again, this is something that happens on XDND,
eg. by acknowledging one status on XdndStatus and finishing with
another one on XdndFinished, feels like late 90's again :).

Cheers,
  Carlos

>
> Michael