On Fri, 8 Sep 2023 14:31:07 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Looks like I spoke too soon. Mappings can actually be removed at runtime, 
>> one such example is `Windows.SPI.HighContrastColorScheme`, which is only 
>> available if `Windows.SPI.HighContrast` is `true`.
>> 
>> This doesn't change the `ChangedValue` implementation, and there are tests 
>> that cover the key removal scenario.
>
> if that's the case, isn't it possible to have the same key added at runtime?  
> this scenario cannot be handled by the current code, as far as I can tell.

I'm not sure if I understand what you mean that the same key can be added at 
runtime. The protocol is as follows:

1. The native toolkit reports either _all_ preferences, or the _changed_ 
preferences (or a mixture of both). If a report includes only changed 
preferences, this _doesn't_ imply that unreported preferences (that were 
reported earlier) are removed.
2. `PlatformPreferences.update` computes the difference between the _currently 
known_ preferences, and the _reported_ preferences. This is what 
`ChangedValue.getEffectiveChanges` does. A mapping that is not included in the 
reported preferences doesn't mean that it was removed; if the native toolkit 
wants to signal removal, it needs to map the respective key to `null` instead.
3. If there are any _effective changes_ (i.e. changes where the current value 
is actually different from the last known value), change notifications are 
fired. This includes additions (where new keys are seen for the first time), as 
well as removals (where a known key is now mapped to `null`).

This protocol gives native toolkits the option to choose how preferences are 
reported, some toolkits may always want to report all preferences, while other 
toolkits may only want to report the changed preferences. The burden of keeping 
track of the total set of preferences is placed on `PlatformPreferences`, not 
on the native toolkit. This allows us to implement the book-keeping logic in a 
single place, rather than in each toolkit individually.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1014#discussion_r1320119628

Reply via email to