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.

Reply via email to