On Tue, 17 Jun 2025 18:23:30 GMT, Phil Race <p...@openjdk.org> wrote:
>> This is the implementation of JEP 504 - Remove the Applet API.
>> API changes are
>> - Remove the entire java.applet package
>> - Remove the javax/swing/JApplet class
>> - Remove applet related APIs in java.beans
>> - Update javadoc referring to applets, including one gif image - now changed
>> to an svg image
>> -
>> Other changes are
>> - Remove references to the removed classes
>> - Remove obsolete tests
>> - Update obsolete code comments
>>
>> sun.awt.AppContext is even more obsolete now than it was before, but
>> eliminating uses of that will be is not required,
>> and will be follow-on internal clean up, at a later date, under unrelated
>> bug ids, and likely not completed in the same
>> release as this JEP is integrated.
>>
>> I have extensively tested this - running all the automated tests used by CI
>> tiers 1 to 8.
>
> Phil Race has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 8359053
Changes requested by aivanov (Reviewer).
src/java.desktop/share/classes/com/sun/media/sound/JavaSoundAudioClip.java line
123:
> 121: try {
> 122: return SoundClip.createSoundClip(tmpFile);
> 123: } catch (IOException e) {
Suggestion:
} catch (IOException ignored) {
To explicitly document that we ignore the exception, and to avoid warnings in
IDE.
src/java.desktop/share/classes/java/awt/doc-files/Modality.html line 352:
> 350: <code>Dialog(owner, true)</code>, etc. Prior to JDK 6
> 351: the default type was toolkit-modal,
> 352: and now with single application per-VM there is no
Suggestion:
and now with single application per VM there is no
src/java.desktop/share/classes/java/awt/doc-files/Modality.html line 353:
> 351: the default type was toolkit-modal,
> 352: and now with single application per-VM there is no
> 353: distinction between application- and toolkit-modality
Suggestion:
distinction between application- and toolkit-modality.
End the sentence with a full stop?
test/jdk/javax/sound/sampled/Clip/AudioContentHandlers.java line 87:
> 85: generateOOME();
> 86: } finally {
> 87: Files.delete(file.toPath());
Suggestion:
file.delete();
Other parts of the test don't use NIO.
test/jdk/javax/sound/sampled/Clip/AutoCloseTimeCheck.java line 54:
> 52: AudioSystem.write(getStream(format), Type.WAVE, file);
> 53: } catch (final Exception ignored) {
> 54: return; // the test is not applicable
I suggest throwing `jtreg.SkippedException`, such tests aren't handled as
passed.
test/jdk/javax/sound/sampled/Clip/AutoCloseTimeCheck.java line 60:
> 58: testBigDelay(file);
> 59: } finally {
> 60: Files.delete(file.toPath());
Suggestion:
file.delete();
Should work just as good.
test/jdk/javax/sound/sampled/Clip/AutoCloseTimeCheck.java line 115:
> 113:
> 114: private static long count() {
> 115: for (final Thread t : Thread.getAllStackTraces().keySet()) {
Why are stack traces are needed?
Thread[] threads = new Thread[Thread.activeCount()];
Thread.enumerate(threads);
would give the same estimate and use less resources.
[`Thread.enumerate`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Thread.html#enumerate(java.lang.Thread%5B%5D))
fills the provided array with currently active threads, and
[`Thread.activeCount`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Thread.html#activeCount())
provides an estimate on the number of live threads.
test/jdk/javax/sound/sampled/Clip/DataPusherThreadCheck.java line 25:
> 23:
> 24: import static javax.sound.sampled.AudioFormat.Encoding.PCM_SIGNED;
> 25: import static javax.sound.sampled.AudioSystem.NOT_SPECIFIED;
Static imports usually follow the regular imports, and both
`test/jdk/javax/sound/sampled/Clip/AudioContentHandlers.java` and
`test/jdk/javax/sound/sampled/Clip/AutoCloseTimeCheck.java` follow this
convention.
And the two other tests import `java.*` packages first and then `javax.*`
packages, whereas this test reverses the (common) order of imports.
test/jdk/javax/sound/sampled/Clip/DataPusherThreadCheck.java line 63:
> 61: AudioSystem.write(audioStream, AudioFileFormat.Type.WAVE,
> file);
> 62: } catch (Exception ignored) {
> 63: return; // the test is not applicable
Throw `jtreg.SkippedException`? The test didn't run.
test/jdk/javax/sound/sampled/Clip/DataPusherThreadCheck.java line 68:
> 66: checkThread(file);
> 67: } finally {
> 68: Files.delete(file.toPath());
Suggestion:
file.delete();
test/jdk/javax/sound/sampled/Clip/DataPusherThreadCheck.java line 87:
> 85:
> 86: private static boolean isDataPushedThreadExist() {
> 87: for (Thread t : Thread.getAllStackTraces().keySet()) {
Can
[Thread.enumerate](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Thread.html#enumerate(java.lang.Thread%5B%5D))
be used instead?
-------------
PR Review: https://git.openjdk.org/jdk/pull/25698#pullrequestreview-2937108877
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153180466
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153183860
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153183390
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153193516
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153196462
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153198451
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153218422
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153222545
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153223792
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153223055
PR Review Comment: https://git.openjdk.org/jdk/pull/25698#discussion_r2153226754
