Hi Sergey, I agree with you. Added two createFromImage methods with and without observer and removed the changes form CMenuItem, CCustomCursor and also from _AppDockIconHandler. Please review the updated code. cr.openjdk.java.net/~mhalder/8029250/webrev.02/ <http://cr.openjdk.java.net/~mhalder/8029250/webrev.02/>
Regards, Manajit > On 25-May-2018, at 2:04 PM, shashidhara.veerabhadra...@oracle.com wrote: > > Hi Manajit, The code changes looks fine to me but wanted to understand the > reason for creating the CImage object at updateImage() function. The reason > is that updateImage() is also called in the constructor and we can store the > CImage object that is created in the updateImage() once and reuse it every > time. Currently we load the image thro' mediatracker and wait for it and then > create the CImage object. I think that is not required. I do not understand > in detail of the CImage class but I think 'creation' at the time of 'update' > is not required. We can very well create once and use it every time we do > update. > > Thanks and regards, > > Shashi > > > On 25/05/18 12:53 AM, Sergey Bylokhov wrote: >> Hi, Manajit. >> I have only a few comments about a style: >> - I think it would be good to have two methods createFromImage() the >> old(without Observer) and new(with Observer). The old method should pass >> null to the new method. In this case CMenuItem/CCustomCursor will not be >> changed. >> - The parentheses around target are not necessary >> CTrayIcon.java:359 if (image != (target).getImage()) >> >> >> On 03/05/2018 04:11, Manajit Halder wrote: >>> Hi Sergey, >>> >>> Please review the updated webrev. >>> http://cr.openjdk.java.net/~mhalder/8029250/webrev.01/ >>> >>> The modified test case moved to closed test as it contains images with >>> unknown source. >>> >>> Regards, >>> Manajit >>> >>> >>> >>>> On 27-Apr-2018, at 4:06 PM, Manajit Halder <manajit.hal...@oracle.com >>>> <mailto:manajit.hal...@oracle.com>> wrote: >>>> >>>> Hi All, >>>> >>>> Kindly review the following AWT enhancement changes: >>>> >>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8029250 >>>> Webrev: http://cr.openjdk.java.net/~mhalder/8029250/webrev.00/ >>>> >>>> Fix: >>>> Added support for gif images (image animation) for Mac system tray. Before >>>> fix only single frame was passed to Mac OS system tray on mouse click from >>>> the Java side. >>>> After fix all the frames are passed at the time interval set in the image >>>> one by one to the Mac OS side. >>>> >>>> Note: >>>> The test was moved from closed test to open test along with 3 images: >>>> ball.gif, spot.gif and duke.gif. The test code was rewritten dropping the >>>> applet code used earlier. >>>> >>>> Regards, >>>> Manajit >>>> >>>> >>>> >>>> >>>> >>> >> >> >
