RFR: 8344059: Remove doPrivileged calls from windows platform sources in the java.desktop module [v7]

Alexey Ivanov aivanov at openjdk.org
Mon Nov 18 16:38:53 UTC 2024


On Sun, 17 Nov 2024 06:50:30 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Since JEP 486 : Permanently Disable the Security Manager
>> [https://bugs.openjdk.org/browse/JDK-8338625] is now integrated, calls to java.security.AccessController.doPrivileged are obsolete and can be removed. 
>> 
>> This PR takes care of the windows-platform files in the java.desktop module to have them removed.
>
> Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix to getBoolean
>  - FIx Boolean.getBoolean

Changes requested by aivanov (Reviewer).

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java line 185:

> 183:         // to be switched off either at runtime or programmatically
> 184:         //
> 185:         String systemFonts = System.getProperty("swing.useSystemFontSettings");

I guess this line and the following line could be combined into one simple line:


useSystemFontSettings = Boolean.getBoolean("swing.useSystemFontSettings");

src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/XPStyle.java line 134:

> 132:             }
> 133:             if (themeActive.booleanValue()) {
> 134:                 String propertyAction = System.getProperty("swing.noxp");

The variable name is now confusing… it's not an action anymore. Alternative, `getProperty` can be inlined in the `if` statement.

src/java.desktop/windows/classes/sun/awt/Win32FontManager.java line 84:

> 82:          * printing the printer driver can access the fonts.
> 83:          */
> 84:         registerJREFontsWithPlatform(jreFontDirName);

Are there any JRE fonts? Shall this line be removed too?

src/java.desktop/windows/classes/sun/awt/Win32FontManager.java line 203:

> 201:         String[] info = new String[2];
> 202:         info[0] = "Arial";
> 203:         info[1] = "c:\\windows\\fonts";

What if Windows is installed not on disk `C:` or not into `Windows` directory?

Yes, it's out of scope of this particular issue, yet it doesn't feel right, even though it's very uncommon these days.

src/java.desktop/windows/classes/sun/awt/Win32FontManager.java line 211:

> 209:                 File file = new File(path);
> 210:                 if (file.exists()) {
> 211:                     dir = dirs[i];

This can be simplified to

info[1] = dirs[i];

and `dir` variable can be removed.

src/java.desktop/windows/classes/sun/awt/windows/WFramePeer.java line 81:

> 79:     private native void clearMaximizedBounds();
> 80: 
> 81:     private static final boolean keepOnMinimize = "true".equals(

Can be replaced with `Boolean.getBoolean("sun.awt.keepWorkingSetOnMinimize");`.

It would be consistent with other updates, too.

src/java.desktop/windows/classes/sun/awt/windows/WPathGraphics.java line 102:

> 100: 
> 101:         if (textLayoutStr != null) {
> 102:             useGDITextLayout = Boolean.getBoolean(textLayoutStr);

This line seems wrong: `textLayoutStr` is a value of the property, and we now read a new property with the value of the `sun.java2d.print.enableGDITextLayout` property.

Is it what's intended? The following line—`textLayoutStr.equalsIgnoreCase("prefer")`—assumes it's not the intended usage.

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

PR Review: https://git.openjdk.org/jdk/pull/22083#pullrequestreview-2443023439
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846832714
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846839129
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846846898
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846849270
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846861507
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846883030
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846889627


More information about the client-libs-dev mailing list