<Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

<Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev-3
8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

-------------

Commit messages:
 - 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Changes: https://git.openjdk.java.net/jdk/pull/2790/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2790&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261689
  Stats: 21 lines in 1 file changed: 19 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2790/head:pull/2790

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev-3
On Tue, 2 Mar 2021 09:15:22 GMT, Alexander Zuev <[hidden email]> wrote:

> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

I finally was able to get the failure on the test system and it seems like that was a result of the incomplete drawing of the frame at the time of screenshot taking. Here's the screenshot in full resolution - note the black part at the right part of the frame.
![fullScreenInit](https://user-images.githubusercontent.com/69642324/109626290-359d2780-7af5-11eb-8138-c8dee43222a7.png)
Setting min/max size of the frame and the content pane inside it fixes the issue so after couple of hundred of the runs i see no failures. Plus in the future should failure occur we will have better information in order to analyze the failure.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zvegintsev-2
In reply to this post by Alexander Zuev-3
On Tue, 2 Mar 2021 09:15:22 GMT, Alexander Zuev <[hidden email]> wrote:

> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 112:

> 110:             frh = bounds.height - insets.top - insets.bottom;
> 111:
> 112:             imageInit = robot.createScreenCapture(new Rectangle(locx, locy, frw, frh));

Looks like the result of this screen capture is unused.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexey Ivanov-2
In reply to this post by Alexander Zuev-3
On Tue, 2 Mar 2021 09:15:22 GMT, Alexander Zuev <[hidden email]> wrote:

> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Should `frame` be declared as `volatile`? It's accessed on main thread in finally block.
In addition to it, `frame.getBounds()` and `frame.getInsets()` are called on main thread instead of EDT.

test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 116:

> 114:             Graphics g = fullScreen.getGraphics();
> 115:             g.setColor(Color.RED);
> 116:             g.drawRect(locx-1, locy-1, frw+1, frh+1);

Suggestion:

            g.drawRect(locx - 1, locy - 1, frw + 1, frh + 1);

test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 93:

> 91:                     frame.setMinimumSize(new Dimension(350, 350));
> 92:                     frame.setMaximumSize(new Dimension(350, 350));
> 93:                     frame.pack();

Wouldn't it make {{desktop}} larger than 300,300?

test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 133:

> 131:                 ImageIO.write(imageShow, "png", new File("imageShow.png"));
> 132:                 ImageIO.write(fullScreen, "png", new File("fullScreenInit.png"));
> 133:                 throw new Exception("Failed to show opaque button");

<del>I suggest moving saving the images into a new method.</del>

I see the set of images is different each time. Probably it makes sense to save all the images which are non-null, what do you think?

test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 192:

> 190:                 ImageIO.write(fullScreen, "png", new File("fullScreenInit.png"));
> 191:                 throw new Exception("Failed to show non-opaque button");
> 192:             }

Does it make sense to move this block above before `invokeAndWait` to make the sequence of actions consistent?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Sergey Bylokhov-2
In reply to this post by Alexander Zuev-3
On Tue, 2 Mar 2021 09:15:22 GMT, Alexander Zuev <[hidden email]> wrote:

> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 101:

> 99:
> 100:             robot.waitForIdle(1000);
> 101:             robot.delay(1000);

I doubt that this is a test issue, 2 seconds was not enough to properly show the frame?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev-3
In reply to this post by Alexander Zvegintsev-2
On Tue, 2 Mar 2021 12:06:26 GMT, Alexander Zvegintsev <[hidden email]> wrote:

>> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"
>
> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 112:
>
>> 110:             frh = bounds.height - insets.top - insets.bottom;
>> 111:
>> 112:             imageInit = robot.createScreenCapture(new Rectangle(locx, locy, frw, frh));
>
> Looks like the result of this screen capture is unused.

Yes, accidentally copied line instead of moving it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev-3
In reply to this post by Alexey Ivanov-2
On Tue, 2 Mar 2021 13:26:51 GMT, Alexey Ivanov <[hidden email]> wrote:

>> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"
>
> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 116:
>
>> 114:             Graphics g = fullScreen.getGraphics();
>> 115:             g.setColor(Color.RED);
>> 116:             g.drawRect(locx-1, locy-1, frw+1, frh+1);
>
> Suggestion:
>
>             g.drawRect(locx - 1, locy - 1, frw + 1, frh + 1);

Fixed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev-3
In reply to this post by Alexey Ivanov-2
On Tue, 2 Mar 2021 13:28:44 GMT, Alexey Ivanov <[hidden email]> wrote:

>> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"
>
> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 93:
>
>> 91:                     frame.setMinimumSize(new Dimension(350, 350));
>> 92:                     frame.setMaximumSize(new Dimension(350, 350));
>> 93:                     frame.pack();
>
> Wouldn't it make {{desktop}} larger than 300,300?

