<AWT Dev> <AWT-Dev> Review request for CR 8005629: javac warnings compiling java.awt.EventDispatchThread and sun.awt.X11.XIconWindow

[Sergey Bylokhov]

Hi, Petr.
Fix looks good.

01.02.2013 16:01, Petr Pchelko wrote:
> Hello, Artem.
>
> Thank you for the review. Here is the new version of the fix:
> http://cr.openjdk.java.net/~pchelko/8005629/webrev.01/
>
>> 1. Changes in XIconWindow seem to be unrelated to the fix.
> The bug is about compiler warnings on Linux, and this one is pointed in the bug, so I think I should fix it
>
>> 2. Did you try to improve the fix even further and get rid of the CPrinterJob.performingPrinting field? It's used in a single place and looks redundant (but it should be double-checked, of course).
> It is used a bit in the superclass. It could be removed, however it does not really make the code cleaner or more understandable, so I'd better leave it on it's place.
>
>> 3. , 4.
> Done.
>
> With best regards. Petr.
>
> On Jan 30, 2013, at 5:30 PM, Artem Ananiev wrote:
>
>> Hi, Petr,
>>
>> a few comments:
>>
>> 1. Changes in XIconWindow seem to be unrelated to the fix.
>>
>> 2. Did you try to improve the fix even further and get rid of the CPrinterJob.performingPrinting field? It's used in a single place and looks redundant (but it should be double-checked, of course).
>>
>> 3. Toolkit.getSystemEventQueue() is protected with a security check. You need to call it using doPrivileged().
>>
>> 4. Current SecondaryLoop implementation in AWT depends on the event dispatch thread. In exceptional cases, the dispatch thread can die and be re-created, which may cause already created secondary loops to behave incorrectly. That's why I would suggest to use a new secondary loop each time CPrinterJob.print() is called.
>>
>> Thanks,
>>
>> Artem
>>
>> On 1/24/2013 8:19 PM, Petr Pchelko wrote:
>>> Hello, AWT team.
>>>
>>> Please review the fix for the issue:
>>> http://bugs.sun.com/view_bug.do?bug_id=8005629
>>> The fix is available at:
>>> http://cr.openjdk.java.net/~pchelko/8005629/webrev.00/
>>>
>>> I've rewritten the printing logic so that it uses a SecondaryLoop now instead of using package-private Conditional. So the EventDispatchAcces class and _macosxGetConditional methods are not needed any more.
>>> There change in XIconWindow is simple, just corrected the class-cast for a vararg method.
>>>
>>> With best regards. Petr.
>>>


-- 
Best regards, Sergey.