<Swing Dev> Review request for 8013566: Failure of GroupLayout in combination of addPreferredGap and addGroup's

[Sergey Bylokhov]

Thanks. The fix looks good.

13.03.15 1:17, Semyon Sadetsky wrote:
> I've found it. Thanks!
> $ ~/jtreg/win32/bin/jtreg 
> -jdk:../build/windows-x86_64-normal-server-fastdebug/jdk/ 
> test/closed/javax/swing/GroupLayout/UnitTest
> Test results: passed: 1
> --Semyon
>
> On 3/12/2015 9:35 AM, Semyon Sadetsky wrote:
>> Hi Sergey,
>>
>> I couldn't find this 6494656 in jdk and jck test repositories.
>> How can I find it?
>>
>> Thanks,
>> --Semyon
>>
>> On 3/11/2015 8:05 PM, Sergey Bylokhov wrote:
>>> Hi, Semyon.
>>> Please confirm that the test for 6494656 completed successfully also.
>>>
>>> 11.03.15 4:01, Semyon Sadetsky wrote:
>>>> JCK run is OK:
>>>>
>>>> Mar 11, 2015 1:58:55 PM Finished executing all tests, wait for 
>>>> cleanup...
>>>> Mar 11, 2015 1:58:55 PM Harness done with cleanup from test run.
>>>> Total time = 33s
>>>> Setup time = 0s
>>>> Cleanup time = 0s
>>>> Test results: passed: 8
>>>>
>>>>> Hi Alexander,
>>>>>
>>>>> your remarks are taken into account : 
>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8013566/webrev.01/ 
>>>>>
>>>>>
>>>>> jtreg result:
>>>>>
>>>>>
>>>>>         Execution successful
>>>>>
>>>>>   * javax/swing/GroupLayout/6613904/bug6613904.java
>>>>>     <../../JTwork/javax/swing/GroupLayout/6613904/bug6613904.jtr>:
>>>>>     javax.swing.GroupLayout.createParallelGroup(..) doesn't throw
>>>>>     IllegalArgumentException for null arg
>>>>>   * javax/swing/GroupLayout/7071166/bug7071166.java
>>>>>     <../../JTwork/javax/swing/GroupLayout/7071166/bug7071166.jtr>:
>>>>>     LayoutStyle.getPreferredGap() - IAE is expected but not thrown
>>>>>   * javax/swing/GroupLayout/8013566/bug8013566.java
>>>>>     <../../JTwork/javax/swing/GroupLayout/8013566/bug8013566.jtr>:
>>>>>     Failure of GroupLayout in combination of addPreferredGap and
>>>>>     addGroup's last row
>>>>>
>>>>> --Semyon
>>>>>
>>>>>
>>>>> On 3/10/2015 3:46 PM, Alexander Scherbatiy wrote:
>>>>>> On 3/6/2015 5:42 PM, Semyon Sadetsky wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> please review a fix for JDK9.
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8013566
>>>>>>> webrev: 
>>>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8013566/webrev.00/ 
>>>>>>>
>>>>>>
>>>>>>      Just minor comments:
>>>>>>    - 'newLeadingPadding.size() == 0' can be changed to 
>>>>>> newLeadingPadding.isEmpty()
>>>>>>    - //frame.dispose() - may be it should not be commented?
>>>>>>
>>>>>>   Could you run regression and JCK GroupLayout tests to check 
>>>>>> that there are no known regressions.
>>>>>>
>>>>>>   Thanks,
>>>>>>   Alexandr.
>>>>>>
>>>>>>> User got here an infinite loop inside GroupLayout's 
>>>>>>> auto-gap-insertion mechanism that was triggered by 
>>>>>>> getPreferredSize() call upon the JFrame.pack().
>>>>>>> The GroupLayout has not trivial algorithm and its code is really 
>>>>>>> nontransparent. The aim of its auto-preferred-gap-insertion 
>>>>>>> algorithm is inserting gaps between the container components. 
>>>>>>> This allows user to not specify gaps manually for convenience.
>>>>>>> In the considered scenario user relays on the 
>>>>>>> auto-preferred-gap-insertion except for one place where he 
>>>>>>> specifies the preferred gap manually. Moreover this manual 
>>>>>>> preferred gap is inserted not at the leading position but at the 
>>>>>>> trail. That is specifically the case when the algorithm fails.
>>>>>>>
>>>>>>> The thing is that the auto-preferred-gap-insertion routine 
>>>>>>> insertAutopadding() cannot distinguish its auto inserted 
>>>>>>> preferred gaps from manual preferred gaps because it uses the 
>>>>>>> "instance of" check but all those gaps are instances of the same 
>>>>>>> AutoPreferredGapSpring class. Also the insertAutopadding() does 
>>>>>>> not insert gaps at trailing positions but only at leading.  More 
>>>>>>> precisely the trailing AutoPreferredGapSpring object left from 
>>>>>>> the child level group (which insertAutopadding() is called 
>>>>>>> recursively) is ignored on the parent group level and parent's 
>>>>>>> insertAutopadding() inserts a new leading AutoPreferredGapSpring 
>>>>>>> object connected to the last component of the child level group, 
>>>>>>> but this in its turn makes a spring counter value (which serves 
>>>>>>> as a loop exit flag) incorrect. As result preferred gaps are 
>>>>>>> added infinitely on this position.
>>>>>>> The fix introduces a check that tests if the 
>>>>>>> AutoPreferredGapSpring related the current leading component is 
>>>>>>> already created as a trailing gap on the child group level, and 
>>>>>>> if it is this trailing AutoPreferredGapSpring is used to setup 
>>>>>>> the source and new AutoPreferredGapSpring object is not created.
>>>>>>>
>>>>>>> --Semyon
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> -- 
>>> Best regards, Sergey.
>>
>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20150316/9c3d4a8f/attachment.html>