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

[Pavel Porvatov]

Hi Anton,

I see you added
  57         } catch (RuntimeException re) {
  58             throw re;

in serializeGUI method. Is it necessary? I think lines 72 and 73 can be 
removed as well...  I didn't noticed that code in the previous webrev, 
unfortunately.

Regards, Pavel
> 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
>>>>>
>>>