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

[Semyon Sadetsky]

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/20150310/5256a2ff/attachment.html>