It will but it forces the invalidation and repainting of the content so test does not fail.

> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 133:
>
>> 131:                 ImageIO.write(imageShow, "png", new File("imageShow.png"));
>> 132:                 ImageIO.write(fullScreen, "png", new File("fullScreenInit.png"));
>> 133:                 throw new Exception("Failed to show opaque button");
>
> <del>I suggest moving saving the images into a new method.</del>
>
> I see the set of images is different each time. Probably it makes sense to save all the images which are non-null, what do you think?

I don't think it is necessary. And since this is an existing test i'm trying not to change its overall behavior and having screenshots at other stages of the testing is not beneficial in the test debugging.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev-3
In reply to this post by Alexey Ivanov-2
On Tue, 2 Mar 2021 13:45:07 GMT, Alexey Ivanov <[hidden email]> wrote:

>> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"
>
> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 192:
>
>> 190:                 ImageIO.write(fullScreen, "png", new File("fullScreenInit.png"));
>> 191:                 throw new Exception("Failed to show non-opaque button");
>> 192:             }
>
> Does it make sense to move this block above before `invokeAndWait` to make the sequence of actions consistent?

Well, functionally it will not make any difference, we are analyzing the screenshots and the only thing that matters is at which time screenshot was taken. The only reason we don't take all screenshots in the beginning and analyze them later is to save test run time if test runs early. At the end it is not critical anymore. And as i said this is an existing test so i'm trying not to change it unless it affects its functionality.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v2]

Alexander Zuev-3
In reply to this post by Alexander Zuev-3
> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:

  Minor fixes.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2790/files
  - new: https://git.openjdk.java.net/jdk/pull/2790/files/c0a2cd4e..2c6594c6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2790&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2790&range=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2790/head:pull/2790

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v2]

Alexander Zuev-3
In reply to this post by Sergey Bylokhov-2
On Tue, 2 Mar 2021 18:14:12 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Minor fixes.
>
> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 101:
>
>> 99:
>> 100:             robot.waitForIdle(1000);
>> 101:             robot.delay(1000);
>
> I doubt that this is a test issue, 2 seconds was not enough to properly show the frame?

The problem is that i tried to reproduce this behavior locally on Ubuntu running this test (and the simplified version of it) literally millions of times. The only time it fails is when it is run on mach5 and even then it fails very rarely - once per hundreds of runs. The changes here - especially forcing the resize - seems to eliminate the problem with mach5.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v2]

Alexey Ivanov-2
In reply to this post by Alexander Zuev-3
On Tue, 2 Mar 2021 21:59:31 GMT, Alexander Zuev <[hidden email]> wrote:

>> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 192:
>>
>>> 190:                 ImageIO.write(fullScreen, "png", new File("fullScreenInit.png"));
>>> 191:                 throw new Exception("Failed to show non-opaque button");
>>> 192:             }
>>
>> Does it make sense to move this block above before `invokeAndWait` to make the sequence of actions consistent?
>
> Well, functionally it will not make any difference, we are analyzing the screenshots and the only thing that matters is at which time screenshot was taken. The only reason we don't take all screenshots in the beginning and analyze them later is to save test run time if test runs early. At the end it is not critical anymore. And as i said this is an existing test so i'm trying not to change it unless it affects its functionality.

Yes, I agree it makes no difference. Yet when you read the test code, the pattern — show or hide the button, take screenshot, analyze it — gets broken. This place is the only exception to the pattern; here the button is shown, screenshot is taken, then the button is hidden and only after hiding the button the screenshot is analyzed, which could give the wrong impression as if the screenshot after hiding the button is being analyzed here.

> It will but it forces the invalidation and repainting of the content so test does not fail.

Got it! It's the intention to cause additional repaint.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v2]

Alexey Ivanov-2
In reply to this post by Alexey Ivanov-2
On Tue, 2 Mar 2021 13:54:27 GMT, Alexey Ivanov <[hidden email]> wrote:

> Should `frame` be declared as `volatile`? It's accessed on main thread in finally block.
> In addition to it, `frame.getBounds()` and `frame.getInsets()` are called on main thread instead of EDT.

I couldn't link this comment to the code because GitHub does not allow adding comments to unmodified lines. Do I understand correctly that you're for leaving it as is?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v2]

Alexander Zuev-3
On Tue, 2 Mar 2021 23:21:49 GMT, Alexey Ivanov <[hidden email]> wrote:

> > Should `frame` be declared as `volatile`? It's accessed on main thread in finally block.
> > In addition to it, `frame.getBounds()` and `frame.getInsets()` are called on main thread instead of EDT.
>
> I couldn't link this comment to the code because GitHub does not allow adding comments to unmodified lines. Do I understand correctly that you're for leaving it as is?

