RFR: 8301302: Platform preferences API [v46]

[Michael Strauß]

On Thu, 7 Dec 2023 05:49:07 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> Michael Strauß has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   renamed Windows.SPI.HighContrastOn to Windows.SPI.HighContrast
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/application/WindowsHighContrastScheme.java line 74:
> 
>> 72:         if (themeName == null) {
>> 73:             return null;
>> 74:         }
> 
> This method is called only from `PlatformImpl` that already does the `null` check on the the string. In general, `null` checks should be done on the "outer most layer" and then all the inner layers can rely on the value being non-null.
> 
> Is this method expected to be called from other places as well? If not, the method can be made package visible.

The method now returns `NONE` when another constant doesn't apply. I've removed the `public` modifier as you've suggested.

> modules/javafx.graphics/src/main/java/com/sun/javafx/application/preferences/PlatformPreferences.java line 79:
> 
>> 77: 
>> 78:     private final List<InvalidationListener> invalidationListeners = new CopyOnWriteArrayList<>();
>> 79:     private final List<MapChangeListener<? super String, Object>> mapChangeListeners = new CopyOnWriteArrayList<>();
> 
> Can these be modified concurrently? What is the need for `CopyOnWriteArrayList`?

It's to prevent `ConcurrentModificationException` if a listener implementation adds or removes itself (or another listener).

> modules/javafx.graphics/src/main/java/javafx/application/Platform.java line 37:
> 
>> 35: import javafx.scene.input.KeyCode;
>> 36: import javafx.scene.paint.Color;
>> 37: import javafx.scene.paint.Paint;
> 
> Unused import.

Fixed.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1419304904
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1419307136
PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1419308447