systray[3/4]: Better validate icon owner

[Robert Shearman]

Kirill K. Smirnov wrote:
>> 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.
>>     
>
> Just a testcase:
> 1) run any wine app (to be sure that explorer is running), e.g winecfg
> 2) run taskmgr
> 3) select taskmgr process in tasklist and kill it. taskmgr window will 
> dissapear but tray icon not.
> 4) move mouse cursor over taskmgr tray icon. Before this patch it will not 
> dissapear, but should.
>   

This is pretty much the same test I did when I originally wrote the code.

> The reason is that ERROR_INVALID_HANDLE is not the only value which indicates 
> the absence of owner window (ERROR_INVALID_WINDOW_HANDLE too). I think that 
> using robust and straightforward IsWindow() function is better than verify 
> last error.
>   

The error value was changed in commit 
19e7fab981a75cdf0a339db35d36d9c1a6e64494. The code should be changed to 
check for ERROR_INVALID_WINDOW_HANDLE now.

> Is it acceptable to check return value of PostMessage only and do not check 
> last error?
>   

Possibly, although I can think of other errors such as out-of-memory 
that probably shouldn't indicate that the parent has disappeared and the 
icon destroyed.

> And what race condition introduces IsWindow() here?
>   

In general, you shouldn't check whether something can be done before you 
try doing it. Just try doing it and cope with it failing.

In this case, you're checking if the window is valid before you post a 
message to it. The window could be destroyed between IsWindow and 
PostMessage, in which case you'd have to detect the error by checking 
the result of the PostMessage call. After you've done this, the IsWindow 
call is redundant.

-- 
Rob Shearman