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