<AWT Dev> <AWT dev>[9] Review request for JDK-8147841: [macosx] Updating TrayIcons popup menu does not work on Mac OS X

[Manajit Halder]

Hi Prasanta,

Could you please push the patch to JDK9 with the following details:

http://cr.openjdk.java.net/~aghaisas/manajit/8147841/webrev.01/ <http://cr.openjdk.java.net/~aghaisas/manajit/8147841/webrev.01/>


8147841: [macosx] Updating TrayIcons popup menu does not work on Mac OS X
Reviewed-by: serb, ssadetsky
username: mhalder

Thanks,
Manajit

> On 13-Apr-2016, at 2:37 pm, Manajit Halder <manajit.halder at oracle.com> wrote:
> 
> Hi Sergey and Semyon,
> 
> Thank you both for the review comments.
> 
> Thanks,
> Manajit
> 
>> On 13-Apr-2016, at 2:01 pm, Semyon Sadetsky <semyon.sadetsky at oracle.com <mailto:semyon.sadetsky at oracle.com>> wrote:
>> 
>> Looks good. 
>> 
>> --Semyon
>> 
>> On 4/1/2016 1:58 PM, Manajit Halder wrote:
>>> Hi Sergey,
>>> 
>>> Thank you for your review comments. The following scenarios were taken care while modifying the code along with your comments.
>>> 
>>> Case 1) popup and newPopup refers to same object. In cases where the same popup menu is used thorughout the execution of the program or both of them are null.
>>>     a) first popup menu is null 
>>> 		return 0 as address.
>>>     b) first popup menu is not null
>>> 		poupup is already set in the constructor.
>>>     
>>> Case 2) popup and newPopup refers to different objects.
>>>     a) newPopup is not null.
>>>         i) popup is not null 
>>> 		call removeNotify() on popup and set it to newPopup
>>>         ii) popup is null 
>>> 		set popup to newPoup
>>>     b) newPopup is null then return 0 address (In this case the first popup menu is not checked for null because we are interested in the newPopup).
>>> 
>>> Please review the modified code @ 
>>>  <http://cr.openjdk.java.net/%7Eaghaisas/manajit/8147841/webrev.01/>http://cr.openjdk.java.net/~aghaisas/manajit/8147841/webrev.01/ <http://cr.openjdk.java.net/~aghaisas/manajit/8147841/webrev.01/>
>>> 
>>> Thanks,
>>> Manajit
>>> 
>>>> On 30-Mar-2016, at 11:53 pm, Sergey Bylokhov <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>>>> 
>>>> Hi, Manajit.
>>>> Please double check two cases.
>>>> - The initial popup menu is set to null in CTrayIcon(); it seems after the fix it will never be changed to non-null value.
>>>> - Who and when will call removeNotify() on the old popup menu after you replace it by the new popup. it will be leaked?
>>>> 
>>>> On 30.03.16 15:45, Manajit Halder wrote:
>>>>> Hi All,
>>>>> 
>>>>> Kindly review the fix for JDK9.
>>>>> 
>>>>> *Bug*:
>>>>> _https://bugs.openjdk.java.net/browse/JDK-8147841_ <https://bugs.openjdk.java.net/browse/JDK-8147841_>
>>>>> _
>>>>> _
>>>>> *Webrev*:
>>>>> http://cr.openjdk.java.net/~arapte/manajit/8147841/webrev.00/ <http://cr.openjdk.java.net/%7Earapte/manajit/8147841/webrev.00/>
>>>>> 
>>>>> *Issue: *
>>>>> [macosx] Updating TrayIcons popup menu does not work on Mac OS X.
>>>>> 
>>>>> *Cause: *
>>>>> Wrong address of Popup menu was referred by native code (Mac OS). The
>>>>> native side was always referring to the popup menu created initially and
>>>>> the popup menu created later was not referred by the native code. Popup
>>>>> menu was getting updated internally (in java code), the new popup menu
>>>>> was not getting accessed/used by the native code (mac os x).
>>>>> 
>>>>> Explanation with code points as below:
>>>>> 
>>>>> File CTrayIcon.m:
>>>>> On mouseDown event java method getPopupMenuModel was called
>>>>> using JNFCallLongMethod. getPopupMenuModel on the java side was always
>>>>> returning the address of the popup menu created initially and because of
>>>>> this updated popup menus was not getting shown on clicking the Tray icon.
>>>>> 
>>>>> *Fix: *
>>>>> In method getPopupMenuModel the address of the new popup menu is updated
>>>>> in case there is a new popup menu. If the new popup points to null then
>>>>> 0 is returned as address. And in cases the new popup and the initial
>>>>> popup menu are same then the initial popup menu's address is returned.
>>>>> 
>>>>> 
>>>>> Regards,
>>>>> Manajit
>>>> 
>>>> 
>>>> -- 
>>>> Best regards, Sergey.
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160413/39046d08/attachment.html>