That looks good. Thanks.
--
best regards,
Anthony
On 3/20/2014 11:27 AM, Petr Pchelko wrote:
Hello, Anthony.
I’ve updated the patch: http://cr.openjdk.java.net/~pchelko/9/8037840/webrev.02/
Now the CancelableFuture methods are final.
With best regards. Petr.
19 марта 2014 г., в 8:49 после полудня, Anthony Petrov
<anthony.pet...@oracle.com> написал(а):
Is there a reason to not declare the @Override CancelableRunnable.run() as
final? Allowing descendants to override it may result in violation of the
cancelable contract.
--
best regards,
Anthony
On 3/19/2014 7:21 PM, Petr Pchelko wrote:
Hello, Anthony.
Thank you for the review.
src/macosx/native/sun/awt/LWCToolkit.m:
I'd like to see the NewGlobalRef/DeleteGlobalRef calls to be more balanced.
Both should be performed either in the utility @interface, or in the JNI method
(I'd prefer the former).
Unfortunately that's impossible. We must call the NewGlobalRef on the calling
thread (in the JNI method), while DeleteGlobalRef could only be called after
the task was executed (either in perform or in dealloc).
Dealloc looks better for me as it's a common pattern to release resources in
dealloc.
Other comments are addressed in the new version of the fix:
http://cr.openjdk.java.net/~pchelko/9/8037840/webrev.01/
With best regards. Petr.
On 19.03.2014, at 18:58, Anthony Petrov <anthony.pet...@oracle.com> wrote:
Hi Petr,
src/macosx/native/sun/awt/LWCToolkit.m:
I'd like to see the NewGlobalRef/DeleteGlobalRef calls to be more balanced.
Both should be performed either in the utility @interface, or in the JNI method
(I'd prefer the former).
src/macosx/classes/sun/lwawt/macosx/CWarningWindow.java
From Object-Oriented Design perspective, the CancelableRunnable doesn't
enforce the Cancelable contract. I'd suggest to redesign this class as follows
(pseudo-code):
CancelableRunnable {
private boolean flag;
public abstract void perform();
final run() {
if (flag) {
perform();
}
}
}
and the implementation classes would only ever override the perform() method.
This way we hide the flag from descendant classes, and also ensure the correct
behavior of any descendant class after its cancel() method is invoked.
Also note the single 'l' in Cancelable. There's already CancelablePrintJob
class in JDK, so I'd suggest to keep using the same flavor of English.
--
best regards,
Anthony
On 3/19/2014 6:15 PM, Petr Pchelko wrote:
Hello, AWT Team.
Please review the fix for the issue:
https://bugs.openjdk.java.net/browse/JDK-8037840
The fix is available at: http://cr.openjdk.java.net/~pchelko/9/8037840/webrev/
This is another fix for the SecurityWarning window. The problem is that we
create a new Java thread to animate the warning icon. And we have a thread per
icon.
That's an overkill and a waste of resources. The idea of this fix is to use
Appkit to schedule delayed tasks.
You might wonder, why I'm rewriting the code I've just fixed in JDK-8037776.
The idea is that JDK-8037776 is a completely safe fix to backport it to 8u20.
This fix is a bit more risky, so it will stay in 9 without a backport to 8u20.
