[Bug 2155] Failure of SetFocus, SetActiveWindow and SetForegroundWindow

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=2155

Rémi Bernon <rbernon at codeweavers.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rbernon at codeweavers.com

--- Comment #25 from Rémi Bernon <rbernon at codeweavers.com> ---
Created attachment 68356
  --> https://bugs.winehq.org/attachment.cgi?id=68356
Patch series addressing several focus inconsistencies.

I spent some time digging into focus issues lately and I believe this issue has
several root causes.

First of all of course, Wine currently doesn't implement any mechanism to
activate other windows on the X11 side. This is implemented by the staged
patch, but I believe it can then trigger other focus problems that Wine suffers
from.


With or without the patch, the root issues are:

1) I believe there's a race condition in the SetForegroundWindow /
SetActiveWindow implementation. When multiple threads are involved,
SetActiveWindow requests from other threads are only applied when the thread
checks its messages, possibly overwriting changes that were done by the thread
itself.

2) The asynchronous nature of the focus handling code: Wine applies the Win32
focus state changes immediately, but processes -- and applies -- the results of
the X11 activation requests only later, during window message processing.

3) Windows allows hidden windows to be focused and foreground, X11 doesn't
allow unmapped windows to be. This is a fundamental difference and it is pretty
hard to solve, we currently implement hidden windows as unmapped X11 windows,
as it is probably the only way to have truly invisible windows, but it makes it
impossible to keep the X11 state consistent in that case.

The solutions are either to map hidden windows, using some other mechanism to
hide them as much as possible, bottom-most attribute being perhaps the least
brittle. Or, as a possible mitigation, force change X11 focus when hidden
window is supposed to be focused, so that X11 will send focus change events
when it changes back to another window.

4) SetForegroundWindow is not supposed to always succeed, although Wine always
pretend it does. The _NET_ACTIVE_WINDOW request also not always succeed, but as
we can't wait for its possible result, it is hard to return a reliable success
status.

This is especially the case with the staged patch, that sends activation
requests to be applied later, but it is also the case with user focus changes.
Having the X11 events processed and applied only when message processing can
trigger a race condition where X11 related focus changes will overwrite the
already executed Win32 focus changes, possibly ending up in an inconsistent
state.

Also, because of 1), when focus events come on several threads, SetActiveWindow
messages coming from other threads may be applied at the same as the X11 events
are processed and applied, making things even more inconsistent.


The attached patch series should help fixing these issues. First of all, it
fixes the incorrect message processing order in 1).

Then it should try to make 2) work better by tracking X11 focus globally in
explorer.exe, so that we don't rely on application processing their messages
anymore, and by discarding old SetForegroundWindow requests based on their
timestamp.

Last, it also implements a similar feature as the staged _NET_ACTIVE_WINDOW
patch to send X11 activation requests, but it adds a mitigation for 3) and 4),
where SetForegroundWindow returns failure if X11 focus is not on another Wine
window, and where SetActiveWindow forcefully unsets X11 focus when hidden
window is activated. It is still not completely matching Windows behavior, as
SetForegroundWindow should probably return FALSE in more cases, but the details
are a little bit more complicated and it may also break some currently working
cases.

I'm currently trying to upstream the first patches, but the way internal
messages gets discarded needed some discussion. The whole series also could
benefit from more extensive testing.

This is also probably going to conflict with the rawinput patch series, as
there are changes in the XInput2 code to get Focus messages with timestamps if
possible. I'll attach rebased versions next in the eventuality this goes into
staging.

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.