After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 637691 - Eating events breaks proxied DND
Eating events breaks proxied DND
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 601731
 
 
Reported: 2010-12-20 21:43 UTC by drago01
Modified: 2011-01-05 09:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GdkWindowCache: Always return GDK_FILTER_CONTINUE in event filters (1018 bytes, patch)
2010-12-20 21:53 UTC, drago01
none Details | Review
GdkDndX11: Decouple the window caches from the source context (2.87 KB, patch)
2011-01-04 21:57 UTC, drago01
none Details | Review

Description drago01 2010-12-20 21:43:17 UTC
While working on bug 601731 I noticed that for some reason the dnd operation (using the shell as proxy) only works once for every app. While debugging I found that the first source context that never gets freed which results into multiple gdk_window_cache_shape_filter running at the same time where the first one eats the events by returning GDK_FILTER_REMOVE, which results into the second context never seeing the events.

Changing it to always return GDK_FILTER_CONTINUE fixed it, and also resulted into the first context being actually freed (no idea why though).

The same seems to happen for the dnd from app -> gnome-panel -> other app case, but here it is caused by gdk_window_cache_filter (likewise making it always return GDK_FILTER_CONTINUE fixes it).
Comment 1 drago01 2010-12-20 21:53:05 UTC
Created attachment 176795 [details] [review]
GdkWindowCache: Always return GDK_FILTER_CONTINUE in event filters

Eating events by returing GDK_FILTER_REMOVE seems to break DND in some
cases.

----

FWIW here is the patch that fixes it, I still don't understand *why* it works
though.
Comment 2 Matthias Clasen 2010-12-20 23:05:49 UTC
I think we need to find out some more here first.

One thing to notice first is that we have a window cache per screen, each with its own shape filter, but the shape filter is run for events on all screens. To be more correct, the shape filter should only return GDK_FILTER_REMOVE if the shape event occurred on the correct screen.

Now, I doubt that you have multiple protocol screens, thus this is probably not the source of the problem.
Comment 3 drago01 2010-12-20 23:14:28 UTC
(In reply to comment #2)
> I think we need to find out some more here first.

Yeah.

> One thing to notice first is that we have a window cache per screen, each with
> its own shape filter, but the shape filter is run for events on all screens.

Well actually it is per context, per screen so if we somehow end up with more than one source context we can have multiple filters for one screen.

Which seems to happen here as the first context is never freed and thus the filter never removed.

It makes sense that using GDK_FILTER_CONTINUE fixes it in that case but not that it fixes the "first context is never being freed issue" (which it does).

> To
> be more correct, the shape filter should only return GDK_FILTER_REMOVE if the
> shape event occurred on the correct screen.
> 
> Now, I doubt that you have multiple protocol screens, thus this is probably not
> the source of the problem.

Yeah this is a single (protocol and physical) screen case.
Comment 4 Matthias Clasen 2010-12-21 01:32:42 UTC
I  have instrumented drag_context_init / finalize and so far haven't seen a source context being left behind. What I do see is that we tend to keep a dest context around, since current_drag_dest in the display keeps a reference. But that should not really matter, since dest contexts never have window caches, and thus don't have any filters installed.

app -> gnome-panel -> other app

Can you describe this case in more detail ? I haven't been able to reproduce it
Comment 5 drago01 2010-12-21 08:51:46 UTC
Here a video (http://193.200.113.196/apache2-default/dndbug.ogv) reproducing it (first dnd action works, second does not), with debug output in the terminal:

init: (0x2113df0)
new_source (0x2113df0)
init: (0x2113ed0)
free (0x2113ed0) is_source: 0
init: (0x2244070)
new_source (0x2244070)
init: (0x2244150)
free (0x2244150) is_source: 0
init: (0x2244230)
free (0x2244230) is_source: 0

Notice that the first source (0x2113df0) does not have a free entry (and that is_source is always 0).

I got the output by adding prints in gdk_drag_context_init(), gdk_drag_context_finalize() and gdk_drag_begin().
Comment 6 Matthias Clasen 2010-12-22 18:44:14 UTC
Sadly, I'm not seeing this here
Comment 7 drago01 2010-12-22 19:39:49 UTC
(In reply to comment #6)
> Sadly, I'm not seeing this here

:/ .. any tips on how to track this down? I am running out of ideas ...
Comment 8 drago01 2011-01-04 21:57:30 UTC
Created attachment 177525 [details] [review]
GdkDndX11: Decouple the window caches from the source context

Currently window caches are per context per screen rather than
globally per screen.

Due to the fact that they register window filters odd interactions
can happen when multiple context are around.

See:
https://bugzilla.gnome.org/show_bug.cgi?id=144324
https://bugzilla.gnome.org/show_bug.cgi?id=637691

Fix that by just keeping a global window_caches list which is shared by
all contexts.
Comment 9 Matthias Clasen 2011-01-05 00:30:57 UTC
Committed a slightly more involved patch that avoid dropping the cache too frequently when doing intra-process dnd.
Comment 10 drago01 2011-01-05 09:33:10 UTC
(In reply to comment #9)
> Committed a slightly more involved patch that avoid dropping the cache too
> frequently when doing intra-process dnd.

Thanks, is it possible to cherry-pick this into the 2.x branch?
(We'd want dnd to work between gtk2 apps and the shell too).