Re: [PATCH 04/18] comctl32: Extend TaskDialog, add simple callback and tests

[Nikolay Sivov]

On 25.02.2017 19:01, Fabian Maurer wrote:
> +/* FIXME: Make thread safe? */
> +static const TASKDIALOGCONFIG *task_config = 0;
> +static HRESULT callback(HWND hwnd, UINT uNotification, WPARAM wParam, LPARAM lParam)
> +{
> - if(task_config->pfCallback)
> -
>    return task_config->pfCallback(hwnd, uNotification, wParam, lParam, task_config->lpCallbackData);
> - return S_OK;
>
> +}
> -
>
> static INT_PTR CALLBACK DialogProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
>  {
> - HRESULT ret_callback;
> - switch (uMsg)
>    {
> -
>    case WM_INITDIALOG:
> -
>        callback(hwndDlg, TDN_DIALOG_CONSTRUCTED, 0, 0);
> -
>        callback(hwndDlg, TDN_CREATED, 0, 0);
> -
>        return TRUE;

How is this going to work with global variable?
> +/* For message checking we use a simplified version from the code used for user32 messages */
> -
>
> +#define TDN_NO_MORE_MESSAGES 0xffffffff
> -
>
> +typedef struct
> +{
> - UINT   msg;
> - WPARAM wparam;
> - LPARAM lparam;
> -
> - struct list entry;
>
> +}message_data;
> -
>
> +static struct list messages = LIST_INIT(messages);
> -
>
> +/* Message lists to test against */
> -
>
> +static const message_data mes_simple_show[] = {
> - { TDN_DIALOG_CONSTRUCTED, 0, 0 },
> - { TDN_CREATED, 0, 0 },
> - { TDN_BUTTON_CLICKED, IDOK, 0 },
> - { TDN_DESTROYED, 0, 0 },
> - { TDN_NO_MORE_MESSAGES }
>
> +};
> -
>
> +/* Functions handling message processing */
> -
>
> +static void message_add(UINT msg, WPARAM wparam, LPARAM lparam)
> +{
> - message_data *data = HeapAlloc(GetProcessHeap(), 0, sizeof(message_data));
> -
> - data->msg = msg;
> - data->wparam = wparam;
> - data->lparam = lparam;
> -
> - list_add_tail(&messages, &data->entry);
>
> +}
> -
>
> +#define ok_sequence(exp ) \
> -
>    ok_sequence_((exp), __FILE__, __LINE__)
> -
>
> +static void ok_sequence_(const message_data *expected_list, const char *file, int line)
> +{
> - message_data *message;
> - BOOL dump = FALSE;
> - UINT count;
> -

For that please use msg.h from comctl32/tests.

[PATCH 04/18] comctl32: Extend TaskDialog, add simple callback and tests

[Fabian Maurer]

> How is this going to work with global variable?

Well, it's just a first implementation, I removed the global variables in later 
patchsets. But I'll rewrite it to store the taskdialog-data using dialog 
functions.
> For that please use msg.h from comctl32/tests.

I'd love to, but I'm not sure how this would work.
After all, the message lists for the taskdialogs use a different struct with 
additional values, like the return value it should give, and a sync value.
I agree that the current way isn't too elegant, how would you go about that?

Regards,
Fabian Maurer

[PATCH 04/18] comctl32: Extend TaskDialog, add simple callback and tests

[Fabian Maurer]

> > For that please use msg.h from comctl32/tests.
>
> I'd love to, but I'm not sure how this would work.
> After all, the message lists for the taskdialogs use a different struct with
> additional values, like the return value it should give, and a sync value.
> I agree that the current way isn't too elegant, how would you go about
> that?

Thought about it a while longer, but it's not easy.
The current code uses
> while (expected->message && actual->message)

to process the messages.
However, the problem is that TDN_CREATED is zero, terminating the loop way to 
early. That's why I added TDN_NO_MORE_MESSAGES in the first place. If I should 
use msg.h from comctl32, then I need to rewrite a bunch of its code.

How do we want to handle it?

Regards,
Fabian Maurer

[PATCH 04/18] comctl32: Extend TaskDialog, add simple callback and tests

[Nikolay Sivov]

On 26.02.2017 22:17, Fabian Maurer wrote:
> > > For that please use msg.h from comctl32/tests.
> >
> > I'd love to, but I'm not sure how this would work.
> > After all, the message lists for the taskdialogs use a different struct with
> > additional values, like the return value it should give, and a sync value.
> > I agree that the current way isn't too elegant, how would you go about
> > that?
>
> Thought about it a while longer, but it's not easy.
> The current code uses
> > while (expected->message && actual->message)
>
> to process the messages.
> However, the problem is that TDN_CREATED is zero, terminating the loop way to 
> early. That's why I added TDN_NO_MORE_MESSAGES in the first place. If I should 
> use msg.h from comctl32, then I need to rewrite a bunch of its code.
>
> How do we want to handle it?

Right, that's because it was meant for testing actual window messages.
We can easily abuse it. You can make up a new message above WM_USER or
even WM_APP, like WM_TD_CALLBACK, and use existing 'id' flag like
WM_NOTIFY messages are already using. After that you'll only need to set
id to TDN_CREATED, and add_message() with resulting structure. This 'id'
field does not have any special meaning in generic code in msg.h and is
only checked for equality.
> Regards,
> Fabian Maurer