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

[Semyon Sadetsky]

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
>>>
>>>
>>
>

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