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.


-- 
Rob Shearman

systray[3/4]: Better validate icon owner

[Reece Dunn]

On 06/02/2008, Robert Shearman <rob(a)codeweavers.com> 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 <rob(a)codeweavers.com> 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)".

-- 
Rob Shearman