<Swing Dev> 7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls

[pavel porvatov]

Hi Frank,
> Hi Pavel,
>   Thanks for you reply.  I have thought it through and have to have 
> this solution.  The pain point is that these private access level 
> listeners are completely encapsulated.  But there is another possible 
> fix that won't bother introducing cleanup api but would require
> 1.  Make ComponentView.setComponentParent method become protected for 
> FormView to override (package level is not adequate because FormView 
> resides under a different package).  This allows FormView instance to 
> determine de-registration condition.
That's public API changing

> 2.  Make ComponentView.c (private Invalidator c) become protected for 
> FormView to access.
That's public API changing as well
>
>  It doesn't bring about new cleanup public api but make hidden 
> setComponentParent and c come to surface.  Do you think it is 
> preferable than cleanup one? 
I believe we should try to find a fix without API changes

> Or I would be more than glad to know another approach.
Unfortunately I don't have time to fix this problem. Does somebody else 
have ideas about the fix?

Regards, Pavel
>
> Best regards,
> Frank
>
> On 10/5/2012 8:39 PM, pavel porvatov wrote:
>> Hi Frank,
>>> Hi Pavel,
>>>   Thanks for your comment.  I improved my fix again and uploaded to 
>>> the following address
>>>   http://cr.openjdk.java.net/~dingxmin/7189299/webrev.03/
>>>
>>>   This revision offers a different approach where
>>> 1.  In ComponentView, setComponentParent method,  the best place 
>>> that is entitled to detect de-registration condition because only in 
>>> class ComponentView can var "Invalidator c" be accessed.
>>> 2.  If the condition is satisfied (this.getParent()==null && 
>>> c.getParent() !=null), call a newly added protected method "cleanup" 
>>> to let subclass do house clean up work.
>>> 3.  FormView.cleanup does the actual work of removing listeners from 
>>> DefaultButtonModel.
>>> 4.  However, listener instances to be de-registered are protected 
>>> members of AbstractButton, we need to have a JButton wrapper 
>>> exposing these listeners, which leads to declaration of 
>>> JButtonWrapper inner class.
>>>
>>>   I think this time is much better than the reflection one. Could 
>>> you please take a took and give your comment?  I may need to refine 
>>> javadoc of cleanup protected method later.
>> You added new method in public API 
>> (javax.swing.text.ComponentView#cleanup). Is it possible to fix the 
>> problem without extending API? I don't think this method will be 
>> useful for other people...
>>
>> Thanks, Pavel
>>>
>>> Thanks and best regards,
>>> Frank
>>>
>>> On 9/13/2012 7:50 PM, Pavel Porvatov wrote:
>>>> Hi Frank,
>>>>> Hi Pavel
>>>>>   It's been a long time since last discussion.  Now I have a new 
>>>>> approach to the issue.
>>>>>   The basic idea is removing the listener pertaining to JButton 
>>>>> instance from DefaultButtonModel when the corresponding FormView 
>>>>> instance is about to retire.
>>>>>
>>>>>   The best place I can find in source code to achieve it is in 
>>>>> FormView's super class ComponentView, method void setComponentParent.
>>>>>   View p = getParent();
>>>>>   if (p != null) {
>>>>>      ....
>>>>>   } else {
>>>>>     // when p is null, it means the FormView instance is about to 
>>>>> retire
>>>>>     // deregister listener off shared DefaultButtonModel instance
>>>>>   }
>>>>>
>>>>>   However, the biggest problem is that the listener to be removed 
>>>>> is a private member of AbstractButton and the only way is holding 
>>>>> the listener object and then calling various removeXXXListener on 
>>>>> DefaultButtonModel with the listener being parameter.  I have to 
>>>>> resort to reflection which makes it look more like a workaround. 
>>>>> What's more, reflection introduces implementation dependency.
>>>>>   In addition to the change to fix, I also updated its jtreg test 
>>>>> case according to your comments.
>>>>>   Could you please review it again @
>>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.02
>>>> We don't use reflection in the SWING library. Could you please try 
>>>> to find another way of fix?
>>>>
>>>> Regards, Pavel
>>>>
>>>
>>
>