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

[Anton Litvinov]

Hello Pavel,

Could you review a new webrev with code changes reflecting your last 
remark concerning a substitution of File streams for Byte streams in the 
test class. The test was checked using "jtreg". It fails on JDK 7 
without this fix, and passes on JDK 7 with fix. URL of the new webrev is 
the following.

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

Thank you,
Anton

On 29.08.2012 20:54, Anton Litvinov wrote:
> Hello Pavel,
>
> Yes, the test fails on JDK 7 without fix and does not fail on JDK 7 
> with fix. Yes, I will definitely prefer to substitute the code writing 
> data to files for a code writing data to byte streams.
>
> Thank you,
> Anton
>
> On 29.08.2012 19:04, Pavel Porvatov wrote:
>> 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
>>>>
>>