On Tue, Nov 24, 2020 at 05:18:32PM -0600, Kevin Puetz wrote:
Check for completion (handles/APC) before pumping any messages, and again after each message is dispatched, returning as soon as the wait condition is satisfied. This matches windows behavior and obsoletes the previous "100 message" workaround for WM_PAINT.
Only report timeout before/during sleep, not while actively working. The previous code had a narrow race (particularly for timeout=0): if GetTickCount incremented between start_time and the loop body it could WAIT_TIMEOUT even if the handles were already signaled. No test; I couldn't think of a way to provoke this consistently.
NOTE: this means CoWaitForMultipleHandles does not time out while there are still queued messages, and will livelock (and never time out) if dispatching of messages continuously posts additional messages. It will exit (successfully) if the handles eventually become signaled. The latter is the only case tested, since I don't know how to write a successful test for "this will livelock and hang the process". But windows does do the same.
Notify IMessageFilter::MessagePending only for messages that wake CoWait from MsgWait (sleeping on an empty queue), but not for messages already posted before it sleeps.
Add tests for IMessageFilter::MessagePending -> PENDINGMSG_CANCELCALL One of these is todo_wine, as was the existing MessageFilter test. The bug is the same: windows does not call MessagePending for DDE/RPC messages, but wine (still) does. I'm not sure what the right structure is to fix this, but it's a separate (and preexisting) issue.
Adding a few helper functions before making any functional changes would help. For instance you could move the if (apt->filter) block to something like: HRESULT call_msg_filter(struct apartment *apt);
and the message loop to something like: BOOL msg_loop(struct apartment *apt, UINT *exit_code); This would return zero if it receives WM_QUIT (to match GetMessage()) and set exit_code. It would initally also include the 100 msg hack.
Having done that, I'd imagine the diff of the functional change should be clearer.
Huw.