RFR: 8325900: Emit a warning on macOS if AWT has set the NSAppearance [v2]

[Kevin Rushforth]

On Thu, 15 Feb 2024 04:20:15 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Platform preferences detection doesn't pick up effective macOS system preferences if AWT owns the NSApplication and has set its NSAppearance to a fixed value.
>> 
>> The workaround is to set the system property "apple.awt.application.appearance=system".
>> 
>> If this property is not set, the following warning will be emitted if a JavaFX application attempts to use the platform preferences API:
>> 
>> 
>> WARNING: Reported preferences may not reflect the macOS system preferences unless the sytem
>> property apple.awt.application.appearance=system is set. This warning can be disabled by
>> setting javafx.preferences.suppressAppleAwtWarning=true.
>
> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Don't call into the native toolkit from Platform.getPreferences()

The code looks fine other than missing `doPrivileged` calls. I left a couple other minor suggestions. I haven't tested it yet, but will do so next week.

Since we are defining a new system property a CSR might be in order (let me think about it).

modules/javafx.graphics/src/main/java/com/sun/glass/ui/Application.java line 806:

> 804:     /**
> 805:      * Checks whether there are any problems with platform preferences detection,
> 806:      * and emits a warning otherwise.

Suggestion: change "otherwise" to "if so" or similar (the current comment indicates that a warning will be produced if there are no problems).

modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacApplication.java line 466:

> 464:     private static final String SUPPRESS_AWT_WARNING_PROPERTY = "javafx.preferences.suppressAppleAwtWarning";
> 465:     private static final String AWT_APPEARANCE_PROPERTY = "apple.awt.application.appearance";
> 466:     private boolean checkSystemAppearance = !Boolean.getBoolean(SUPPRESS_AWT_WARNING_PROPERTY);

This needs to be wrapped in a `doPrivileged` call, with a  `@SuppressWarnings` annotation, or it will fail to run with a security manager (which we still support as of now, even though it is deprecated).

See, for example, `PlatformImpl.java` for the pattern to use.

modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacApplication.java line 470:

> 468:     @Override
> 469:     public void checkPlatformPreferencesSupport() {
> 470:         if (checkSystemAppearance && "NSApplicationAWT".equals(applicationClassName)) {

Minor: consider using a static constant for the name of the AWT app class?

modules/javafx.graphics/src/main/java/com/sun/glass/ui/mac/MacApplication.java line 473:

> 471:             checkSystemAppearance = false;
> 472: 
> 473:             if (!"system".equals(System.getProperty(AWT_APPEARANCE_PROPERTY))) {

You need to wrap the call to `System.getProperty` in `doPrivileged (so you will need to create a local var).

-------------

PR Review: https://git.openjdk.org/jfx/pull/1367#pullrequestreview-1886580420
PR Review Comment: https://git.openjdk.org/jfx/pull/1367#discussion_r1493344795
PR Review Comment: https://git.openjdk.org/jfx/pull/1367#discussion_r1493345874
PR Review Comment: https://git.openjdk.org/jfx/pull/1367#discussion_r1493346242
PR Review Comment: https://git.openjdk.org/jfx/pull/1367#discussion_r1493346477