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

[Anton Litvinov]

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