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

Phil Race prr at openjdk.org
Mon Nov 18 20:05:06 UTC 2024


On Mon, 18 Nov 2024 15:45:20 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Fix to getBoolean
>>  - FIx Boolean.getBoolean
>
> 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");

A goal here is to MINIMISE optional refactoring, which is what this suggestion is.

> 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.

A goal here is to MINIMISE optional refactoring, this is borderline. I'll leave it up to Prasanta.

> 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?

That has has nothing to do with removing doPrivileged. Leave it.

> 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.

A goal here is to MINIMISE optional refactoring, which is what this suggestion is.

> 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.

Let's leave it. I regret making the suggestion to use Boolean.getBoolean earlier

> 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.

If there's an existing problem, let's examine it under a separate bug. This change is about equivalence

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847169851
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847171162
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847172487
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847172744
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847174694
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847182852


More information about the client-libs-dev mailing list