[PATCH resend] winex11.drv: Don't set the foreground for focus events that are older than the last SetFocus sent without an event

[Rémi Bernon]

On 8/15/19 3:57 PM, Gabriel Ivăncescu wrote:
> @@ -803,7 +820,7 @@ static void focus_out( Display *display , HWND hwnd )
>   
>       if (ximInComposeMode) return;
>   
> -    x11drv_thread_data()->last_focus = hwnd;
> +    thread_data->last_focus = hwnd;
>       if ((xic = X11DRV_get_ic( hwnd ))) XUnsetICFocus( xic );
>   
>       if (root_window != DefaultRootWindow(display))
> @@ -812,6 +829,12 @@ static void focus_out( Display *display , HWND hwnd )
>           return;
>       }
>       if (hwnd != GetForegroundWindow()) return;
> +
> +    if ((long)(thread_data->focus_serial - serial) > 0)
> +    {
> +        TRACE("ignoring old serial %lu/%lu\n", serial, thread_data->focus_serial);
> +        return;
> +    }

I'm not sure if this is the right thing to do here.

I understand the reason to skip calling SetForegroundWindow on FocusIn 
events - because SetFocus called by DefWinProc would already have done 
it before, as a consequence of ShowWindow / WM_ACTIVATE messages - but 
in the case of focus out events, I believe it is already skipped if the 
newly focused window is another Wine window.

Also we will not send the WM_CANCELMODE message anymore and I don't know 
if it is OK.

>       SendMessageW( hwnd, WM_CANCELMODE, 0, 0 );
>   
>       /* don't reset the foreground window, if the window which is
> @@ -855,7 +878,7 @@ static BOOL X11DRV_FocusOut( HWND hwnd, XEvent *xev )
>           return TRUE;
>       }
>       if (!hwnd) return FALSE;
> -    focus_out( event->display, hwnd );
> +    focus_out( event->display, hwnd, event->serial );
>       return TRUE;
>   }
>   
> @@ -1385,6 +1408,7 @@ void wait_for_withdrawn_state( HWND hwnd, BOOL set )
>    */
>   void CDECL X11DRV_SetFocus( HWND hwnd )
>   {
> +    struct x11drv_thread_data *thread_data;
>       struct x11drv_win_data *data;
>   
>       HWND parent;
> @@ -1400,6 +1424,21 @@ void CDECL X11DRV_SetFocus( HWND hwnd )
>       }
>       if (!data->managed || data->embedder) set_input_focus( data );
>       release_win_data( data );
> +
> +    hwnd = GetAncestor(hwnd, GA_ROOT);
> +    if (hwnd != (thread_data = x11drv_thread_data())->setfocus_hwnd)

I think it would be more readable to initialize thread_data along with 
its declaration, like it's done elsewhere, unless there's a good reason 
not to.

> +    {
> +        thread_data->setfocus_hwnd = hwnd;
> +
> +        /* If we are not processing an event, store the current serial after syncing
> +           with the X server, so that we ignore all older focus-related events. This
> +           prevents focus being reverted later when the app processes its messages. */
> +        if (!thread_data->current_event)
> +        {
> +            XSync(thread_data->display, False);
> +            thread_data->focus_serial = LastKnownRequestProcessed(thread_data->display) + 1;
> +        }
> +    }
>   }
>   
>   
> diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h
> index 0d3695b..63598d7 100644
> --- a/dlls/winex11.drv/x11drv.h
> +++ b/dlls/winex11.drv/x11drv.h
> @@ -326,10 +326,12 @@ struct x11drv_thread_data
>       XEvent  *current_event;        /* event currently being processed */
>       HWND     grab_hwnd;            /* window that currently grabs the mouse */
>       HWND     last_focus;           /* last window that had focus */
> +    HWND     setfocus_hwnd;        /* top-level window corresponding to the last SetFocus */
>       XIM      xim;                  /* input method */
>       HWND     last_xic_hwnd;        /* last xic window */
>       XFontSet font_set;             /* international text drawing font set */
>       Window   selection_wnd;        /* window used for selection interactions */
> +    unsigned long focus_serial;    /* serial number when last SetFocus without an event happened */
>       unsigned long warp_serial;     /* serial number of last pointer warp request */
>       Window   clip_window;          /* window used for cursor clipping */
>       HWND     clip_hwnd;            /* message window stored in desktop while clipping is active */
> 

I would rename focus_serial to setfocus_serial and keep it close to 
setfocus_hwnd as they are meant to work together - or the other way 
around. This way we also understand that set_focus

Otherwise I would say it looks OK to me with my limited knowledge of the 
whole thing works.
-- 
Rémi Bernon <rbernon at codeweavers.com>