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

[Alexander Scherbatiy]

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