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

[Semyon Sadetsky]

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