RFR: 8342456: Remove calls to doPrivileged in javafx.graphics/other [v2]
Kevin Rushforth
kcr at openjdk.org
Thu Oct 31 15:58:42 UTC 2024
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 953:
> 951: try {
> 952: return loadStylesheetUnPrivileged(fname);
> 953: } catch (java.security.AccessControlException ace) {
As a note for other reviewers, @lukostyra and I discussed this offline, and determined that it was best to remove the `try` / `catch` along with the entire `catch` block. `AccessControlException` is no longer possible, so this was slated for removal as part of subsequent follow-up bug [JDK-8342998](https://bugs.openjdk.org/browse/JDK-8342998) already. By removing it as part of _this_ PR, it saves the effort of removing the `doPrivileged` calls and fixing up the unused permissions in dead code only to have them removed shortly afterwards.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1619#discussion_r1824726316
More information about the openjfx-dev
mailing list