On Thu, 21 Mar 2024 17:17:10 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Damon Nguyen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments
>
> test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 79:
>
>> 77: and Mission Control behavior.
>> 78:
>> 79: Close the windows.
>
> Won't it fail the test?
This was a part of the original instructions. I modified it to clarify that we
need to close the Parent and Child windows, not the PassFailJFrame as well.
Thanks
> test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 84:
>
>> 82: """;
>> 83:
>> 84: private static final int W = 200, H = 200;
>
> Suggestion:
>
> private static final int W = 200;
> private static final int H = 200;
>
> Java Coding Guidelines do not recommend having several declarations on the
> same line.
Fixed. Thanks
> test/jdk/java/awt/image/multiresolution/MultiDisplayTest.java line 141:
>
>> 139: public ParentFrame() {
>> 140: EventQueue.invokeLater(this::CreateUI);
>> 141: }
>
> This looks weird…
>
> As far as I can see, the frame is created when Start button is clicked, which
> implies this operation is running on EDT. There's no reason to use
> `invokeLater`, all the code from `CreateUI` method could be placed into the
> constructor.
Agreed. I modified it as you suggested. Likewise for the child frame.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534351092
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534351797
PR Review Comment: https://git.openjdk.org/jdk/pull/18354#discussion_r1534351579
