On Mon, 18 Sep 2023 10:45:11 GMT, Alexey Ivanov <[email protected]> wrote:
>> Harshitha Onkar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review changes
>
> test/jdk/java/awt/event/PaintEvent/RepaintTest.java line 44:
>
>> 42: // invoking setBounds, and the other that invokes repaint directly on
>> 43: // the Frame to repaint a Component. The Component displays an integer
>> 44: // that increments everytime paint is invoked on it.
>
> This comment seems confusing because no buttons are created in the test.
Removed comment
> test/jdk/java/awt/event/PaintEvent/RepaintTest.java line 63:
>
>> 61:
>> 62: int count = counter.getCount();
>> 63: robot.delay(300);
>
> Does this delay affect anything? I think it can be safely removed.
Delay removed.
> test/jdk/java/awt/event/PaintEvent/RepaintTest.java line 71:
>
>> 69: if (counter.getCount() == count) {
>> 70: throw new RuntimeException("Failed");
>> 71: }
>
> This is not thread-safe. The `paintCount` in `IncrementComponent` needs to be
> volatile. Better yet, use `AtomicInteger` and its
> [`incrementAndGet`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/atomic/AtomicInteger.html#incrementAndGet())
> or
> [`getAndIncrement`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/atomic/AtomicInteger.html#getAndIncrement())
> as well as
> [`get`](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/atomic/AtomicInteger.html#get()),
> it will make the test code thread-safe.
Updated
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15751#discussion_r1329316851
PR Review Comment: https://git.openjdk.org/jdk/pull/15751#discussion_r1329316824
PR Review Comment: https://git.openjdk.org/jdk/pull/15751#discussion_r1329316788
