<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