<Swing Dev> [7u8] Review request for 7193219: JComboBox serialization fails in JDK 1.7

Anton Litvinov anton.litvinov at oracle.com
Wed Aug 29 12:15:50 UTC 2012


Hello Pavel,

Thank you very much for a positive review of the fix and detailed 
remarks for the test. Could you review a corrected version of the test 
in a new webrev. I would like to clarify that a previous version of the 
test contained some irrelevant components, because I was not sure if I 
could significantly modify the original test case provided with the bug 
itself. URL of a webrev with the test class corrected according to your 
remarks is provided below.

Webrev: http://cr.openjdk.java.net/~alitvinov/7193219/webrev.02

The modified test was checked using "jtreg" tool on JDK with and without 
this fix, and the test behaved properly. The following changes were made:
1. Setting visibility of frames was removed, because it  does not 
influence reproducibility of the test.
2. All components except JPanel and JComboBox were eliminated from the test.
3. Frames' titles were corrected.
4. Method "test" was renamed and moved into EDT execution block.
5. Method "createAndShowGUI" was renamed.
6. Calls to JFrame's "dispose" method were added.

Thank you,
Anton

On 28.08.2012 21:45, Pavel Porvatov wrote:
> Hi Anton,
>
> The fix looks good but the test should be updated:
>
> 1. Is the following line needed for the test:
> 63         frame.setVisible(true);
> ?
> If yes then you should use SunToolkit.realSync to wait until frame 
> become visible.
> The same comment for the line
> 85         frame.setVisible(true);
>
> 2. Are label and layout and layout needed for the test purpose? If no, 
> can you remove unused components?
>
> 3. The following code looks strange (two titles?)
>  45         JFrame frame = new JFrame("HelloWorldSwing");
> ...
>  47         frame.setTitle("why why why");
>
> 4. The test method should be on the EDT
>
> 5. There is no need to use the mainPanel field. Logically it should be 
> local variable
>
> BTW: can you start with the fix for jdk8 and only then backport it to 
> jdk7?
>
> Regards, Pavel
>> Hello,
>>
>> Please review the following fix for a bug.
>>
>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7193219
>> Webrev: http://cr.openjdk.java.net/~alitvinov/7193219/webrev.01
>>
>> For details on this bug please look at "Evaluation" field on a web 
>> page of this bug. The provided webrev contains both a fix and a 
>> corresponding unit-test. Before publishing this webrev all unit-tests 
>> from the "java.awt" and
>> "javax.swing" related to serialization were run and no negative 
>> changes were observed comparing the results of tests' runs on JDK 
>> with and without patch represented by this webrev.
>>
>> This is the second version of the fix. The first version was 
>> submitted to the AWT development group through a review request with 
>> the same subject and after discussion with engineers and additional 
>> investigation of the problem it was decided to apply the fix in Swing 
>> part of JDK.
>>
>> Thank you,
>> Anton
>



More information about the swing-dev mailing list