Pretty much. Making frame volatile will not change the behavior, the assignment to it made in one place and it's on EDT inside invokeAndWait block, making it volatile will change nothing, it will be fully assigned by the time we leave EDT. And calling non-disruptive getters such getInsets or getBounds from main thread is not a problem either. By that time frame should be visible and in position - otherwise we will have much more interesting problems.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v2]

Alexander Zuev-3
In reply to this post by Alexey Ivanov-2
On Tue, 2 Mar 2021 23:17:26 GMT, Alexey Ivanov <[hidden email]> wrote:

>> Well, functionally it will not make any difference, we are analyzing the screenshots and the only thing that matters is at which time screenshot was taken. The only reason we don't take all screenshots in the beginning and analyze them later is to save test run time if test runs early. At the end it is not critical anymore. And as i said this is an existing test so i'm trying not to change it unless it affects its functionality.
>
> Yes, I agree it makes no difference. Yet when you read the test code, the pattern — show or hide the button, take screenshot, analyze it — gets broken. This place is the only exception to the pattern; here the button is shown, screenshot is taken, then the button is hidden and only after hiding the button the screenshot is analyzed, which could give the wrong impression as if the screenshot after hiding the button is being analyzed here.

Ok, i give up, you won - i'll fix it.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v3]

Alexander Zuev-3
In reply to this post by Alexander Zuev-3
> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"

Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:

  Moved checking block up to keep consistency of operations.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2790/files
  - new: https://git.openjdk.java.net/jdk/pull/2790/files/2c6594c6..0bfe02fd

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2790&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2790&range=01-02

  Stats: 14 lines in 1 file changed: 7 ins; 7 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2790/head:pull/2790

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v3]

Sergey Bylokhov-2
In reply to this post by Alexander Zuev-3
On Tue, 2 Mar 2021 22:08:48 GMT, Alexander Zuev <[hidden email]> wrote:

>> test/jdk/javax/swing/JComponent/7154030/bug7154030.java line 101:
>>
>>> 99:
>>> 100:             robot.waitForIdle(1000);
>>> 101:             robot.delay(1000);
>>
>> I doubt that this is a test issue, 2 seconds was not enough to properly show the frame?
>
> The problem is that i tried to reproduce this behavior locally on Ubuntu running this test (and the simplified version of it) literally millions of times. The only time it fails is when it is run on mach5 and even then it fails very rarely - once per hundreds of runs. The changes here - especially forcing the resize - seems to eliminate the problem with mach5.

Then it is a product bug, If the problem is unrelated to the performance and the root cause is in the wrong/broken final layout. Do we want to suggest the same thing if the user will get the same issue in their app?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v3]

Alexander Zuev-3
On Wed, 3 Mar 2021 06:31:44 GMT, Sergey Bylokhov <[hidden email]> wrote:

>> The problem is that i tried to reproduce this behavior locally on Ubuntu running this test (and the simplified version of it) literally millions of times. The only time it fails is when it is run on mach5 and even then it fails very rarely - once per hundreds of runs. The changes here - especially forcing the resize - seems to eliminate the problem with mach5.
>
> Then it is a product bug, If the problem is unrelated to the performance and the root cause is in the wrong/broken final layout. Do we want to suggest the same thing if the user will get the same issue in their app?

It might be a product bug. Or it might be something specific to the machines with mach5 system. Or to some specific machine having hardware issue (i noticed that in my case all failures happened on one specific Linux test system). That's the problem. I have a simplified test case that should check for that specific problem and i'm running a dedicated Linux host in my home office with the same OS version than problematic machine that does nothing but runs this test in interactive mode for days. Today only i ran this test over 700000 times - 0 failures. If i would have a way to at least semi-reliable reproduce the problem i would create a new bug and would debug and fix it. But i don't. And the changes i made is simply a way to let this test to run even on the problematic host because the problem that happens have nothing to do with the functionality related to the test itself.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v3]

Alexey Ivanov-2
In reply to this post by Alexander Zuev-3
On Wed, 3 Mar 2021 05:14:11 GMT, Alexander Zuev <[hidden email]> wrote:

>> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"
>
> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>
>   Moved checking block up to keep consistency of operations.

Marked as reviewed by aivanov (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
Reply | Threaded
Open this post in threaded view
|

Re: <Swing Dev> RFR: 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button" [v3]

Alexander Zvegintsev-2
In reply to this post by Alexander Zuev-3
On Wed, 3 Mar 2021 05:14:11 GMT, Alexander Zuev <[hidden email]> wrote:

>> 8261689: javax/swing/JComponent/7154030/bug7154030.java still fails with "Exception: Failed to hide opaque button"
>
> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>
>   Moved checking block up to keep consistency of operations.

Marked as reviewed by azvegint (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2790
12