<AWT Dev> [9] Review Request: JDK-8037840 [macosx] Rewrite CWarning window to eliminate the ExecutorService

[Anthony Petrov]

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