Re: [v2 PATCH 2/2] comctl32/tests: Some tests for property page initialization

[Sebastian Lackner]

On 19.02.2017 19:24, Nikolay Sivov wrote:
> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
> ---
>
> Resending, it depends on header change in 1/2.
>
> dlls/comctl32/tests/propsheet.c | 142 ++++++++++++++++++++++++++++++++++++++++
>  include/prsht.h                 |   1 +
>  2 files changed, 143 insertions(+)
>
> diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c
> index 7547e793e8..d11fac4bb0 100644
> --- a/dlls/comctl32/tests/propsheet.c
> +++ b/dlls/comctl32/tests/propsheet.c
> @@ -987,6 +987,147 @@ if (0)
>      DestroyWindow(hdlg);
>  }
>
> +struct custom_proppage
> +{
> +    union
> +    {
> +        PROPSHEETPAGEA pageA;
> +        PROPSHEETPAGEW pageW;
> +    } u;
> +    unsigned int addref_called;
> +    unsigned int release_called;
> +};
> +
> +static UINT CALLBACK proppage_callback_a(HWND hwnd, UINT msg, PROPSHEETPAGEA *psp)
> +{
> +    struct custom_proppage *cpage = (struct custom_proppage *)psp->lParam;
> +    PROPSHEETPAGEA *psp_orig = &cpage->u.pageA;
> +
> +    ok(hwnd == NULL, "Expected NULL hwnd, got %p\n", hwnd);
> +
> +    ok(psp->lParam && psp->lParam != (LPARAM)psp, "Expected newly allocated page description, got %lx, %p\n",
> +            psp->lParam, psp);
> +    ok(psp_orig->pszTitle == psp->pszTitle, "Expected same page title pointer\n");
> +    ok(!lstrcmpA(psp_orig->pszTitle, psp->pszTitle), "Expected same page title string\n");
> +
> +    switch (msg)
> +    {
> +    case PSPCB_ADDREF:
> +        ok(psp->dwSize > PROPSHEETPAGEA_V1_SIZE, "Expected ADDREF for V2+ only, got size %u\n", psp->dwSize);
> +        cpage->addref_called++;
> +        break;
> +    case PSPCB_RELEASE:
> +        ok(psp->dwSize >= PROPSHEETPAGEA_V1_SIZE, "Unexpected RELEASE, got size %u\n", psp->dwSize);
> +        cpage->release_called++;
> +        break;
> +    default:
> +        ok(0, "Unexpected message %u\n", msg);
> +    }
> +
> +    return 1;
> +}
> +
> +static UINT CALLBACK proppage_callback_w(HWND hwnd, UINT msg, PROPSHEETPAGEW *psp)
> +{
> +    struct custom_proppage *cpage = (struct custom_proppage *)psp->lParam;
> +    PROPSHEETPAGEW *psp_orig = &cpage->u.pageW;
> +
> +    ok(hwnd == NULL, "Expected NULL hwnd, got %p\n", hwnd);
> +    ok(psp->lParam && psp->lParam != (LPARAM)psp, "Expected newly allocated page description, got %lx, %p\n",
> +            psp->lParam, psp);
> +    ok(psp_orig->pszTitle == psp->pszTitle, "Expected same page title pointer\n");
> +    ok(!lstrcmpW(psp_orig->pszTitle, psp->pszTitle), "Expected same page title string\n");
> +
> +    switch (msg)
> +    {
> +    case PSPCB_ADDREF:
> +        ok(psp->dwSize > PROPSHEETPAGEW_V1_SIZE, "Expected ADDREF for V2+ only, got size %u\n", psp->dwSize);
> +        cpage->addref_called++;
> +        break;
> +    case PSPCB_RELEASE:
> +        ok(psp->dwSize >= PROPSHEETPAGEW_V1_SIZE, "Unexpected RELEASE, got size %u\n", psp->dwSize);
> +        cpage->release_called++;
> +        break;
> +    default:
> +        ok(0, "Unexpected message %u\n", msg);
> +    }
> +
> +    return 1;
> +}
> +
> +static void test_CreatePropertySheetPage(void)
> +{
> +    static const WCHAR titleW[] = {'T','i','t','l','e',0};
> +    struct custom_proppage page;
> +    HPROPSHEETPAGE hpsp;
> +    BOOL ret;
> +
> +    memset(&page.u.pageA, 0, sizeof(page.u.pageA));
> +    page.u.pageA.dwFlags = PSP_USECALLBACK;
> +    page.u.pageA.pfnDlgProc = page_dlg_proc_messages;
> +    page.u.pageA.pfnCallback = proppage_callback_a;
> +    page.u.pageA.lParam = (LPARAM)&page;
> +    page.u.pageA.pszTitle = "Title";
> +
> +    /* Only minimal size validation is performed */
> +    for (page.u.pageA.dwSize = PROPSHEETPAGEA_V1_SIZE - 1; page.u.pageA.dwSize <= PROPSHEETPAGEA_V4_SIZE + 1; page.u.pageA.dwSize++)
> +    {
> +        page.addref_called = 0;
> +        hpsp = CreatePropertySheetPageA(&page.u.pageA);
> +
> +        if (page.u.pageA.dwSize < PROPSHEETPAGEA_V1_SIZE)
> +        todo_wine
> +            ok(hpsp == NULL, "Expected failure, size %u\n", page.u.pageA.dwSize);
> +        else
> +        {
> +            ok(hpsp != NULL, "Failed to create a page, size %u\n", page.u.pageA.dwSize);
> +        todo_wine_if(page.u.pageA.dwSize > PROPSHEETPAGEA_V1_SIZE)
> +            ok(page.addref_called == (page.u.pageA.dwSize > PROPSHEETPAGEA_V1_SIZE) ? 1 : 0, "Expected ADDREF callback message\n");
> +        }
> +
> +        if (hpsp)
> +        {
> +            page.release_called = FALSE;

It would be better to use 0 when its a counter.
> +            ret = DestroyPropertySheetPage(hpsp);
> +            ok(ret, "Failed to destroy a page\n");
> +        todo_wine
> +            ok(page.release_called == 1, "Expected RELEASE callback message\n");
> +        }
> +    }
> +
> +    memset(&page.u.pageW, 0, sizeof(page.u.pageW));
> +    page.u.pageW.dwFlags = PSP_USECALLBACK;
> +    page.u.pageW.pfnDlgProc = page_dlg_proc_messages;
> +    page.u.pageW.pfnCallback = proppage_callback_w;
> +    page.u.pageW.lParam = (LPARAM)&page;
> +    page.u.pageW.pszTitle = titleW;
> +
> +    for (page.u.pageW.dwSize = PROPSHEETPAGEW_V1_SIZE - 1; page.u.pageW.dwSize <= PROPSHEETPAGEW_V4_SIZE + 1; page.u.pageW.dwSize++)
> +    {
> +        page.addref_called = 0;
> +        hpsp = CreatePropertySheetPageW(&page.u.pageW);
> +
> +        if (page.u.pageW.dwSize < PROPSHEETPAGEW_V1_SIZE)
> +        todo_wine
> +            ok(hpsp == NULL, "Expected failure, size %u\n", page.u.pageW.dwSize);
> +        else
> +        {
> +            ok(hpsp != NULL, "Failed to create a page, size %u\n", page.u.pageW.dwSize);
> +        todo_wine_if(page.u.pageA.dwSize > PROPSHEETPAGEA_V1_SIZE)

Shouldn't it be pageW here?
> +            ok(page.addref_called == (page.u.pageW.dwSize > PROPSHEETPAGEW_V1_SIZE) ? 1 : 0, "Expected ADDREF callback message\n");
> +        }
> +
> +        if (hpsp)
> +        {
> +            page.release_called = 0;
> +            ret = DestroyPropertySheetPage(hpsp);
> +            ok(ret, "Failed to destroy a page\n");
> +        todo_wine
> +            ok(page.release_called == 1, "Expected RELEASE callback message\n");
> +        }
> +    }
> +}
> +
>  START_TEST(propsheet)
>  {
>      test_title();
> @@ -998,4 +1139,5 @@ START_TEST(propsheet)
>      test_messages();
>      test_PSM_ADDPAGE();
>      test_PSM_INSERTPAGE();
> +    test_CreatePropertySheetPage();
>  }
> diff --git a/include/prsht.h b/include/prsht.h
> index f12f402afc..2b2a3e18f3 100644
> --- a/include/prsht.h
> +++ b/include/prsht.h
> @@ -293,6 +293,7 @@ DECL_PRSHT_TYPE_AW(LPCPROPSHEETPAGE, LATEST)
>  #define PSP_USEFUSIONCONTEXT    0x00004000
>  #define PSP_COMMANDLINKS        0x00040000
>
> +#define PSPCB_ADDREF            0
>  #define PSPCB_RELEASE           1
>  #define PSPCB_CREATE            2
>
> -- 2.11.0

[v2 PATCH 2/2] comctl32/tests: Some tests for property page initialization

[Sebastian Lackner]

On 19.02.2017 19:28, Sebastian Lackner wrote:
> > +            page.release_called = FALSE;
> > It would be better to use 0 when its a counter.

It is intentional that this was not changed in v3?

[v2 PATCH 2/2] comctl32/tests: Some tests for property page initialization

[Nikolay Sivov]

On 19.02.2017 22:02, Sebastian Lackner wrote:
> On 19.02.2017 19:28, Sebastian Lackner wrote:
> > > +            page.release_called = FALSE;
> > > It would be better to use 0 when its a counter.
>
> It is intentional that this was not changed in v3?

No, I skipped that while reading your reply.