user32: Fix error handling in {Begin,End,}DeferWindowPos() to match Windows behavior (resend)

[Rodrigo Rivas]

[Sorry for the double post, I forgot to reply to the list]

On Tue, Aug 25, 2015 at 3:35 PM, Nikolay Sivov <bunglehead at gmail.com> wrote:
> On 25.08.2015 16:08, Rodrigo Rivas wrote:
>>
>> I sent this on July 15th, but it went through the cracks. Maybe the
>> mail text was not detailed enough, so I will try and explain it better
>> this time.
>>
>> The error behaviour of functions BeginDeferWindowPos(),
>> DeferWindowPos() and EndDeferWindowPos() in Wine is different than in
>> Windows.
>>
>> In Windows, when DeferWindowPos() is used with an invalid HWND, this
>> function fails, but the eventual EndDeferWindowPos() will succeed and
>> move all the other windows in the set.
>> In Wine, however, DeferWindowPos() does not check the HWND, so it
>> never fails. But then, it fails in EndDeferWindowPos(), moving only
>> the windows passed before the invalid HWND and leaving the windows
>> after the invalid HWND unmoved.
>>
>> The Windows behaviour is consistent in every version I tested (XP and 7).
>>
>> The attached patch makes the Wine error behaviour the same as the Windows
>> one.
>> It also considers the case where the HWND is valid when calling
>> DeferWindowPos() but is destroyed before calling EndDeferWindowPos():
>> The error is simply ignored.
>>
>> It fixes bug #23187.
>>
>> ---
>>   dlls/user32/tests/Makefile.in |   1 +
>>   dlls/user32/tests/defer.c     | 168
>> ++++++++++++++++++++++++++++++++++++++++++
>>   dlls/user32/winpos.c          |  14 ++--
>>   3 files changed, 178 insertions(+), 5 deletions(-)
>>   create mode 100644 dlls/user32/tests/defer.c
>>
>
> I don't think you need a new file for that, how about moving this to win.c?

It may make sense in win.c. I assumed that user32/tests/win.c is
related to user32/win.c. And since the affected functions are in
user32/winpos.c... maybe I should have named it user32/tests/winpos.c?
But I don't have a strong preference
If you really want it, would you like me to do the merge and resend
the patch? Or is it not necessary?

> Regarding fix itself, it's probably better to split it to two - fix
> DeferWindowPos() in first + related tests, and End* one in the second.

In this I disagree. Changing DeferWindowPos() without changing
EndDeferWindowPos() would make little sense, because the error
checking is moving from EDWP to DWP.
The special case of the window being destroyed between DWP and EDWP is
just a consequence of EDWP not having any error checking left.
You could have one patch that adds the check to DWP and another that
removes it from EDWP, but what's the point? The intermediate step will
not even pass the test case, it will fail at
TestDestroyDeferWindowPos().

So to fair you would have to split in: a) fix DWP + tess1+test2; b)
fix EDWP + test3.