RFR: 8301302: Platform preferences API [v18]

[Nir Lisker]

On Tue, 31 Oct 2023 17:28:35 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Please read [this document](https://gist.github.com/mstr2/9f46f92c98d3c86aa6a0b4224a9a6548) for an introduction to the Platform Preferences API, and how it interacts with the proposed style theme and stage appearance features.
>
> Michael Strauß has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - formatting
>  - Javadoc change

modules/javafx.graphics/src/main/java/com/sun/javafx/application/PlatformImpl.java line 1098:

> 1096:         } else {
> 1097:             setAccessibilityTheme(null);
> 1098:         }

Can this not be written more simply as

    String val = Boolean.TRUE.equals(preferences.get("Windows.SPI.HighContrastOn"))
              && preferences.get("Windows.SPI.HighContrastColorScheme") instanceof String s ? s : null;
    setAccessibilityTheme(val);

?

modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 61:

> 59:     final Map<String, Object> effectivePreferences = new HashMap<>();
> 60:     final Map<String, Object> unmodifiableEffectivePreferences = Collections.unmodifiableMap(effectivePreferences);
> 61:     final PreferenceProperties properties = new PreferenceProperties(this);

Why are these not `private`? I don't see them being used outside of this class.

Also, if possible, add some documentation on these fields so maintainers can understand better what each is for,

modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 67:

> 65: 
> 66:     public PlatformPreferences(Map<String, String> wellKnownKeys) {
> 67:         this.wellKnownKeys = wellKnownKeys;

Is the argument map guaranteed to not change outside of this class, or is there a need for a defensive copy?

modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 192:

> 190:     /**
> 191:      * Updates this map of preferences with a new set of platform preferences.
> 192:      * The specified preferences may include all available preferences, or only the changed preferences.

I would mention here that removed preferences are specified by being mapped to `null`, but that the resulting preferences (after update) cannot map to `null`.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378839517
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1378817922
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1380059912
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1380097328