<AWT Dev> <Awt Dev> [9] Review Request for 8080395: consider making sun.awt.CausedFocusEvent functionality public

[Semyon Sadetsky]

Please review the updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.04/

- CausedFocusEvent is restored to avoid CNFE during deserialization.

- readResolve() is added to FocusEvent to handle the null cause test.

- deserialization compatibility test scenarios added

--Semyon

On 11/6/2015 3:57 PM, Alexander Scherbatiy wrote:
>
> There is a problem with FocusEvent deserialization that the read cause 
> value is not checked to null.
> You can fix it with the current fix or just create a separate issue on 
> it.
>
> Otherwise, the fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 10/29/2015 12:33 AM, Sergey Bylokhov wrote:
>> On 27.10.15 13:01, Semyon Sadetsky wrote:
>>>
>>>
>>> On 10/26/2015 5:31 PM, Sergey Bylokhov wrote:
>>>>>> The test should verify this also.
>>>>>> I assume it should be reverted and updated to:
>>>>>> 252 if (cause == null)
>>>>>> 253 throw new IllegalArgumentException("null cause");
>>>>> Please review the updated webrev
>>>>> http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.02/
>>>>
>>>> is possible to write a test fox this missed check?
>>>> What about a sentence in:
>>>> ********
>>>> 228      * <p> This method throws an
>>>> 229      * {@code IllegalArgumentException} if {@code source}
>>>> 230      * is {@code null}.
>>>> ********
>>>> 250     public FocusEvent(Component source, int id, boolean temporary,
>>>> 251                       Component opposite, Cause cause) {
>>>> ********
>>>> Should we mention cause and IAE as well in the javadoc?
>>> Test case is added. null case is mentioned in javadoc.
>>> http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.03/
>>
>> The fix looks fine. I am not sure about this:
>> * {@code IllegalArgumentException} if {@code source} or {@code cause}
>> * is {@code null}.
>> I suppose it should be "are {@code null}?
>>
>>>>>>
>>>>>> Since you update the serialVersionUID then the comment is not valid
>>>>>> anymore: "JDK 1.1 serialVersionUID".
>>>>>>
>>>>>> Also I have requested an additional clarification from the core libs
>>>>>> team to confirm that we have an ability to change 
>>>>>> serialVersionUID in
>>>>>> the major release.
>>>>> Please decide finally you do need a new serialVersionUID or you don't
>>>>> need it. Generally, adding a field is a compatible change FocusEvent
>>>>> will be able to read the previous format. It is matter of
>>>>> opinion/convention only.
>>>>
>>>> In this case the cause field will contain unspecified null value.
>>>> I guess we will need to file a new ccc request.
>>>>
>>>>>>
>>>>>>>
>>>>>>>> On 11.09.15 0:15, Semyon Sadetsky wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> Please review fix for JDK9:
>>>>>>>>>
>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8080395
>>>>>>>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.00/
>>>>>>>>>
>>>>>>>>> This fix moves the caused property to 
>>>>>>>>> java.awt.event.FocusEvent to
>>>>>>>>> make
>>>>>>>>> it public. The sun.awt.CausedFocusEvent class is removed as
>>>>>>>>> unnecessary.
>>>>>>>>> The API change was approved http://ccc.us.oracle.com/8080395.
>>>>>>>>>
>>>>>>>>> --Semyon
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>