Created attachment 243411 [details] [review]
window: Split out some code querying the state of buttons

This will be used to actually check that the button is being pressed down,
and significantly cut down on the amount of code introduced in the next
commit.

Created attachment 243412 [details] [review]
window: Eliminate a potential race condition with _NET_WM_MOVERESIZE

Clients using _NET_WM_MOVERESIZE to start a drag operation may encounter
a race condition if the user presses and releases a mouse button very
fast, getting "stuck" in a grab state. While this is easily fixed with
the user pressing the button or hitting Escape as the EWMH spec suggests,
its's still a bit of annoyance for users.

After starting a grab operation, check that the button is actually pressed
by the client, and if not, cancel the grab operation. This prevents the
stuck grab in a race-free way, although it requires an extra round-trip
to the server.

With client-side decorations becoming more popular, the use of
_NET_WM_MOVERESIZE is on the rise, thus this bug is seen more frequently
than before.

Review of attachment 243411 [details] [review]:

::: src/core/window.c
@@ +6627,3 @@
 #define _NET_WM_MOVERESIZE_CANCEL           11
 
+static int

We always use uint for masks

@@ +6638,3 @@
+
+  meta_error_trap_push (window->display);
+  XIQueryPointer (window->display->xdisplay,

XIQueryPointer has no reliable return value that indicates whether it succeeded or an error occurred:

    if (!_XReply(dpy, (xReply *)&rep,
                 (sizeof(xXIQueryPointerReply) - sizeof(xReply))/4, xFalse)) {
        UnlockDisplay(dpy);
        SyncHandle();
        return False;
    }

[...]

    UnlockDisplay(dpy);
    SyncHandle();
    return rep.same_screen;

So with the error trap pushed, buttons might be uninitialized, and free(button.mask) might segfault. So you need to meta_error_trap_push_with_return(). To avoid having to do the XQueryPointer twice, in the next patch you probably should move the button == 0 case *inside* the grab, and fix up display->grab_button after you decide which one it was. (You an decide whether you still want the helper in that case... it might still increase readability even if it is only called once.)

Review of attachment 243412 [details] [review]:

Looks OK, but see comments on the other patch.

Created attachment 244228 [details] [review]
window: Eliminate a potential race condition with _NET_WM_MOVERESIZE

Clients using _NET_WM_MOVERESIZE to start a drag operation may encounter
a race condition if the user presses and releases a mouse button very
fast, getting "stuck" in a grab state. While this is easily fixed with
the user pressing the button or hitting Escape as the EWMH spec suggests,
its's still a bit of annoyance for users.

After starting a grab operation, check that the button is actually pressed
by the client, and if not, cancel the grab operation. This prevents the
stuck grab in a race-free way, although it requires an extra round-trip
to the server.

With client-side decorations becoming more popular, the use of
_NET_WM_MOVERESIZE is on the rise, thus this bug is seen more frequently
than before.

Review of attachment 244228 [details] [review]:

Looks good to me

Attachment 244228 [details] pushed as a487d4d - window: Eliminate a potential race condition with _NET_WM_MOVERESIZE