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

[Semyon Sadetsky]

Thanks. The CSR was created.

--Semyon


On 11/28/2017 12:38 PM, Phil Race wrote:
>
> 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
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>