<i18n dev> RFR: 8344061: Remove doPrivileged calls from shared implementation code in the java.desktop module : part 2 [v3]

Phil Race prr at openjdk.org
Mon Nov 18 22:41:20 UTC 2024


On Mon, 18 Nov 2024 17:37:50 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Phil Race has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8344061
>
> src/java.desktop/share/classes/sun/awt/AppContext.java line 521:
> 
>> 519:                 // Create a thread that belongs to the thread group associated
>> 520:                 // with the AppContext and invokes EventQueue.postEvent.
>> 521:                 Thread thread = new Thread(r);
> 
> Previously, the thread was created in the app context thread group and the properties of the created thread were modified, such as its priority and daemon flag.
> 
> https://github.com/openjdk/jdk/blob/d76b5b888e15b507631068f508e261cab75c841e/src/java.desktop/share/classes/sun/awt/AppContext.java#L559-L563

I don't know what I was thinking here.

> src/java.desktop/share/classes/sun/awt/FontConfiguration.java line 163:
> 
>> 161:         } catch (Exception e) {
>> 162:         }
>> 163:         return exists.booleanValue();
> 
> Suggestion:
> 
>         try {
>             File f = new File(fileName);
>             return f.exists();
>         } catch (Exception e) {
>             return false;
>         }
> 
> In this case, no variable is need.
> 
> If you prefer to have the variable, it can now be of primitive type `boolean`.
> 
> Can an exception be thrown at all? Neither `new File` nor `f.exists()` throw a checked exception.

f.exists() could throw SecurityException - unchecked - so that is why it WAS there.
I guess it is legitimate to remove that now.

> src/java.desktop/share/classes/sun/font/CreatedFontTracker.java line 1:
> 
>> 1: /*
> 
> The following imports are unused now:
> 
> import java.security.AccessController;
> import java.security.PrivilegedAction;

ok

> src/java.desktop/share/classes/sun/font/FileFont.java line 38:
> 
>> 36: import sun.java2d.DisposerRecord;
>> 37: 
>> 38: import java.io.IOException;
> 
> `java.io.IOException` is unused now.

ok

> src/java.desktop/share/classes/sun/font/SunFontManager.java line 1705:
> 
>> 1703:                 f = new File(pathDirs[p] + File.separator + s);
>> 1704:                 if (f.exists()) {
>> 1705:                     path = f.getAbsolutePath();
> 
> This can now be
> 
> return f.getAbsolutePath();
> 
> Then `path` variable can be removed.

I've been avoiding refactoring wihout an SM relevant reason. I will make this update but generally don't want to do this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22161#discussion_r1847358729
PR Review Comment: https://git.openjdk.org/jdk/pull/22161#discussion_r1847363922
PR Review Comment: https://git.openjdk.org/jdk/pull/22161#discussion_r1847365402
PR Review Comment: https://git.openjdk.org/jdk/pull/22161#discussion_r1847365763
PR Review Comment: https://git.openjdk.org/jdk/pull/22161#discussion_r1847368489


More information about the i18n-dev mailing list