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

[Semyon Sadetsky]

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/~aghaisas/manajit/8147841/webrev.01/ 
> <http://cr.openjdk.java.net/%7Eaghaisas/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_
>>> _
>>> _
>>> *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/ef1ae0f2/attachment.html>