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

[Petr Pchelko]

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.
>>