On Thu, 27 Apr 2023 12:13:57 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Tejesh R has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updated based on review comments > > test/jdk/java/awt/ScrollPane/ComponentScrollTest.java line 47: > >> 45: public ScrollPane scrollpane; >> 46: public Frame frame; >> 47: public int count = 0; > > Suggestion: > > public volatile int count = 0; > > `count` is accessed from two threads, it should be `volatile` or you should > use `AtomicInteger` instead. Updated. > test/jdk/java/awt/ScrollPane/ScrollPaneLimitation.java line 53: > >> 51: public static boolean mouseWasPressed = false; >> 52: public static Component child = null; >> 53: private Object lock = new Object(); > > Suggestion: > > private final Object lock = new Object(); Updated. > test/jdk/java/awt/ScrollPane/ScrollPaneLimitation.java line 83: > >> 81: if (e.getID() == MouseEvent.MOUSE_PRESSED) { >> 82: if (e.getSource() == >> ScrollPaneLimitation.child >> 83: && e.getY() > SCROLL_POS) { > > The two `if` blocks could be merged together into one. Updated. > test/jdk/java/awt/ScrollPane/ScrollPaneLimitation.java line 145: > >> 143: throw new RuntimeException("test was interrupted"); >> 144: } >> 145: } > > A `CountDownLatch` would be better rather than plain `lock`. Updated. > test/jdk/java/awt/ScrollPane/ScrollPaneRemoveAdd.java line 138: > >> 136: waiting--; >> 137: } >> 138: } > > This method is never used, it can safely be dropped. Updated. > test/jdk/java/awt/ScrollPane/ScrollPaneWindowsTest.java line 77: > >> 75: hScroll = (ScrollPaneAdjustable) sp.getHAdjustable(); >> 76: vScroll.addAdjustmentListener(this); >> 77: hScroll.addAdjustmentListener(this); > > `vScroll` and `hScroll` should also go into `invokeAndWait` above. Updated. > test/jdk/java/awt/ScrollPane/ScrollPositionIntact.java line 85: > >> 83: >> 84: int i; >> 85: i = (int) (sp.getScrollPosition().getX()); > > Suggestion: > > int i = (int) (sp.getScrollPosition().getX()); Updated. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183230264 PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229539 PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183230178 PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183230201 PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229514 PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229628 PR Review Comment: https://git.openjdk.org/jdk/pull/13621#discussion_r1183229572