RFR: 8342456: Remove calls to doPrivileged in javafx.graphics/other [v2]

[Kevin Rushforth]

On Thu, 31 Oct 2024 14:09:52 GMT, Lukasz Kostyra <lkostyra at openjdk.org> wrote:

>> This PR removes uses of `AccessControler.doPrivileged()` from `javafx.graphics` package. All uses of `doPrivileged()` were removed _excluding_ `com.sun.javafx.tk` and `com.sun.ui.glass` subpackages - those are handled by [JDK-8342453](https://bugs.openjdk.org/browse/JDK-8342453) (already completed) and [JDK-8342454](https://bugs.openjdk.org/browse/JDK-8342454) respectively.
>> 
>> Most of these changes are fairly straightforward and follow the standard replacement from `AccessController.doPrivileged(LAMBDA)` to just simply `LAMBDA` with a couple exceptions:
>> 
>> - `StyleManager.java` where `loadStylesheet()` followed two steps - attempting straightforward stylesheet load via `loadStylesheetUnPrivileged()` and in case of `AccessControlException` retrying the load via `AccessController.doPrivileged` with read-only access to stylesheet JAR. Now that `doPrivileged` is deprecated and not used anymore, `loadStylesheetUnPrivileged()` was always successful and the fallback became essentially dead code. Fallback was removed and `loadStylesheetUnPrivileged()` was renamed to `loadStylesheet()`.
>> - `Scene.java` integrates with `com.sun.javafx.tk` so only straightforward `doPrivileged()` calls were removed - `AccessControlContext` remained, as it needs to be removed from both tk and Scene via a follow-up ([JDK-8342993](https://bugs.openjdk.org/browse/JDK-8342993))
>> 
>> Building graphics module now produces less warnings than before this change with no new warning messages (all warnings refer to the use of `Unsafe` which is out of scope of this change).
>
> Lukasz Kostyra has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Scene: Remove missed doPrivileged use

Overall this looks good.

Here is my first batch of comments (there might be more, since I haven't looked at all files yet).

modules/javafx.graphics/src/main/java/com/sun/javafx/util/Utils.java line 684:

> 682:     public static boolean hasFullScreenStage(final Screen screen) {
> 683:         @SuppressWarnings("removal")
> 684:         final List<Window> allWindows = Window.getWindows();

After this change there is an unused import of `ACCESS_WINDOW_LIST_PERMISSION` and the `SuppressWarnings` annotation is no longer needed.

modules/javafx.graphics/src/main/java/com/sun/marlin/RendererStats.java line 362:

> 360:         private RendererStatsHolder() {
> 361:             AccessController.doPrivileged(
> 362:                 (PrivilegedAction<Void>) () -> {

There are unused imports after this change.

modules/javafx.graphics/src/main/java/javafx/scene/PropertyHelper.java line 38:

> 36:             @SuppressWarnings("removal")
> 37:             boolean answer =
> 38:                 AccessController.doPrivileged((java.security.PrivilegedAction<Boolean>) () -> {

Following the removal of the doPrivileged call, the second sentence in the comment block of this method -- starting with "Note that ..." -- is wrong and can be removed.

modules/javafx.graphics/src/main/java/javafx/scene/Scene.java line 1448:

> 1446:         final AccessControlContext acc = AccessController.getContext();
> 1447:         snapshotRunnableList.add(() -> {
> 1448:             AccessController.doPrivileged((PrivilegedAction<Void>) () -> {

I think that this method has a now-unneeded `SuppressWarnings` annotation.

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

PR Review: https://git.openjdk.org/jfx/pull/1619#pullrequestreview-2408466054
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824820526
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824775661
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824792621
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824782309