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

[Andy Goryachev]

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

modules/javafx.graphics/src/main/java/com/sun/javafx/css/StyleManager.java line 1063:

> 1061:             ** That way there in no information leaked.
> 1062:             */
> 1063:             catch (java.net.URISyntaxException e) {

are you sure the change is equivalent?

For example, the old code catches `URISyntaxException` and `PrivilegedActionException` returning `null`, but the new code does not, unless I am mistaken.

modules/javafx.graphics/src/main/java/com/sun/javafx/font/Disposer.java line 62:

> 60:             tgn != null;
> 61:             tg = tgn, tgn = tg.getParent());
> 62:         Thread t = new Thread(tg, disposerInstance, "Prism Font Disposer");

very minor: I would have separated `for()` from L62 by a newline.  This `for` is already confusing enough.

modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFile.java line 137:

> 135:                 isCopy = isDecoded = false;
> 136:             } catch (Exception e) {
> 137:             } finally {

even though the code change is equivalent, it will print "temp file not deleted" followed by "temp file deleted".

modules/javafx.graphics/src/main/java/com/sun/javafx/util/ModuleHelper.java line 39:

> 37: 
> 38:     static {
> 39:         verbose = Boolean.getBoolean("javafx.verbose");

minor I would rather moved it to L36

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

> 687:                 (PrivilegedAction<List<Window>>) () -> Window.getWindows(),
> 688:                 null,
> 689:                 ACCESS_WINDOW_LIST_PERMISSION);

@kevinrushforth once we finish removing AccessController, do we need to remove `FXPermissions` class?

modules/javafx.graphics/src/main/java/com/sun/marlin/MarlinProperties.java line 271:

> 269: 
> 270:     // system property utilities
> 271:     @SuppressWarnings("removal")

really, this should be in Utils.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824664938
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824770375
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824780352
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824784460
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824787462
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824788989