<AWT Dev> Review reuest for 8146168: [TEST_BUG] instability of java/awt/Frame/MaximizedToMaximized/MaximizedToMaximized.java

[Semyon Sadetsky]

+1

--Semyon

On 12/30/2015 6:12 PM, Rajeev Chamyal wrote:
> Looks fine to me.
>
> Regards,
> Rajeev Chamyal
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: 30 December 2015 19:22
> To: Ambarish Rapte; Rajeev Chamyal; Semyon Sadetsky; Prasanta Sadhukhan; awt-dev at openjdk.java.net
> Subject: Re: <AWT Dev> Review reuest for 8146168: [TEST_BUG] instability of java/awt/Frame/MaximizedToMaximized/MaximizedToMaximized.java
>
> Looks fine.
>
> On 30/12/15 13:12, Ambarish Rapte wrote:
>> Hi Sergey,
>> 	I missed that the Robot synchronizes toolkit thread as well.
>> 	So, Just a call to Robot.waitForIdle() will synchronize the flow and make an easy fix.
>>
>> 	Please check the updated patch,
>> 	http://cr.openjdk.java.net/~arapte/8146168/webrev.02/
>>
>>
>> Thanks,
>> Ambarish
>> 	
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Tuesday, December 29, 2015 8:04 PM
>> To: Ambarish Rapte; Rajeev Chamyal; Semyon Sadetsky; Prasanta
>> Sadhukhan; awt-dev at openjdk.java.net
>> Subject: Re: <AWT Dev> Review reuest for 8146168: [TEST_BUG]
>> instability of
>> java/awt/Frame/MaximizedToMaximized/MaximizedToMaximized.java
>>
>> Hi, Ambarish.
>> Did you try to check why Robot.waitForIdle() does not work in this
>> case?(in jdk9 it is implemented via (SunToolkit.realSync())
>>
>> On 29/12/15 15:49, Ambarish Rapte wrote:
>>> Hi Rajeev,
>>>
>>> Thanks for the review,
>>>
>>> Please check the updated test file as per the review comments,
>>>
>>> http://cr.openjdk.java.net/~arapte/8146168/webrev.01/
>>>
>>> Regards,
>>>
>>> Ambarish
>>>
>>> *From:*Rajeev Chamyal
>>> *Sent:* Tuesday, December 29, 2015 4:36 PM
>>> *To:* Ambarish Rapte; Semyon Sadetsky; Prasanta Sadhukhan;
>>> awt-dev at openjdk.java.net
>>> *Subject:* RE: Review reuest for 8146168: [TEST_BUG] instability of
>>> java/awt/Frame/MaximizedToMaximized/MaximizedToMaximized.java
>>>
>>> Hello Ambarish,
>>>
>>> 1)Please mark frameDisplayed & frameMaximized as volatile as these
>>> are accessed from different threads.
>>>
>>> 2)Instead of disposing frame multiple times you can add a finally block.
>>>
>>> 3)I think you can also remove the @author tag from test.
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>> *From:* Ambarish Rapte
>>> *Sent:* 29 December 2015 10:58
>>> *To:* Semyon Sadetsky; Prasanta Sadhukhan; Rajeev Chamyal;
>>> awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>>> *Subject:* Review reuest for 8146168: [TEST_BUG] instability of
>>> java/awt/Frame/MaximizedToMaximized/MaximizedToMaximized.java
>>>
>>> Hi All,
>>>
>>> Please review the fix for JDK9,
>>>
>>> Webrev: http://cr.openjdk.java.net/~arapte/8146168/webrev.00/
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146168
>>>
>>> Issue:
>>>
>>>                    The test fails, on low performance system.
>>>
>>> Cause:
>>>
>>>                    The APIs, Frame.setVisible() &
>>> Frame.setExtendedState(Frame.MAXIMIZED_BOTH)
>>>
>>>                    take little more time to finish. Due to which the
>>> result verification occurs before above
>>>
>>>                    mentioned APIs complete.
>>>
>>>                    These APIs run on toolkit thread, But as the
>>> toolkit APIs should not be used by bug:
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8063102
>>>
>>> The original test was altered to remove ((SunToolkit)
>>> toolkit).realSync();
>>>
>>> Fix:
>>>
>>>                    There are two fixes,
>>>
>>> 1.Synchronize the calls
>>>
>>> This is the fix in webrev.00 provided at above link.
>>>
>>> Added ComponentAdapter  to synchronize Frame.setVisible()
>>>
>>> Added WindowStateListener to synchronize
>>> Frame.setExtendedState(Frame.MAXIMIZED_BOTH)
>>>
>>> 2.Add delay after call to above APIs
>>>
>>> Adding robot.delay(1000) would also solve the issue.
>>>
>>> If above fix 1: does not sound good, delay would do the same with
>>> lesser code change.
>>>
>>> Merry Christmas,
>>>
>>> Ambarish
>>>
>>
>> --
>> Best regards, Sergey.
>>
>
> --
> Best regards, Sergey.