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

Reply via email to