<Swing Dev> 7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Tue Nov 6 13:26:53 UTC 2012
On 11/5/2012 11:20 AM, Frank Ding wrote:
> Hi Alexander,
> Based on your comments last time, I refined my patch of v6 and
> offered v7 @
> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.07/
This version of the fix looks good for me.
It seems that it is the only good way to check that a button model
contains AbstarctButton.Handler listener.
Could you also check that others models used in the
createComponent() method do not have such memory leaks (even the NPE is
not thrown)?
> 4. 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?
> Answer: I am afraid I could not make it in the auto test
> (bug7189299.java) but I can achieve it to some degree in manual test
> FormSubmit, the one you illustrated below.
> My idea of checking it in FormSubmit.java is subclassing JEditorPane
> and overriding 'public EditorKit getEditorKit()' where stack traces
> can be examined in the overridden method to make sure
> FormView.submitData occurs only once. This approach works because
> FormView.submitData() calls JEditorPane.getEditorKit but is tricky.
> However, it's the only way I can think of without any additional
> framework support. If you are in favor of adding the check in
> FormSubmit.java, I am willing to do that.
At least a separated manual test can be added that asks a user to
put a response.html file to a server and according to the server url
checks that JTeditor pane show the response text after a button pressing.
html = new JEditorPane("text/html",
"<html><body><form action=\"" + userURL + "\">"
+ "<input type=submit name=submit
value=\"submit\"/>"
+ "</form></body></html>");
response.html:
<html> <body> Hello World! </body> </html>
Thanks,
Alexandr.
> Thanks again for all your comments and your support. Once again, if
> you have any further concern or comment, please don't hesitate to let
> me know.
>
> 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.
>
More information about the swing-dev
mailing list