<AWT Dev> [10] Review request for 8189201: [macosx] NotSerializableException during JFrame with MenuBar serialization

[Phil Race]

Ok but you need a CSR first for the API changes .. and so this
fix cannot be backported.

-phil.

On 11/08/2017 10:35 AM, Semyon Sadetsky wrote:
> On 11/07/2017 12:54 PM, Phil Race wrote:
>> > The fields cannot be changed to transient because this would break 
>> the logic after deserialization of the component.
>>
>> Can you explain what logic would be broken since changing these to 
>> transient looks tempting ..
>
> Some evnts would not reach the accessibility listener.
>
>>
>> It can't be a problem for serialisation of actual non-null values 
>> since demonstrably that
>> can't ever have worked.
> They can be involved if the listener is added.
>>
>> If there is broken logic for when they are null, what is it ?
> AccessibleAWTComponentHandler and AccessibleAWTFocusHandler rethrow 
> events that a necessary for AccessibleAWTComponent listeners. If those 
> objects are absent the listener will not receive the events. Since 
> this is a public API it only means that the contract is not met.
>
> --Semyon
>>
>> -phil.
>>
>> On 11/07/2017 10:42 AM, Semyon Sadetsky wrote:
>>>
>>>
>>> On 11/07/2017 10:21 AM, Sergey Bylokhov wrote:
>>>> On 07/11/2017 08:48, Semyon Sadetsky wrote:
>>>>>> But from the other point of view all our listeners like 
>>>>>> "componentListener", "keyListener" etc are transient and have 
>>>>>> some special steps in Component.writeObject().
>>>>> Yes, it may be necessary to to do something with 
>>>>> propertyListenersCount in writeObject() as well. But this may 
>>>>> change the format compatibility. I have created a separate bug to 
>>>>> investigate this: https://bugs.openjdk.java.net/browse/JDK-8190867
>>>>
>>>> But it does not explain why other containers for listeners marked 
>>>> as transient. I guess the reason is in this part of spec in 
>>>> Component.writeObject():
>>>>      * Writes default serializable fields to stream.  Writes
>>>>      * a variety of serializable listeners as optional data.
>>>>      * The non-serializable listeners are detected and
>>>>      * no attempt is made to serialize them.
>>>>
>>>> This is why all of them marked as transient and during 
>>>> serialization we iterates over them and save only listeners which 
>>>> implements Serializable interface.
>>> I did get this. This spec is unrelated to the current issue. The 
>>> current issue is very simple:
>>>
>>> AccessibleAWTComponent class is Serializable but its fields 
>>> accessibleAWTComponentHandler and  accessibleAWTFocusHandler are not.
>>>
>>> This violates the Java serialization policy and causes exception in 
>>> JDK code. The proposed fix eliminates this issue. The fields cannot 
>>> be changed to transient because this would break the logic after 
>>> deserialization of the component.
>>>>
>>>>>
>>>>> --Semyon
>>>>>>
>>>>>> On 31/10/2017 09:05, Semyon Sadetsky wrote:
>>>>>>> The updated webrev: 
>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8189201/webrev.02/
>>>>>>>
>>>>>>>
>>>>>>> On 10/23/2017 01:54 PM, Semyon Sadetsky wrote:
>>>>>>>> Actually ScreenMenu is fully removed in uninstallUI() (line 54 
>>>>>>>> of AquaMenuBarUI), so there is no need to make its property 
>>>>>>>> listener Serializable, please, ignore the change in 
>>>>>>>> ScreenMenuPropertyListener.
>>>>>>>>
>>>>>>>> --Semyon
>>>>>>>>
>>>>>>>>
>>>>>>>> On 10/23/2017 01:25 PM, Semyon Sadetsky wrote:
>>>>>>>>> On 10/23/2017 12:58 PM, Sergey Bylokhov wrote:
>>>>>>>>>
>>>>>>>>>> On 23/10/2017 12:41, Semyon Sadetsky wrote:
>>>>>>>>>>>> The AquaMenuBarBorder class is L&F specific, and it should 
>>>>>>>>>>>> not be serialized/deserialized. Such information(plus l&f 
>>>>>>>>>>>> client properties/listeners/etc) should be removed from the 
>>>>>>>>>>>> component before serialization. And during deserialization 
>>>>>>>>>>>> the L&F of the target system should be applied.
>>>>>>>>>>> That is valid concern. The webrev is updated 
>>>>>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8189201/webrev.01/
>>>>>>>>>>
>>>>>>>>>> The same is applicable to ScreenMenuPropertyListener as well 
>>>>>>>>>> because it is also L&F specific.
>>>>>>>>>> I assume that the changes in 
>>>>>>>>>> AccessibleAWTComponentHandler/etc are necessary because 
>>>>>>>>>> somecode installs the listeners on the components, since the 
>>>>>>>>>> bug is not reproduced in Metal I assume that this listeners 
>>>>>>>>>> are installed by Aqua, in this case these listeners also 
>>>>>>>>>> should be removed from the component before serialization.
>>>>>>>>> ScreenMenu is not a Swing component.
>>>>>>>>>
>>>>>>>>> --Semyon
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>