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

Tue Oct 30 13:38:37 UTC 2012

Hi Frank,

I tried your V5 patch and run the test described in the issue (see below).
I pressed the button and it does not release. This is because the 
AbstractButton.Handler listener is removed because of the fix.
You could check the line
     941   if (!listener.equals(this)) {
that 'this' means the JButtonWrapper instead of an action listener.

Could you also look deeper in the code and say why two FormViews are 
created (and each adds it's own listener to the button model) in this 
sample?

-----------------------------------------------
import java.awt.*;
import javax.swing.*;

public class FormSubmit {

     public static void main(String[] args) throws Exception {
         SwingUtilities.invokeAndWait(new Runnable() {

             @Override
             public void run() {

                 JEditorPane html = new JEditorPane("text/html", 
"<html><body><FORM ACTION=\"examplescript.cgi\"><INPUT type=\"submit\" 
value=\"Submit\"></FORM></BODY></HTML>");
                 JFrame frame = new JFrame();
                 frame.setLayout(new BorderLayout());
                 frame.add(html, BorderLayout.CENTER);
                 frame.setSize(200, 100);
                 frame.setDefaultCloseOperation(frame.EXIT_ON_CLOSE);
                 frame.setVisible(true); // Uncomment this line to see 
the Exception on JDK 7
                 SwingUtilities.updateComponentTreeUI(html);
             }
         });
     }
}
-----------------------------------------------

Thanks,
Alexandr.

On 10/29/2012 11:48 AM, Frank Ding wrote:
> Hi all,
>   Seems Pavel has been busy these days.  Is anybody willing to review 
> v5 change?
>
> Thanks,
> Frank
>
> On 10/17/2012 1:51 PM, Frank Ding wrote:
>> Hi Pavel,
>>   Thanks for your comment pointing out the problems posed by static 
>> fields.  V4 cannot be an accepted work around unless static fields 
>> are avoided.  But I think in v4 removing stale listeners is only 
>> applied to those installed by FormView.
>>   I have revised the approach adopted in v4 where a JButtonWrapper 
>> class is declared for the purpose of accessing actionListener, 
>> changeListener and itemListener defined in its base class.  In 
>> addition, overriding setModel method in JButtonWrapper allows us to 
>> undo registration of aforementioned listeners on the newModel passed 
>> in as setModel method parameter.  In order to have FormView function 
>> correctly in terms of triggering the actionPerformed method defined 
>> in FormView,  setModel also explicitly add FormView.this as action 
>> listener to the newModel.  Further more, setModel deregisters any 
>> action listeners of class FormView because that is the stale action 
>> listener registered by setModel last time.  In doing all the tricks 
>> in setModel, we successfully achieved the goal of removing stale 
>> listener while making FormView JButton properly function.
>>
>>   By the way, I also updated the jtreg test case bug7189299.java a 
>> little bit to assert number of item listeners and change listeners 
>> because in setModel, the default changeListener and itemListener 
>> pertaining to the JButton are removed as well.
>>
>>   The v5 code review is @
>>   http://cr.openjdk.java.net/~dingxmin/7189299/webrev.05/
>>
>>   Your review is highly appreciated.
>>
>> Thanks and best regards,
>> Frank
>>
>> On 10/15/2012 5:16 PM, pavel porvatov wrote:
>>> Hi Frank,
>>>
>>> You are still removing all listeners, not only installed by FormView.
>>>
>>> You also introduced new static field (private static JButtonWrapper 
>>> currJButton = null), but there are a lot problems with static fields:
>>>
>>> 1. Memory leaks: static fields hold reference to potentially huge 
>>> objects unlimited time
>>> 2. Conflicts between applications from different AppContext (e. g. 
>>> applets)
>>> etc.
>>>
>>> So the best way is to avoid static fields at all.
>>>
>>> Regards, Pavel
>>>
>>> Hi Pavel,
>>>>   I happened to have another way of working around it though 
>>>> admittedly it's ugly.  Actually a good solution instead of a 
>>>> workaround requires either involving large code modification or 
>>>> introduction of new api.
>>>>   Idea of v4 change this time is keeping track of every 
>>>> JButtonWrapper instance once instantialized so that the listeners 
>>>> pertaining to stale JButton instance can be de-registered upon new 
>>>> JButton creation.
>>>>   You can check out code review below
>>>>   http://cr.openjdk.java.net/~dingxmin/7189299/webrev.04/
>>>>
>>>>   Your prompt comment is warmly welcomed and expected.
>>>>
>>>> Best regards,
>>>> Frank
>>>>
>>>>
>>>
>>
>