From: Rodrigo Rivas Subject: user32: Fix error handling in {Begin,End,}DeferWindowPos() to match Windows behavior (resend) Message-Id: Date: Tue, 25 Aug 2015 15:08:43 +0200 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 diff --git a/dlls/user32/tests/Makefile.in b/dlls/user32/tests/Makefile.in index 7149dc8..982b0fe 100644 --- a/dlls/user32/tests/Makefile.in +++ b/dlls/user32/tests/Makefile.in @@ -9,6 +9,7 @@ C_SRCS = \ cursoricon.c \ dce.c \ dde.c \ + defer.c \ dialog.c \ edit.c \ generated.c \ diff --git a/dlls/user32/tests/defer.c b/dlls/user32/tests/defer.c new file mode 100644 index 0000000..eb63384 --- /dev/null +++ b/dlls/user32/tests/defer.c @@ -0,0 +1,168 @@ +/* + * Unit tests for DeferWindowPos + * + * Copyright 2015 Rodrigo Rivas Costa + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "windows.h" + +#include "wine/test.h" + + +#define WIDTH_CHILDREN 50 +#define NCHILDREN 3 +#define IDX_FAIL 1 + +static HINSTANCE g_hInstance; +static HWND hChildren[NCHILDREN]; + +static void ExpectY(HWND hwnd, int y) +{ + RECT r; + POINT p; + GetWindowRect(hwnd, &r); + p.x = r.left; + p.y = r.top; + MapWindowPoints(NULL, GetParent(hwnd), &p, 1); + ok(p.y == y, "Window has not been properly moved: %d != %d\n", (int)p.y, y); +} + +static void TestDeferWindowPos(int posY) +{ + int i; + BOOL res; + HDWP hdwp; + + hdwp = BeginDeferWindowPos(1); + ok(hdwp != NULL, "BeginDeferWindowPos failed\n"); + for (i = 0; i < NCHILDREN; ++i) + { + HWND h = hChildren[i]; + hdwp = DeferWindowPos(hdwp, h, NULL, i*WIDTH_CHILDREN, posY, 0, 0, SWP_NOZORDER|SWP_NOSIZE|SWP_NOACTIVATE); + ok(hdwp != NULL, "DeferWindowPos failed\n"); + } + res = EndDeferWindowPos(hdwp); + ok(res, "EndDeferWindowPos failed\n"); + for (i = 0; i < NCHILDREN; ++i) + ExpectY(hChildren[i], posY); +} + +static void TestBogusDeferWindowPos(int posY) +{ + int i; + BOOL res; + HDWP hdwp; + + hdwp = BeginDeferWindowPos(1); + ok(hdwp != NULL, "BeginDeferWindowPos failed\n"); + for (i = 0; i < NCHILDREN; ++i) + { + HDWP hnext; + HWND h = hChildren[i]; + if (i==IDX_FAIL) + h = (HWND)0; //Insert an error. This is not a valid HWND. + hnext = DeferWindowPos(hdwp, h, NULL, i*WIDTH_CHILDREN, posY, 0, 0, SWP_NOZORDER|SWP_NOSIZE|SWP_NOACTIVATE); + if (i==IDX_FAIL) + { + DWORD err = GetLastError(); + ok(hnext == NULL, "DeferWindowPos accepted a bogus HWND\n"); + ok(err == ERROR_INVALID_WINDOW_HANDLE, "Bogus HWND should return ERROR_INVALID_WINDOW_HANDLE, got %d\n", (int)err); + } + else + { + ok(hnext != NULL, "DeferWindowPos failed\n"); + hdwp = hnext; + } + } + res = EndDeferWindowPos(hdwp); + ok(res, "EndDeferWindowPos failed\n"); + for (i = 0; i < NCHILDREN; ++i) + { + if (i != IDX_FAIL) + ExpectY(hChildren[i], posY); + } +} + +static void TestDestroyDeferWindowPos(int posY) +{ + int i; + BOOL res; + HDWP hdwp; + + hdwp = BeginDeferWindowPos(1); + ok(hdwp != NULL, "BeginDeferWindowPos failed\n"); + for (i = 0; i < NCHILDREN; ++i) + { + HWND h = hChildren[i]; + hdwp = DeferWindowPos(hdwp, h, NULL, i*WIDTH_CHILDREN, posY, 0, 0, SWP_NOZORDER|SWP_NOSIZE|SWP_NOACTIVATE); + ok(hdwp != NULL, "DeferWindowPos failed\n"); + /* Destroying a window after DeferWindowPos but before EndDeferWindowPos should be silently ignored */ + if (i == IDX_FAIL) + DestroyWindow(h); + } + res = EndDeferWindowPos(hdwp); + ok(res, "EndDeferWindowPos failed\n"); + for (i = 0; i < NCHILDREN; ++i) + { + if (i != IDX_FAIL) + ExpectY(hChildren[i], posY); + } +} + +static LRESULT CALLBACK TestWndProc(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) +{ + switch (msg) + { + case WM_CREATE: + { + int i; + int posY = 0; + for (i=0; i < NCHILDREN; ++i) + { + hChildren[i] = CreateWindowExA(0, "Button", NULL, WS_CHILD|WS_VISIBLE, + i*WIDTH_CHILDREN, posY, WIDTH_CHILDREN, WIDTH_CHILDREN, + hWnd, NULL, g_hInstance, NULL); + } + + TestDeferWindowPos(++posY); + TestBogusDeferWindowPos(++posY); + TestDestroyDeferWindowPos(++posY); + } + break; + } + return DefWindowProcA(hWnd, msg, wParam, lParam); +} + +START_TEST(defer) +{ + WNDCLASSEXA cls; + ATOM acls; + + g_hInstance = GetModuleHandleA(NULL); + memset(&cls, 0, sizeof(cls)); + cls.cbSize = sizeof(cls); + cls.lpfnWndProc = TestWndProc; + cls.hInstance = g_hInstance; + cls.lpszClassName = "TestWnd"; + + acls = RegisterClassExA(&cls); + + HWND hWnd = CreateWindowExA(0, MAKEINTRESOURCEA(acls), "TestWnd", WS_OVERLAPPEDWINDOW, + CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, + NULL, NULL, g_hInstance, NULL); + ok(hWnd != NULL, "CreateWindowEx failed\n"); +} diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c index f92a3dc..7cb3a4f 100644 --- a/dlls/user32/winpos.c +++ b/dlls/user32/winpos.c @@ -2349,6 +2349,11 @@ HDWP WINAPI DeferWindowPos( HDWP hdwp, HWND hwnd, HWND hwndAfter, hwnd = WIN_GetFullHandle( hwnd ); if (is_desktop_window( hwnd )) return 0; + if (!IsWindow( hwnd )) + { + SetLastError( ERROR_INVALID_WINDOW_HANDLE ); + return 0; + } if (!(pDWP = get_user_handle_ptr( hdwp, USER_DWP ))) return 0; if (pDWP == OBJ_OTHER_PROCESS) @@ -2418,7 +2423,6 @@ BOOL WINAPI EndDeferWindowPos( HDWP hdwp ) { DWP *pDWP; WINDOWPOS *winpos; - BOOL res = TRUE; int i; TRACE("%p\n", hdwp); @@ -2430,20 +2434,20 @@ BOOL WINAPI EndDeferWindowPos( HDWP hdwp ) return FALSE; } - for (i = 0, winpos = pDWP->winPos; res && i < pDWP->actualCount; i++, winpos++) + for (i = 0, winpos = pDWP->winPos; i < pDWP->actualCount; i++, winpos++) { TRACE("hwnd %p, after %p, %d,%d (%dx%d), flags %08x\n", winpos->hwnd, winpos->hwndInsertAfter, winpos->x, winpos->y, winpos->cx, winpos->cy, winpos->flags); if (WIN_IsCurrentThread( winpos->hwnd )) - res = USER_SetWindowPos( winpos ); + USER_SetWindowPos( winpos ); else - res = SendMessageW( winpos->hwnd, WM_WINE_SETWINDOWPOS, 0, (LPARAM)winpos ); + SendMessageW( winpos->hwnd, WM_WINE_SETWINDOWPOS, 0, (LPARAM)winpos ); } HeapFree( GetProcessHeap(), 0, pDWP->winPos ); HeapFree( GetProcessHeap(), 0, pDWP ); - return res; + return TRUE; }