On Mon, 21 Mar 2022 16:52:26 GMT, Alexandre Iline <shurail...@openjdk.org>
wrote:
>> lawrence.andrews has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fixed review comments
>
> test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
> line 74:
>
>> 72: private static volatile AtomicInteger currentJSliderValue;
>> 73: private static volatile AtomicInteger jSliderInitialValue;
>> 74: private static volatile AccessibleAction accessibleAction;
>
> This field seem to be only accessed from the EDT, similar to UI components
> above, so why is it volatile and the components are not?
Fixed
> test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
> line 179:
>
>> 177: validDecrementCountDownLatch = new CountDownLatch(1);
>> 178: validIncrementCountDownLatch = new CountDownLatch(1);
>> 179: currentJSliderValue = new AtomicInteger();
>
> if this is a volatile, than perhaps you do not need an atomic?
Fixed
> test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
> line 265:
>
>> 263: robot.waitForIdle();
>> 264: final Point[] point = new Point[1];
>> 265: final Rectangle[] rect = new Rectangle[1];
>
> finals here are redundant although do not hurt
Fixed
> test/jdk/javax/accessibility/JSlider/AccessibleAction/JSliderAccessibleAction.java
> line 282:
>
>> 280: public static void main(String[] args) throws InterruptedException,
>> 281: InvocationTargetException, AWTException {
>> 282: JSliderAccessibleAction jSliderAccessibleAction =
>
> The class appears to have no state, why do you need an instance of it? Why
> not to make the methods static?
Fixed make the method to be static
-------------
PR: https://git.openjdk.java.net/jdk/pull/7734
