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