Thanks again for your feedback and assistance. This is my 4th revision. I've
created a static inline is_win9x() as you suggested. I agree that this should
be in include/wine/test.h. One minor complication is that some test files
define a BOOL or int "is_win9x" local variable, but that is easy to overcome.
A few months ago, I wrote an extensive windows version testing patch for
wine/test.h, but I appear to have either mis-placed it or accidentally blown it
away! =)
I've added win_skip() messages where tests are skipped on win9x to avoid
crashing the and I clarified the FIXME (I hope it makes more sense). I'm
trying to say that we should add tests to make sure height & width are checked
since CreateCursor currently doesn't validate the size of the cursor you want
to create against the values returned by GetSystemMetrics().
Finally, by Griswold's suggestion, I renamed test_SetCursor and
test_DestroyCursor, adding an underscore prefix and a macro that will fill in
the __LINE__ to clean up the code slightly.
If you can please run this again on a 9x platform and also an NT platform (XP
or vista) I would greatly appreciate it! I just want to make sure everything
is happy before submitting it and thanks again for all your help!
Daniel
From 01ca4d54401fa5d8226a28153caf24c5b4747e17 Mon Sep 17 00:00:00 2001
From: Daniel Santos <[email protected]>
Date: Fri, 10 Jul 2009 14:57:04 -0500
Subject: user32: Add more tests for SetCursor & DestroyCursor
---
dlls/user32/tests/cursoricon.c | 291 +++++++++++++++++++++++++++++++++-------
1 files changed, 241 insertions(+), 50 deletions(-)
diff --git a/dlls/user32/tests/cursoricon.c b/dlls/user32/tests/cursoricon.c
index 3f8bd82..f69a64f 100644
--- a/dlls/user32/tests/cursoricon.c
+++ b/dlls/user32/tests/cursoricon.c
@@ -18,6 +18,12 @@
* 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
+ *
+ * FIXME:
+ * - Add tests for CreateCursor and show that width & height can exceed
+ * SM_CXCURSOR & SM_CYCURSOR (as the current wine implementation will allow
+ * this).
+ * - Add tests for DestroyIcon32.
*/
#include <assert.h>
@@ -64,6 +70,132 @@ static HANDLE child_process;
#define PROC_INIT (WM_USER+1)
+static inline int is_win9x() {
+ SetLastError(0xdeadbeef);
+ lstrcmpW(NULL, NULL);
+ return (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED);
+}
+
+/*************************************************************************
+ * test_SetCursor
+ *
+ * Encapsulates tests to SetCursor() function.
+ *
+ * PARAMS
+ * line [I] The line this function was called from.
+ * target [I] A handle to use when calling SetCursor
+ * shouldFail [I] Rather of not the call to SetCursor should fail
+ * retValTodo [I] True if return value is broken in wine (wine's return value
+ * is broken in some, but not all cases)
+ *
+ * RETURNS
+ * The return value of the SetCursor() call.
+ */
+#define test_SetCursor(a, b, c) _test_SetCursor(__LINE__, a, b, c)
+static HCURSOR _test_SetCursor(int line, HANDLE hNewCursor, BOOL shouldFail,
+ BOOL retValTodo)
+{
+ HCURSOR cursor1, cursor2, cursor3;
+ DWORD error;
+
+ /* GetCursor should have no errors */
+ SetLastError(0xdeadbeef);
+ cursor1 = GetCursor();
+ error = GetLastError();
+ ok_(__FILE__, line)(error == 0xdeadbeef,
+ "GetCursor() changed last error: %u.\n",
+ error);
+
+ /* Call SetCursor on supplied handle */
+ SetLastError(0xdeadbeef);
+ cursor2 = SetCursor(hNewCursor);
+ error = GetLastError();
+ cursor3 = GetCursor();
+
+ if (shouldFail) {
+ todo_wine ok_(__FILE__, line)(error == ERROR_INVALID_CURSOR_HANDLE,
+ "Last error is %u (0x%x), expected "
+ "ERROR_INVALID_CURSOR_HANDLE (%u).\n",
+ error, error, ERROR_INVALID_CURSOR_HANDLE);
+
+ if(retValTodo) {
+ todo_wine ok_(__FILE__, line)(!cursor2,
+ "SetCursor() returned non-zero (%p).\n",
+ cursor2);
+ } else {
+ ok_(__FILE__, line)(!cursor2,
+ "SetCursor() returned non-zero (%p).\n",
+ cursor2);
+ }
+
+ todo_wine ok_(__FILE__, line)(cursor1 == cursor3,
+ "SetCursor() changed cursor.\n");
+ } else {
+ /* WinXP: For some reason, SetCursor() is returning NULL when
hNewCursor
+ * is NULL. Is it this way on other versions? MSDN says it should
+ * return the old cursor.
+ */
+ if(hNewCursor) {
+ ok_(__FILE__, line)(cursor1 == cursor2,
+ "SetCursor() did not return the previous cursor, "
+ "expected %p, got %p.\n",
+ cursor1, cursor2);
+ } else {
+ todo_wine ok_(__FILE__, line)(!cursor2,
+ "SetCursor() returned %p, expected NULL.\n",
+ cursor2);
+ }
+
+ ok_(__FILE__, line)(cursor3 == hNewCursor,
+ "GetCursor() did not return the value we previously passed to "
+ "SetCursor(). Expected %p, got %p.\n",
+ hNewCursor, cursor3);
+ }
+
+ return cursor2;
+}
+
+
+/*************************************************************************
+ * test_DestroyCursorIcon
+ *
+ * Encapsulates simple tests to DestroyCursor and DestroyIcon functions.
+ *
+ * PARAMS
+ * line [I] The line this function was called from.
+ * h [I] A handle to use when calling DestroyIcon/DestroyCursor
+ * isCursor [I] TRUE for DestroyCursor, FALSE for DestroyIcon
+ * expectedRet [I] The expected return value
+ * expectedError [I] The expected last error
+ *
+ * RETURNS
+ * The return value of the function call.
+ */
+#define test_DestroyCursorIcon(a, b, c, d) \
+ _test_DestroyCursorIcon(__LINE__, a, b, c, d)
+static BOOL _test_DestroyCursorIcon(int line, HANDLE h, BOOL isCursor,
+ BOOL expectedRet, DWORD expectedError)
+{
+ BOOL ret;
+ DWORD error;
+ const char* funcName = isCursor ? "DestroyCursor" : "DestroyIcon";
+
+ SetLastError(0xdeadbeef);
+ ret = isCursor ? DestroyCursor(h) : DestroyIcon(h);
+ error = GetLastError();
+
+ /* broken on NT4 Server SP6a when handle == 1 */
+ ok_(__FILE__, line)(ret == expectedRet || broken(h == (HANDLE)1),
+ "%s returned %d, expected %s.\n",
+ funcName, ret, expectedRet ? "TRUE" : "FALSE");
+ ok_(__FILE__, line)(error == expectedError || broken(h == (HANDLE)1),
+ "Last error is %u (0x%x), expected %u (0x%x).\n",
+ error, error, expectedError, expectedError);
+
+ return ret;
+}
+
+
static LRESULT CALLBACK callback_child(HWND hwnd, UINT msg, WPARAM wParam,
LPARAM lParam)
{
BOOL ret;
@@ -76,10 +208,11 @@ static LRESULT CALLBACK callback_child(HWND hwnd, UINT
msg, WPARAM wParam, LPARA
SetLastError(0xdeadbeef);
ret = DestroyCursor((HCURSOR) lParam);
error = GetLastError();
- todo_wine ok(!ret || broken(ret) /* win9x */, "DestroyCursor on
the active cursor succeeded.\n");
+ todo_wine ok(!ret || broken(ret), /* win9x */
+ "DestroyCursor on the active cursor of another process
succeeded.\n");
ok(error == ERROR_DESTROY_OBJECT_OF_OTHER_THREAD ||
- error == 0xdeadbeef, /* vista */
- "Last error: %u\n", error);
+ error == 0xdeadbeef, /* vista */
+ "Last error: %u\n", error);
return TRUE;
case WM_DESTROY:
PostQuitMessage(0);
@@ -169,6 +302,18 @@ static void do_parent(void)
0, 0, 200, 200, 0, 0, 0, NULL);
ok(parent != 0, "CreateWindowA failed. Error: %u\n", GetLastError());
+ /* make sure Destroy{Cursor,Icon} wont accept other user handle types */
+ if(is_win9x()) {
+ win_skip("Invalid cursor handles will crash win9x\n");
+ } else {
+ todo_wine {
+ test_DestroyCursorIcon(parent, TRUE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ test_DestroyCursorIcon(parent, FALSE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ }
+ }
+
/* Start child process. */
memset(&startup, 0, sizeof(startup));
startup.cb = sizeof(startup);
@@ -217,7 +362,7 @@ static void test_child_process(void)
cursor = CreateIconIndirect(&cursorInfo);
ok(cursor != NULL, "CreateIconIndirect returned %p.\n", cursor);
- SetCursor(cursor);
+ test_SetCursor(cursor, FALSE, FALSE);
/* Destroy the cursor. */
SendMessage(child, WM_USER+1, 0, (LPARAM) cursor);
@@ -246,7 +391,7 @@ static void test_CopyImage_Check(HBITMAP bitmap, UINT
flags, INT copyWidth, INT
&& (expectedDepth == 16 || expectedDepth == 32))
{
/* Windows 95 doesn't create DIBs with a depth of 16 or 32 bit */
- if (GetVersion() & 0x80000000)
+ if (is_win9x())
{
expectedDepth = 24;
}
@@ -377,7 +522,7 @@ static void test_CopyImage_Bitmap(int depth)
Skip this test on Windows 95, it always creates a monochrome DDB
in this case */
- if (!(GetVersion() & 0x80000000))
+ if (!is_win9x())
{
info->bmiHeader.biBitCount = 1;
info->bmiColors[0].rgbRed = 0xFF;
@@ -520,12 +665,12 @@ static void test_CreateIcon(void)
hIcon = CreateIcon(0, 16, 16, 1, 1, bmp_bits, bmp_bits);
ok(hIcon != 0, "CreateIcon failed\n");
test_icon_info(hIcon, 16, 16, 1);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
hIcon = CreateIcon(0, 16, 16, 1, display_bpp, bmp_bits, bmp_bits);
ok(hIcon != 0, "CreateIcon failed\n");
test_icon_info(hIcon, 16, 16, display_bpp);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
hbmMask = CreateBitmap(16, 16, 1, 1, bmp_bits);
ok(hbmMask != 0, "CreateBitmap failed\n");
@@ -560,7 +705,7 @@ static void test_CreateIcon(void)
hIcon = CreateIconIndirect(&info);
ok(hIcon != 0, "CreateIconIndirect failed\n");
test_icon_info(hIcon, 16, 16, display_bpp);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
DeleteObject(hbmMask);
DeleteObject(hbmColor);
@@ -577,7 +722,7 @@ static void test_CreateIcon(void)
hIcon = CreateIconIndirect(&info);
ok(hIcon != 0, "CreateIconIndirect failed\n");
test_icon_info(hIcon, 16, 16, 1);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
DeleteObject(hbmMask);
DeleteObject(hbmColor);
@@ -610,7 +755,7 @@ static void test_CreateIcon(void)
hIcon = CreateIconIndirect(&info);
ok(hIcon != 0, "CreateIconIndirect failed\n");
test_icon_info(hIcon, 32, 32, 8);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
DeleteObject(hbmColor);
bmpinfo->bmiHeader.biBitCount = 16;
@@ -628,7 +773,7 @@ static void test_CreateIcon(void)
hIcon = CreateIconIndirect(&info);
ok(hIcon != 0, "CreateIconIndirect failed\n");
test_icon_info(hIcon, 32, 32, 8);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
DeleteObject(hbmColor);
bmpinfo->bmiHeader.biBitCount = 32;
@@ -646,7 +791,7 @@ static void test_CreateIcon(void)
hIcon = CreateIconIndirect(&info);
ok(hIcon != 0, "CreateIconIndirect failed\n");
test_icon_info(hIcon, 32, 32, 8);
- DestroyIcon(hIcon);
+ test_DestroyCursorIcon(hIcon, FALSE, TRUE, 0xdeadbeef);
DeleteObject(hbmMask);
DeleteObject(hbmColor);
@@ -740,7 +885,8 @@ static void test_LoadImageFile(const unsigned char *
image_data,
broken(error == 0xdeadbeef) || /* Win9x */
broken(error == ERROR_BAD_PATHNAME), /* Win98, WinMe */
"Last error: %u\n", error);
- if (handle != NULL) DestroyCursor(handle);
+ if (handle != NULL)
+ test_DestroyCursorIcon(handle, TRUE, TRUE, 0xdeadbeef);
/* Load as icon. For all tested formats, this should fail */
SetLastError(0xdeadbeef);
@@ -751,7 +897,8 @@ static void test_LoadImageFile(const unsigned char *
image_data,
broken(error == 0xdeadbeef) || /* Win9x */
broken(error == ERROR_BAD_PATHNAME), /* Win98, WinMe */
"Last error: %u\n", error);
- if (handle != NULL) DestroyIcon(handle);
+ if (handle != NULL)
+ test_DestroyCursorIcon(handle, FALSE, TRUE, 0xdeadbeef);
/* Load as bitmap. Should succeed if bmp, fail for everything else */
SetLastError(0xdeadbeef);
@@ -771,6 +918,7 @@ static void test_LoadImageFile(const unsigned char *
image_data,
static void test_LoadImage(void)
{
HANDLE handle;
+ HCURSOR cursor;
BOOL ret;
DWORD error, bytes_written;
CURSORICONFILEDIR *icon_data;
@@ -845,16 +993,26 @@ static void test_LoadImage(void)
ok(icon_info.hbmMask != NULL, "No hbmMask!\n");
}
+ /* Test SetCursor with this icon */
+ cursor = test_SetCursor(handle, FALSE, TRUE);
+
+ /* Now change it back to the previous cursor */
+ test_SetCursor(cursor, FALSE, FALSE);
+
/* Clean up. */
- SetLastError(0xdeadbeef);
- ret = DestroyCursor(handle);
- ok(ret, "DestroyCursor() failed.\n");
- error = GetLastError();
- ok(error == 0xdeadbeef, "Last error: %u\n", error);
+ test_DestroyCursorIcon(handle, TRUE, TRUE, 0xdeadbeef);
HeapFree(GetProcessHeap(), 0, icon_data);
DeleteFileA("icon.ico");
+ /* Test passing invalid handles to SetCursor. */
+ if(is_win9x()) {
+ win_skip("Invalid cursor handles will crash win9x\n");
+ } else {
+ test_SetCursor(handle, TRUE, FALSE);
+ test_SetCursor(GetCurrentThread(), TRUE, TRUE);
+ }
+
test_LoadImageFile(bmpimage, sizeof(bmpimage), "bmp", 1);
test_LoadImageFile(gifimage, sizeof(gifimage), "gif", 0);
test_LoadImageFile(gif4pixel, sizeof(gif4pixel), "gif", 0);
@@ -917,11 +1075,7 @@ static void test_CreateIconFromResource(void)
}
/* Clean up. */
- SetLastError(0xdeadbeef);
- ret = DestroyCursor(handle);
- ok(ret, "DestroyCursor() failed.\n");
- error = GetLastError();
- ok(error == 0xdeadbeef, "Last error: %u\n", error);
+ test_DestroyCursorIcon(handle, TRUE, TRUE, 0xdeadbeef);
/* Test creating an icon. */
SetLastError(0xdeadbeef);
@@ -947,11 +1101,7 @@ static void test_CreateIconFromResource(void)
}
/* Clean up. */
- SetLastError(0xdeadbeef);
- ret = DestroyCursor(handle);
- ok(ret, "DestroyCursor() failed.\n");
- error = GetLastError();
- ok(error == 0xdeadbeef, "Last error: %u\n", error);
+ test_DestroyCursorIcon(handle, TRUE, TRUE, 0xdeadbeef);
HeapFree(GetProcessHeap(), 0, hotspot);
}
@@ -1222,6 +1372,26 @@ static void test_DestroyCursor(void)
UINT display_bpp;
HDC hdc;
+ /* Try to destroy various invalid handles */
+ if(is_win9x()) {
+ win_skip("Invalid cursor handles will crash win9x\n");
+ } else {
+ todo_wine {
+ test_DestroyCursorIcon(0, TRUE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ test_DestroyCursorIcon((HANDLE)1, TRUE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ test_DestroyCursorIcon(INVALID_HANDLE_VALUE, TRUE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ test_DestroyCursorIcon(0, FALSE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ test_DestroyCursorIcon((HANDLE)1, FALSE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ test_DestroyCursorIcon(INVALID_HANDLE_VALUE, FALSE, FALSE,
+ ERROR_INVALID_CURSOR_HANDLE);
+ }
+ }
+
hdc = GetDC(0);
display_bpp = GetDeviceCaps(hdc, BITSPIXEL);
ReleaseDC(0, hdc);
@@ -1237,29 +1407,54 @@ static void test_DestroyCursor(void)
if(!cursor) {
return;
}
- SetCursor(cursor);
+ test_SetCursor(cursor, FALSE, FALSE);
+
+ /* When you attempt to destroy the current cursor, windows obliges you.
+ * On win9x, it returns TRUE (indicating success) and on NT it returns
+ * FALSE, but neither sets the last error.
+ */
SetLastError(0xdeadbeef);
ret = DestroyCursor(cursor);
- ok(!ret || broken(ret) /* succeeds on win9x */, "DestroyCursor on the
active cursor succeeded\n");
error = GetLastError();
- ok(error == 0xdeadbeef, "Last error: %u\n", error);
+ ok(error == 0xdeadbeef, "Last error: 0x%x.\n", error);
+ ok(!ret || broken(ret) /* succeeds on win9x */,
+ "DestroyCursor on the active cursor returned TRUE.\n");
+
+ /* QUESTION: This if statment effectively skips these tests on win9x, do we
+ * want to do that? */
if (!ret)
{
cursor2 = GetCursor();
- ok(cursor2 == cursor, "Active was set to %p when trying to destroy
it\n", cursor2);
- SetCursor(NULL);
-
- /* Trying to destroy the cursor properly fails now with
- * ERROR_INVALID_CURSOR_HANDLE. This happens because we called
- * DestroyCursor() 2+ times after calling SetCursor(). The calls to
- * GetCursor() and SetCursor(NULL) in between make no difference. */
+ ok(cursor2 == cursor, "Active cursor was set to %p when trying to "
+ "destroy it, expected it to be %p.\n", cursor2, cursor);
+
+ /* Make sure showing/hiding the invalid cursor still works. */
+ SetLastError(0xdeadbeef);
+ ok(ShowCursor(TRUE) == 1, "Cursor count != 1\n");
+ ok(GetLastError() == 0xdeadbeef, "Last error: 0x%x.\n",
GetLastError());
+
+ SetLastError(0xdeadbeef);
+ ok(ShowCursor(FALSE) == 0, "Cursor count != 0\n");
+ ok(GetLastError() == 0xdeadbeef, "Last error: 0x%x.\n",
GetLastError());
+ test_SetCursor(NULL, FALSE, FALSE);
+
+ /* Trying to destroy the cursor again fails now with
+ * ERROR_INVALID_CURSOR_HANDLE. This happens because we've previously
+ * called DestroyCursor() on this handle. The calls to GetCursor()
+ * and SetCursor(NULL) in between make no difference. It would appear
+ * that windows doesn't mind having an invalid cursor handle as its
+ * current cursor much the way a comatose person doesn't mind a
+ * stranger in their hospital room.
+ */
+ SetLastError(0xdeadbeef);
ret = DestroyCursor(cursor);
todo_wine {
ok(!ret, "DestroyCursor succeeded.\n");
error = GetLastError();
- ok(error == ERROR_INVALID_CURSOR_HANDLE || error == 0xdeadbeef, /*
vista */
- "Last error: 0x%08x\n", error);
+ ok(error == ERROR_INVALID_CURSOR_HANDLE
+ || broken(error == 0xdeadbeef), /* vista */
+ "Last error: 0x%08x\n", error);
}
}
@@ -1271,17 +1466,13 @@ static void test_DestroyCursor(void)
SetLastError(0xdeadbeef);
ret = DestroyCursor(cursor);
- ok(ret || broken(!ret) /* fails on win9x */, "DestroyCursor on the active
cursor failed.\n");
error = GetLastError();
- ok(error == 0xdeadbeef, "Last error: 0x%08x\n", error);
+ ok(error == 0xdeadbeef, "Last error: 0x%x.\n", error);
+ ok(ret || broken(!ret) /* fails on win9x */,
+ "DestroyCursor on OEM cursor failed.\n");
/* Try setting the cursor to a destroyed OEM cursor. */
- SetLastError(0xdeadbeef);
- SetCursor(cursor);
- error = GetLastError();
- todo_wine {
- ok(error == 0xdeadbeef, "Last error: 0x%08x\n", error);
- }
+ test_SetCursor(cursor, FALSE, FALSE);
/* Check if LoadCursor() returns the same handle with the same icon. */
cursor2 = LoadCursor(NULL, IDC_ARROW);
--
1.6.3.3
