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

[Alexander Scherbatiy]

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