<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 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> 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/88e0b046/attachment-0001.html>