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

[Alexander Scherbatiy]

   The fix looks good to me.

   Thanks,
   Alexandr.

On 3/11/2015 2:01 PM, 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
>>>>
>>>>
>>>
>>
>