On Mon, 18 Nov 2024 17:37:50 GMT, Alexey Ivanov <aiva...@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
