Reviewed-by: Michael Catanzaro <mcatanzaro at igalia.com> Looks mostly good to me. The only problem I see is that you've left stale conditionals in both data_offer_accept and data_offer_receive, which can't be hit anymore since you added the bails up above. Be sure that fd gets freed in data_offer_receive when it's not sent to the drop source; you've introduced a leak there.
On Wed, 2015-09-30 at 22:49 +0200, Carlos Garnacho wrote: > Weston now sends wl_data_source.drop_performed and .drag_finished in > order to notify about the different phases of DnD. > > wl_data_source.cancelled is also used as mentioned in the docs, being > emitted also on DnD when the operation is meant to fail (eg. source > and dest didn't agree on a mimetype). > > The dnd demo is also fixed so the struct dnd_drag isn't leaked. > > https://bugs.freedesktop.org/show_bug.cgi?id=91943 > https://bugs.freedesktop.org/show_bug.cgi?id=91944 > > Signed-off-by: Carlos Garnacho <carl...@gnome.org> > --- > clients/dnd.c | 39 +++++++++++++++++++++++++++++++-------- > clients/window.c | 2 +- > src/compositor.h | 2 ++ > src/data-device.c | 41 +++++++++++++++++++++++++++++++++++------ > 4 files changed, 69 insertions(+), 15 deletions(-) > > diff --git a/clients/dnd.c b/clients/dnd.c > index e6a0c39..6c2ed57 100644 > --- a/clients/dnd.c > +++ b/clients/dnd.c > @@ -318,14 +318,8 @@ data_source_send(void *data, struct > wl_data_source *source, > } > > static void > -data_source_cancelled(void *data, struct wl_data_source *source) > +destroy_dnd_drag(struct dnd_drag *dnd_drag) > { > - struct dnd_drag *dnd_drag = data; > - > - /* The 'cancelled' event means that the source is no longer > in > - * use by the drag (or current selection). We need to clean > - * up the drag object created and the local state. */ > - > wl_data_source_destroy(dnd_drag->data_source); > > /* Destroy the item that has been dragged out */ > @@ -339,10 +333,39 @@ data_source_cancelled(void *data, struct > wl_data_source *source) > free(dnd_drag); > } > > +static void > +data_source_cancelled(void *data, struct wl_data_source *source) > +{ > + struct dnd_drag *dnd_drag = data; > + > + /* The 'cancelled' event means that the source is no longer > in > + * use by the drag (or current selection). We need to clean > + * up the drag object created and the local state. */ > + destroy_dnd_drag(dnd_drag); > +} > + > +static void > +data_source_drop_performed(void *data, struct wl_data_source > *source) > +{ > +} > + > +static void > +data_source_drag_finished(void *data, struct wl_data_source *source) > +{ > + struct dnd_drag *dnd_drag = data; > + > + /* The operation is already finished, we can destroy all > + * related data. > + */ > + destroy_dnd_drag(dnd_drag); > +} > + > static const struct wl_data_source_listener data_source_listener = { > data_source_target, > data_source_send, > - data_source_cancelled > + data_source_cancelled, > + data_source_drop_performed, > + data_source_drag_finished, > }; > > static cairo_surface_t * > diff --git a/clients/window.c b/clients/window.c > index 47a79aa..feb9515 100644 > --- a/clients/window.c > +++ b/clients/window.c > @@ -5353,7 +5353,7 @@ registry_handle_global(void *data, struct > wl_registry *registry, uint32_t id, > d->shm = wl_registry_bind(registry, id, > &wl_shm_interface, 1); > wl_shm_add_listener(d->shm, &shm_listener, d); > } else if (strcmp(interface, "wl_data_device_manager") == 0) > { > - d->data_device_manager_version = MIN(version, 2); > + d->data_device_manager_version = MIN(version, 3); > d->data_device_manager = > wl_registry_bind(registry, id, > &wl_data_device_manager_int > erface, > diff --git a/src/compositor.h b/src/compositor.h > index c4c81f0..40c02c2 100644 > --- a/src/compositor.h > +++ b/src/compositor.h > @@ -306,6 +306,8 @@ struct weston_data_source { > struct wl_resource *resource; > struct wl_signal destroy_signal; > struct wl_array mime_types; > + struct weston_data_offer *offer; > + int accepted; > > void (*accept)(struct weston_data_source *source, > uint32_t serial, const char *mime_type); > diff --git a/src/data-device.c b/src/data-device.c > index 1612091..c5aac55 100644 > --- a/src/data-device.c > +++ b/src/data-device.c > @@ -62,12 +62,18 @@ data_offer_accept(struct wl_client *client, > struct wl_resource *resource, > { > struct weston_data_offer *offer = > wl_resource_get_user_data(resource); > > + /* Protect against untimely calls from older data offers */ > + if (!offer->source || offer != offer->source->offer) > + return; > + > /* FIXME: Check that client is currently focused by the > input > * device that is currently dragging this data source. > Should > * this be a wl_data_device request? */ > > - if (offer->source) > + if (offer->source) { > offer->source->accept(offer->source, serial, > mime_type); > + offer->source->accepted = mime_type != NULL; > + } > } > > static void > @@ -76,6 +82,10 @@ data_offer_receive(struct wl_client *client, > struct wl_resource *resource, > { > struct weston_data_offer *offer = > wl_resource_get_user_data(resource); > > + /* Protect against untimely calls from older data offers */ > + if (!offer->source || offer != offer->source->offer) > + return; > + > if (offer->source) > offer->source->send(offer->source, mime_type, fd); > else > @@ -99,8 +109,14 @@ destroy_data_offer(struct wl_resource *resource) > { > struct weston_data_offer *offer = > wl_resource_get_user_data(resource); > > - if (offer->source) > + if (offer->source) { > + if (offer->source->offer == offer) { > + wl_data_source_send_drag_finished(offer > ->source->resource); > + offer->source->offer = NULL; > + } > + > wl_list_remove(&offer > ->source_destroy_listener.link); > + } > free(offer); > } > > @@ -147,6 +163,9 @@ weston_data_source_send_offer(struct > weston_data_source *source, > wl_array_for_each(p, &source->mime_types) > wl_data_offer_send_offer(offer->resource, *p); > > + source->offer = offer; > + source->accepted = 0; > + > return offer->resource; > } > > @@ -301,6 +320,7 @@ weston_drag_set_focus(struct weston_drag *drag, > serial = wl_display_next_serial(display); > > if (drag->data_source) { > + drag->data_source->accepted = 0; > offer = weston_data_source_send_offer(drag > ->data_source, > resource); > if (offer == NULL) > @@ -398,9 +418,16 @@ drag_grab_button(struct weston_pointer_grab > *grab, > enum wl_pointer_button_state state = state_w; > > if (drag->base.focus_resource && > + drag->base.data_source && > pointer->grab_button == button && > - state == WL_POINTER_BUTTON_STATE_RELEASED) > - wl_data_device_send_drop(drag->base.focus_resource); > + state == WL_POINTER_BUTTON_STATE_RELEASED) { > + if (drag->base.data_source->accepted) { > + wl_data_device_send_drop(drag > ->base.focus_resource); > + wl_data_source_send_drop_performed(drag > ->base.data_source->resource); > + } else { > + wl_data_source_send_cancelled(drag > ->base.data_source->resource); > + } > + } > > if (pointer->button_count == 0 && > state == WL_POINTER_BUTTON_STATE_RELEASED) { > @@ -878,11 +905,13 @@ create_data_source(struct wl_client *client, > source->accept = client_source_accept; > source->send = client_source_send; > source->cancel = client_source_cancel; > + source->offer = NULL; > + source->accepted = 0; > > wl_array_init(&source->mime_types); > > source->resource = > - wl_resource_create(client, > &wl_data_source_interface, 1, id); > + wl_resource_create(client, > &wl_data_source_interface, 3, id); > wl_resource_set_implementation(source->resource, > &data_source_interface, > source, destroy_data_source); > } > @@ -958,7 +987,7 @@ WL_EXPORT int > wl_data_device_manager_init(struct wl_display *display) > { > if (wl_global_create(display, > - &wl_data_device_manager_interface, 2, > + &wl_data_device_manager_interface, 3, > NULL, bind_manager) == NULL) > return -1; > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
