Re: systray[3/4]: Better validate icon owner

[Robert Shearman]

Kirill K. Smirnov wrote:
> @@ -131,15 +130,18 @@ static LRESULT WINAPI adaptor_wndproc(HW
>          case WM_LBUTTONDBLCLK:
>          case WM_RBUTTONDBLCLK:
>          case WM_MBUTTONDBLCLK:
> -
>        /* notify the owner hwnd of the message */
> -
>        WINE_TRACE("relaying 0x%x\n", msg);
> -
>        ret = PostMessage(icon->owner, icon->callback_message, (WPARAM) icon->id, (LPARAM) msg);
> -
>        if (!ret && (GetLastError() == ERROR_INVALID_HANDLE))
> -
>        if (!IsWindow(icon->owner))
>        {
>            WINE_WARN("application window was destroyed without removing "
>                      "notification icon, removing automatically\n");
>            delete_icon_directly(icon);
>        }
> -
>        else
> -
>        {
> -
>            /* notify the owner hwnd of the message */
> -
>            WINE_TRACE("relaying 0x%x\n", msg);
> -
>            PostMessage(icon->owner, icon->callback_message, (WPARAM) icon->id, (LPARAM) msg);
> -
>        }
>        break;
>
>    case WM_NCDESTROY:

I don't get why you need this change. PostMessage should correctly 
handle the case where icon->owner has been destroyed and adding a call 
to IsWindow just introduces a race condition.

systray[3/4]: Better validate icon owner

[Reece Dunn]

On 06/02/2008, Robert Shearman  wrote:
> Kirill K. Smirnov wrote:
> > @@ -131,15 +130,18 @@ static LRESULT WINAPI adaptor_wndproc(HW
> >          case WM_LBUTTONDBLCLK:
> >          case WM_RBUTTONDBLCLK:
> >          case WM_MBUTTONDBLCLK:
> > -
> >        /* notify the owner hwnd of the message */
> > -
> >        WINE_TRACE("relaying 0x%x\n", msg);
> > -
> >        ret = PostMessage(icon->owner, icon->callback_message, (WPARAM) icon->id, (LPARAM) msg);
> > -
> >        if (!ret && (GetLastError() == ERROR_INVALID_HANDLE))
> > -
> >        if (!IsWindow(icon->owner))
> >        {
> >            WINE_WARN("application window was destroyed without removing "
> >                      "notification icon, removing automatically\n");
> >            delete_icon_directly(icon);
> >        }
> > -
> >        else
> > -
> >        {
> > -
> >            /* notify the owner hwnd of the message */
> > -
> >            WINE_TRACE("relaying 0x%x\n", msg);
> > -
> >            PostMessage(icon->owner, icon->callback_message, (WPARAM) icon->id, (LPARAM) msg);
> > -
> >        }
> >        break;
> >
> >    case WM_NCDESTROY:
>
> I don't get why you need this change. PostMessage should correctly
> handle the case where icon->owner has been destroyed and adding a call
> to IsWindow just introduces a race condition.

Is there a test case in the PostMessage tests to verify that behaviour?

- Reece

systray[3/4]: Better validate icon owner

[Robert Shearman]

Reece Dunn wrote:
> On 06/02/2008, Robert Shearman  wrote:
> > PostMessage should correctly
> > handle the case where icon->owner has been destroyed and adding a call
> > to IsWindow just introduces a race condition.
>
> Is there a test case in the PostMessage tests to verify that behaviour?

No - I don't think the tests will tell us anything we don't already 
know, but don't let me stop you adding a test for 
"PostMessage((HWND)0xdeadbeef, WM_USER, 0, 0)".