Re: RFR: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
On Fri, 20 Aug 2021 09:41:15 GMT, Сергей Цыпанов
wrote:
> This is a continuation of
>
> - https://bugs.openjdk.java.net/browse/JDK-6736490
> - https://bugs.openjdk.java.net/browse/JDK-8035284
> - https://bugs.openjdk.java.net/browse/JDK-8145680
> - https://bugs.openjdk.java.net/browse/JDK-8251548
>
> As mentioned in JDK-6736490:
>
> _An explicit initialization of a volatile class instance variable, such as
> private volatile Object = null; or private volatile int scale = 0; is
> unnecessary since the Java spec automatically initializes objects to null and
> primitive type short, int, long, float and double to 0 and boolean to false.
> Explicit initialization of volatile variable to a value the same as the
> default implicit initialized value results in an unnecessary store and membar
> operation._
src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java line
64:
> 62: Set tags = essentialTags;
> 63: if (tags == null) {
> 64: essentialTags = Set.of(
What is the purpose of the local tags here?
-
PR: https://git.openjdk.java.net/jdk/pull/5197
Integrated: 8267161 : Write automated test case for JDK-4479161
On Mon, 9 Aug 2021 18:36:07 GMT, lawrence.andrews wrote: > 1) Automated the manual test case. > 2) Removed html dependent file > 3) Removed javax.swing.JApplet dependent. > 4) Test case can be executed independently as well with jtreg framework. > 5) Added methods to know that JFrame and Other component is visible before > starting the user action via Robot. > > @shurymury This pull request has now been integrated. Changeset: d85560ed Author:lawrence.andrews Committer: Alexey Ivanov URL: https://git.openjdk.java.net/jdk/commit/d85560ed0f10a586b659db1c0201373457f1a5a9 Stats: 172 lines in 2 files changed: 88 ins; 50 del; 34 mod 8267161: Write automated test case for JDK-4479161 Reviewed-by: serb, aivanov - PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v6]
On Fri, 20 Aug 2021 15:01:35 GMT, Alexey Ivanov wrote:
>> lawrence.andrews has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fixed review comments
>
> test/jdk/java/awt/im/4959409/bug4959409.java line 87:
>
>> 85: keyPressedEventLatch.countDown();
>> 86: jLabel.setText("keyPressed receive for Shift+1");
>> 87: System.out.println("keyPressed receive for
>> Shift+1");
>
> Here, it was correct: “received”.
Over curiously did search and replace . Corrected
-
PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v7]
On Fri, 20 Aug 2021 15:10:41 GMT, lawrence.andrews wrote: >> 1) Automated the manual test case. >> 2) Removed html dependent file >> 3) Removed javax.swing.JApplet dependent. >> 4) Test case can be executed independently as well with jtreg framework. >> 5) Added methods to know that JFrame and Other component is visible before >> starting the user action via Robot. >> >> @shurymury > > lawrence.andrews has updated the pull request incrementally with one > additional commit since the last revision: > > Fixed the right word Marked as reviewed by aivanov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v7]
> 1) Automated the manual test case. > 2) Removed html dependent file > 3) Removed javax.swing.JApplet dependent. > 4) Test case can be executed independently as well with jtreg framework. > 5) Added methods to know that JFrame and Other component is visible before > starting the user action via Robot. > > @shurymury lawrence.andrews has updated the pull request incrementally with one additional commit since the last revision: Fixed the right word - Changes: - all: https://git.openjdk.java.net/jdk/pull/5058/files - new: https://git.openjdk.java.net/jdk/pull/5058/files/f85ad4f4..c4b2649f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5058=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5058=05-06 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5058.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5058/head:pull/5058 PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v6]
On Fri, 20 Aug 2021 14:50:51 GMT, lawrence.andrews
wrote:
>> 1) Automated the manual test case.
>> 2) Removed html dependent file
>> 3) Removed javax.swing.JApplet dependent.
>> 4) Test case can be executed independently as well with jtreg framework.
>> 5) Added methods to know that JFrame and Other component is visible before
>> starting the user action via Robot.
>>
>> @shurymury
>
> lawrence.andrews has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fixed review comments
test/jdk/java/awt/im/4959409/bug4959409.java line 87:
> 85: keyPressedEventLatch.countDown();
> 86: jLabel.setText("keyPressed receive for Shift+1");
> 87: System.out.println("keyPressed receive for
> Shift+1");
Here, it was correct: “received”.
-
PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v5]
On Fri, 20 Aug 2021 13:45:23 GMT, Alexey Ivanov wrote:
>> lawrence.andrews has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Added WindowListener to check Frame is opened and simplified the testcase
>
> test/jdk/java/awt/im/4959409/bug4959409.java line 120:
>
>> 118: });
>> 119:
>> 120: clickTextField(robot, points[0].x + rect[0].width / 2,
>> points[0].y + rect[0].height / 2);
>
> I'd probably wrap this line, it doesn't fit even in 120 columns.
done
> test/jdk/java/awt/im/4959409/bug4959409.java line 131:
>
>> 129:
>> 130: if (!keyPressedEventLatch.await(TIMEOUT, TimeUnit.SECONDS)) {
>> 131: throw new RuntimeException("Did not received keyPressed for
>> Shift + 1 , test failed");
>
> The grammar:
> Suggestion:
>
> throw new RuntimeException("Did not receive keyPressed for Shift
> + 1 , test failed");
done
> test/jdk/java/awt/im/4959409/bug4959409.java line 147:
>
>> 145:
>> 146: public static void main(String[] args) throws InterruptedException,
>> InvocationTargetException,
>> 147: AWTException {
>
> Suggestion:
>
> public static void main(String[] args) throws Exception {
>
> Shorter.
done
-
PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v6]
> 1) Automated the manual test case. > 2) Removed html dependent file > 3) Removed javax.swing.JApplet dependent. > 4) Test case can be executed independently as well with jtreg framework. > 5) Added methods to know that JFrame and Other component is visible before > starting the user action via Robot. > > @shurymury lawrence.andrews has updated the pull request incrementally with one additional commit since the last revision: Fixed review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/5058/files - new: https://git.openjdk.java.net/jdk/pull/5058/files/b40a1353..f85ad4f4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5058=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5058=04-05 Stats: 13 lines in 1 file changed: 1 ins; 3 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/5058.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5058/head:pull/5058 PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v5]
On Fri, 20 Aug 2021 13:39:58 GMT, Alexey Ivanov wrote:
>> lawrence.andrews has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Added WindowListener to check Frame is opened and simplified the testcase
>
> test/jdk/java/awt/im/4959409/bug4959409.java line 59:
>
>> 57: private static JLabel jLabel;
>> 58:
>> 59: public static void createUIAndTest() throws InterruptedException,
>> InvocationTargetException, AWTException {
>
> `throws Exception` would be enough and shorter:
> Suggestion:
>
> public static void createUIAndTest() throws Exception {
>
>
> It's a test code, any exception means the test fails, so we don't care much
> about which specific exceptions can be thrown.
Okay
> test/jdk/java/awt/im/4959409/bug4959409.java line 80:
>
>> 78:
>> 79: jTextField.addKeyListener(new KeyAdapter() {
>> 80:
>
> I'd remove this blank line for consistency with the anonymous class above.
Done
> test/jdk/java/awt/im/4959409/bug4959409.java line 92:
>
>> 90: } else {
>> 91: jLabel.setText("Did not received keyPressed for
>> Shift+1");
>> 92: System.out.println("Did not received keyPressed
>> for Shift+1");
>
> The grammar:
> Suggestion:
>
> jLabel.setText("Did not receive keyPressed for
> Shift+1");
> System.out.println("Did not receive keyPressed for
> Shift+1");
done
-
PR: https://git.openjdk.java.net/jdk/pull/5058
Re: RFR: 8267161 : Write automated test case for JDK-4479161 [v5]
On Thu, 19 Aug 2021 18:54:48 GMT, lawrence.andrews
wrote:
>> 1) Automated the manual test case.
>> 2) Removed html dependent file
>> 3) Removed javax.swing.JApplet dependent.
>> 4) Test case can be executed independently as well with jtreg framework.
>> 5) Added methods to know that JFrame and Other component is visible before
>> starting the user action via Robot.
>>
>> @shurymury
>
> lawrence.andrews has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Added WindowListener to check Frame is opened and simplified the testcase
Marked as reviewed by aivanov (Reviewer).
test/jdk/java/awt/im/4959409/bug4959409.java line 59:
> 57: private static JLabel jLabel;
> 58:
> 59: public static void createUIAndTest() throws InterruptedException,
> InvocationTargetException, AWTException {
`throws Exception` would be enough and shorter:
Suggestion:
public static void createUIAndTest() throws Exception {
It's a test code, any exception means the test fails, so we don't care much
about which specific exceptions can be thrown.
test/jdk/java/awt/im/4959409/bug4959409.java line 80:
> 78:
> 79: jTextField.addKeyListener(new KeyAdapter() {
> 80:
I'd remove this blank line for consistency with the anonymous class above.
test/jdk/java/awt/im/4959409/bug4959409.java line 92:
> 90: } else {
> 91: jLabel.setText("Did not received keyPressed for
> Shift+1");
> 92: System.out.println("Did not received keyPressed
> for Shift+1");
The grammar:
Suggestion:
jLabel.setText("Did not receive keyPressed for
Shift+1");
System.out.println("Did not receive keyPressed for
Shift+1");
test/jdk/java/awt/im/4959409/bug4959409.java line 120:
> 118: });
> 119:
> 120: clickTextField(robot, points[0].x + rect[0].width / 2,
> points[0].y + rect[0].height / 2);
I'd probably wrap this line, it doesn't fit even in 120 columns.
test/jdk/java/awt/im/4959409/bug4959409.java line 131:
> 129:
> 130: if (!keyPressedEventLatch.await(TIMEOUT, TimeUnit.SECONDS)) {
> 131: throw new RuntimeException("Did not received keyPressed for
> Shift + 1 , test failed");
The grammar:
Suggestion:
throw new RuntimeException("Did not receive keyPressed for Shift +
1 , test failed");
test/jdk/java/awt/im/4959409/bug4959409.java line 147:
> 145:
> 146: public static void main(String[] args) throws InterruptedException,
> InvocationTargetException,
> 147: AWTException {
Suggestion:
public static void main(String[] args) throws Exception {
Shorter.
-
PR: https://git.openjdk.java.net/jdk/pull/5058
RFR: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
This is a continuation of - https://bugs.openjdk.java.net/browse/JDK-6736490 - https://bugs.openjdk.java.net/browse/JDK-8035284 - https://bugs.openjdk.java.net/browse/JDK-8145680 - https://bugs.openjdk.java.net/browse/JDK-8251548 As mentioned in JDK-6736490: _An explicit initialization of a volatile class instance variable, such as private volatile Object = null; or private volatile int scale = 0; is unnecessary since the Java spec automatically initializes objects to null and primitive type short, int, long, float and double to 0 and boolean to false. Explicit initialization of volatile variable to a value the same as the default implicit initialized value results in an unnecessary store and membar operation._ - Commit messages: - 8272756: Remove unnecessary explicit initialization of volatile fields in java.desktop Changes: https://git.openjdk.java.net/jdk/pull/5197/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5197=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8272756 Stats: 50 lines in 25 files changed: 0 ins; 0 del; 50 mod Patch: https://git.openjdk.java.net/jdk/pull/5197.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5197/head:pull/5197 PR: https://git.openjdk.java.net/jdk/pull/5197
