RFR: 8342993: Remove uses of AccessController and AccessControlContext from JavaFX [v2]

[Andy Goryachev]

On Thu, 14 Nov 2024 22:53:06 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> This PR removes all remaining uses of `AccessController` and `AccessControlContext`, which represent the last remaining uses of the terminally deprecated security APIs except for those in the `/ios/` or `/android/` directories.
>> 
>> With the removal of doPrivileged and the `if (System.getSecurityManager() != null)` code paths, the ACC is no longer used, so can be completely eliminated. Along with this, I removed all unused imports of security-related APIs and all related `@SuppressWarnings("removal") annotations.
>> 
>> ### Notes to reviewers
>> 
>> * Most of the changes were straight-forward removals of methods and fields to save, retrieve and pass around the `AccessControlContext`.
>> * The Toolkit class stores a collection of listeners in a `WeakHashMap` with the listener as the key (thus weakly held) and the ACC as the value. We no longer need or want the ACC, but I kept the use of `WeakHashMap` and changed the value type to `Object`, storing a singleton dummy object as the value for each entry. This minimizes the changes, while preserving the behavior of reclaiming the entries when they are garbage collected.
>
> Kevin Rushforth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   additional comments

looks good, with some minor comments.

I don't know if @mstr2 's idea of converting weak hash maps to sets would help, since it does not save any memory (might actually eat more memory).  putting a `null` value might reduce the memory footprint, but needs to be checked if it would work, and putting a Boolea.TRUE would work without adding a dummy object.

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/Toolkit.java line 98:

> 96: 
> 97:     // Used as the (dummy) value for the various listener maps
> 98:     private static final Object dummyObj = new Object();

minor: maybe use all-uppercase name for this static final object?

we could also be using Boolean.TRUE (or null), since there is a comment down below explaining the purpose of these maps.

curiously, there is no WeakHashSet.

modules/javafx.web/src/main/java/com/sun/webkit/dom/JSObject.java line 45:

> 43:     // We do this, rather than removing the parameter, in order to keep the
> 44:     // native WebKit code the same across different release families.
> 45:     private static final Object dummyAcc = new Object();

very minor: all uppercase name

modules/javafx.web/src/main/java/com/sun/webkit/dom/JSObject.java line 120:

> 118:     public void setSlot(int index, Object value) throws JSException {
> 119:         Invoker.getInvoker().checkEventThread();
> 120:         setSlotImpl(peer, peer_type, index, value,dummyAcc);

missing space after `,`

modules/javafx.web/src/main/java/com/sun/webkit/dom/JSObject.java line 129:

> 127:     public Object call(String methodName, Object... args) throws JSException {
> 128:         Invoker.getInvoker().checkEventThread();
> 129:         return callImpl(peer, peer_type, methodName, args,dummyAcc);

missing space after `,`

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

PR Review: https://git.openjdk.org/jfx/pull/1638#pullrequestreview-2437376185
PR Review Comment: https://git.openjdk.org/jfx/pull/1638#discussion_r1842997206
PR Review Comment: https://git.openjdk.org/jfx/pull/1638#discussion_r1843010983
PR Review Comment: https://git.openjdk.org/jfx/pull/1638#discussion_r1843011173
PR Review Comment: https://git.openjdk.org/jfx/pull/1638#discussion_r1843011305