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

[Frank Ding]

Hi Alexandr,
   Thanks for your quick reply.  I will refine the patch and address all 
your concerns ASAP.

Best regards,
Frank

On 11/1/2012 8:08 PM, Alexander Scherbatiy wrote:
> On 11/1/2012 10:35 AM, Frank Ding wrote:
>> Hi Alexandr,
>>   Thanks for your review.  The problem that the JButton does not 
>> release itself as you mentioned is, after taking closer look at 
>> relevant code again, that I removed instance AbstractButton.Handler 
>> as change listener where repainting the button press animation is 
>> defined.  So this time I get around this issue and here comes v6 patch @
>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.06/
>>
>>   Below are answers to what you have raised in previous email.
>> 1. if ( if (!listener.equals(this)) { that 'this' means the 
>> JButtonWrapper instead of an action listener.
>>   'this' refers to JButtonWrapper.this because the closest class 
>> definition is JButtonWrapper.  I also verified it during my debugging.
>
>     Could you give an example there this check is useful? It seems 
> that the JButtonWrapper does not implement ActionListener interface 
> and so the listener and the this always should not be equal.
>
>
>    I dumped action listeners from the button and model before the fix 
> and after it.
>    Before the fix the button has the latest FormView listener and the 
> model has AbstractButton$Handler added each time after a component 
> input creation.
>    After the fix both the button and the model have the FormView 
> listener and the button does not have the AbstractButton$Handler.
>    Could you preserve the listeners location and just remove old 
> listeners from the model?
>
>    The fix sets the model to the JButtonWrapper first and than removes 
> unneccessary listeners from the model. Is it possible to rearrange 
> order of these operations?
>
>    I see that you check number of the listeners in the test. Could you 
> add the check that the action listener is invoked only once after a 
> component tree updating and
>    the action does the same that it does before a component tree 
> updating.
>
>    Thanks,
>    Alexandr.
>>
>> 2. Why are two FormViews created in this sample?
>> SwingUtilities.updateComponentTreeUI() triggers html View hierarchy 
>> to update by instantiating new UI instances involved in the html 
>> form.  It could have been avoided to have 2 View instances but we are 
>> restricted to current design that LF change causes creation of View 
>> instance which further leads to creation of new UI instances.  
>> Creation of new UI instances upon LF change can be referenced in 
>> BasicTextUI.modelChanged().
>>
>> 3. Why are there 2 listeners added to the button model in this sample?
>> The issue is caused by combination of the 2 factors.  First, a shared 
>> ButtonModel is created per html Element.  Second, JButton.setModel() 
>> automatically registers its private Handler in the new model but no 
>> way to deregister it.  The first is a design choice but the second is 
>> a design flaw.  Consequence is that the shared ButtonModel instance 
>> bears more and more JButton.Handler.
>>
>> I think v6 patch is promising as a final fix.  If you have any 
>> further concern or comment, please don't hesitate to let me know. 
>> Thanks again for your careful review.
>>
>> Best regards,
>> Frank
>>
>> On 10/30/2012 9:38 PM, Alexander Scherbatiy wrote:
>>> 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
>>>>
>>
>