On Mon, 18 Nov 2024 20:18:39 GMT, Harshitha Onkar <[email protected]> wrote:
> Post JEP-486 (Permanently Disable the Security Manager) cleanup.
> Calls to java.security.AccessController.doPrivileged are obsolete thus
> removed in this PR.
>
> This PR addresses removal of AccessController.doPrivileged() calls from
> unix-platform files in the java.desktop module. Any SM related imports that
> are no longer needed are removed.
>
> This PR is limited to removing doPrivileged() calls and excludes any
> refactoring, reformatting, or other clean up that is out-of-scope for this
> fix.
>
> PS: I have explicitly add comments to the changes where a more watchful
> review is required.
src/java.desktop/unix/classes/sun/awt/PlatformGraphicsInfo.java line 75:
> 73: }
> 74: return headless;
> 75: }
Added break statement since we are no longer using return statement.
src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java line 504:
> 502: = result
> 503: = (display != null &&
> !display.trim().isEmpty());
> 504: }
Note: isOnWayland() - is a three way assignment (i.e `a = b = c`)
`isOnWayland = result = (display != null && !display.trim().isEmpty())`
src/java.desktop/unix/classes/sun/awt/X11/XClipboard.java line 134:
> 132: if (pollInterval <= 0) {
> 133: pollInterval =
> Integer.getInteger("awt.datatransfer.clipboard.poll.interval"
> 134: , defaultPollInterval);
Integer.getInteger() used here since the return value expected is of type
integer.
src/java.desktop/unix/classes/sun/awt/X11/XRobotPeer.java line 50:
> 48:
> 49: tryGtk = Boolean.parseBoolean(
> 50: System.getProperty("awt.robot.gtk", "true")
tryGtk - Boolean.parseBoolean(System.getProperty()) used instead of
Boolean.getBooean() since "awt.robot.gtk" has a default value of true.
src/java.desktop/unix/classes/sun/awt/X11/XToolkit.java line 444:
> 442: thread.setDaemon(true);
> 443: return thread;
> 444: });
Needs careful review. With the updated code I'm directly assigning toolkit
thread to new Thread as below
toolkitThread = new Thread(ThreadGroupUtils.getRootThreadGroup(), this, name,
0, false);
src/java.desktop/unix/classes/sun/awt/X11GraphicsEnvironment.java line 326:
> 324:
> 325: @SuppressWarnings("removal")
> 326: Boolean result = java.security.AccessController.doPrivileged(
Boolean var `result` not required hence removed and replaced with simple return
true or false with updated code changes (as below).
src/java.desktop/unix/classes/sun/awt/screencast/ScreencastHelper.java line 70:
> 68:
> 69: static {
> 70: SCREENCAST_DEBUG =
> Boolean.getBoolean("awt.robot.screenshotDebug");
Boolean.getBoolean() is used here since default value is false for
awt.robot.screenshotDebug.
src/java.desktop/unix/classes/sun/print/PrintServiceLookupProvider.java line
914:
> 912: }
> 913: } catch (IOException io) {
> 914: io.printStackTrace();
Review required. In the original code IOException was being thrown here -
AccessController.doPrivileged(
new PrivilegedExceptionAction<ArrayList<String>>() {
public ArrayList<String> run() throws IOException
Now that the doPrevileged calls is removed, Do we catch the IOException and
print stacktrace or propagate it?
If the IOException is propagated then IOException needs to be thrown by method
that in-turn call execCmd().
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847253186
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847250860
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847236327
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847235133
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847243244
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847232152
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847229074
PR Review Comment: https://git.openjdk.org/jdk/pull/22218#discussion_r1847227950