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

[Pavel Porvatov]

Hi Anton,

Does the test fail without the fix?

All code look good except one thing: could you please replace 
FileStreams by Byte streams? Regression tests should avoid to change 
file system, and if there is such need tests should restore file system 
in initial state
> 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
>>