<Swing Dev> 7189299 DefaultButtonModel instance keeps stale listeners of JButton in case of multiple SwingUtilities.updateComponentTreeUI() calls
Frank Ding
dingxmin at linux.vnet.ibm.com
Tue Jan 29 05:31:17 UTC 2013
Hi Alexandr,
Please review my 9th attempt @
http://cr.openjdk.java.net/~dingxmin/7189299/webrev.09/
The revision also passed test LoadingWebPageToJEditorPane.
Thanks && Best regards,
Frank
On 1/28/2013 5:04 PM, Frank Ding wrote:
> Hi Alexandr,
> Thanks for your suggestion. I will prepare a patch according to it
> soon.
>
> Best regards,
> Frank
>
> On 1/21/2013 9:41 PM, Alexander Scherbatiy wrote:
>> On 1/18/2013 10:50 AM, Frank Ding wrote:
>>> Hi Alexandr,
>>> Do you have any idea?
>>
>> What about one more way removing listeners:
>> - disadvantage: listeners are hardcoded
>> - advantage: there is no need to override JButton, JList,... classes
>>
>> ------------------------------------------
>> private void clearModel(Object model) {
>> if (model instanceof DefaultButtonModel) {
>> DefaultButtonModel buttonModel = (DefaultButtonModel) model;
>> String listenerClass = "javax.swing.AbstractButton$Handler";
>>
>> for (ActionListener listener :
>> buttonModel.getActionListeners()) {
>> if
>> (listenerClass.equals(listener.getClass().getName())) {
>> buttonModel.removeActionListener(listener);
>> }
>> }
>> // remove change listeners
>> // remove item listeners
>>
>> } else if(model instanceof ListModel){
>> // remove list listeners
>> }
>> }
>> ------------------------------------------
>>
>> Thanks,
>> Alexandr.
>>
>>>
>>> Best regards,
>>> Frank
>>>
>>> On 12/28/2012 11:16 AM, Frank Ding wrote:
>>>> Hi Alexandr,
>>>> I did some more investigations.
>>>> 1. The model can be accessed as below which is also illustrated
>>>> in the jtreg test
>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.07/test/javax/swing/text/html/7189299/bug7189299.java.html
>>>>
>>>>
>>>> HTMLEditorKit htmlEditorKit = (HTMLEditorKit) html.getEditorKit();
>>>> StyleContext.NamedStyle style = ((StyleContext.NamedStyle)
>>>> htmlEditorKit.getInputAttributes());
>>>> DefaultButtonModel model = ((DefaultButtonModel)
>>>> style.getAttribute(StyleConstants.ModelAttribute));
>>>>
>>>> Though it needs type conversion, model can be exposed to client
>>>> code. This implies client code has the chance to add listeners.
>>>> Another way of updating listener in model is by first getting
>>>> JButton or other html Swing components and then call listener
>>>> related api that affects model. I dumped vars above but did not
>>>> find out possibility for application to get Swing component reference.
>>>>
>>>> 2. As of swing.text.html library, I searched 'ModelAttribute'
>>>> under javax/swing directory with command "grep -R 'ModelAttribute'
>>>> ." which shows aside from FormView and StyleConstants, the classes
>>>> referencing it are HTMLDocument, HTMLWriter. This means only
>>>> FormView adds listeners to model.
>>>>
>>>> My first attempt at fixing the problem (only for JButton listener
>>>> leak) is trying to remove all listeners completely. See
>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.01/. Pavel
>>>> argued that it may cause existing application to malfunction. Then
>>>> I came up with
>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.02/ where only
>>>> stale listeners are identified and removed using reflection. But
>>>> Pavel insisted reflection should be prohibited. So I tried to work
>>>> around it by extending JButton in FormView to access the private
>>>> listener instance such that only stale listener is removed. The
>>>> perfect revision is
>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.07/.
>>>>
>>>> I propose we fix JButton issue as it leads to NPE visible to
>>>> application. Bug in other components can be put off if Swing team
>>>> intend to fix it elegantly or duplicate code like what is in v.07.
>>>> Any idea or comment, please let me know.
>>>>
>>>> Thanks and best regards,
>>>> Frank
>>>>
>>>> On 12/10/2012 6:40 PM, Alexander Scherbatiy wrote:
>>>>> On 12/6/2012 12:39 PM, Frank Ding wrote:
>>>>>> Hi Alexandr,
>>>>>> I did several attempts but still have hang issues somewhere.
>>>>>> It seems the new approach of caching gui components created each
>>>>>> time is not practical because of the threading restriction
>>>>>> already imposed on HTMLDocument.
>>>>>> Can we make compromise by turning to previous solution (v7 in
>>>>>> particular) and generalize it to other gui components (which
>>>>>> means there would be JCheckBoxWrapper, JRadioButtonWrapper,
>>>>>
>>>>> The issue is that models contain listeners from all previous
>>>>> components. Models are used to store data and state of components
>>>>> when form view is recreated.
>>>>> FormView adds it's own listeners to components (and as result
>>>>> to the models). Could you make a little investigation to check
>>>>> which other listeners can the models hold?
>>>>> Can a user adds a listener? Can listeners be added by other
>>>>> parts of the swing.text.html library?
>>>>>
>>>>> If old listeners does not play role, it is possible just to
>>>>> remove them all.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>>> JPasswordFieldWrapper, JTextFieldWrapper, JListWrapper,
>>>>>> JComboBoxWrapper, and JButtonWrapper). Or shall we fix JButton
>>>>>> only because it causes obvious exception visible to client? I am
>>>>>> short of ideas...
>>>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.07/
>>>>>>
>>>>>> Best regards,
>>>>>> Frank
>>>>>>
>>>>>> On 12/5/2012 2:12 PM, Frank Ding wrote:
>>>>>>> Hi Alexandr,
>>>>>>> I observed the same issue in my environment as well. My
>>>>>>> proposed patch would cause severe regression issues. I am
>>>>>>> looking into the locking issue.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Frank
>>>>>>>
>>>>>>> On 12/3/2012 6:53 PM, Alexander Scherbatiy wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Could you check the following sample with your latest fix? It
>>>>>>>> just hangs on my side.
>>>>>>>>
>>>>>>>> - put the sample.html and the response.html files on a server
>>>>>>>> - update path to the html files in the
>>>>>>>> LoadingWebPageToJEditorPane class
>>>>>>>>
>>>>>>>> ------------- sample.html ------------
>>>>>>>> <form name="input" action="response.html" method="get">
>>>>>>>> Username: <input type="text" name="user">
>>>>>>>> <input type="submit" value="Submit">
>>>>>>>> </form>
>>>>>>>> ------------- response.html ------------
>>>>>>>> <html>
>>>>>>>> <body>
>>>>>>>> Hello World!
>>>>>>>> </body>
>>>>>>>> </html>
>>>>>>>> ------------- LoadingWebPageToJEditorPane.html ------------
>>>>>>>> import java.net.URL;
>>>>>>>> import javax.swing.JEditorPane;
>>>>>>>> import javax.swing.JFrame;
>>>>>>>> import javax.swing.JScrollPane;
>>>>>>>> import javax.swing.SwingUtilities;
>>>>>>>>
>>>>>>>> public class LoadingWebPageToJEditorPane {
>>>>>>>>
>>>>>>>> private static final String HTML_PATH =
>>>>>>>> "http://serever/path/sample.html";
>>>>>>>>
>>>>>>>> public static void main(String[] a) throws Exception {
>>>>>>>>
>>>>>>>> SwingUtilities.invokeAndWait(new Runnable() {
>>>>>>>>
>>>>>>>> @Override
>>>>>>>> public void run() {
>>>>>>>> try {
>>>>>>>>
>>>>>>>> JFrame frame = new JFrame();
>>>>>>>> frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>>>>>>>>
>>>>>>>> JEditorPane editorPane = new JEditorPane();
>>>>>>>> editorPane.setPage(new URL(HTML_PATH));
>>>>>>>>
>>>>>>>> frame.add(new JScrollPane(editorPane));
>>>>>>>> frame.setSize(300, 200);
>>>>>>>> frame.setVisible(true);
>>>>>>>> SwingUtilities.updateComponentTreeUI(editorPane);
>>>>>>>> } catch (Exception ex) {
>>>>>>>> throw new RuntimeException(ex);
>>>>>>>> }
>>>>>>>> }
>>>>>>>> });
>>>>>>>> }
>>>>>>>> }
>>>>>>>> ---------------------------------------
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alexandr.
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/28/2012 10:29 AM, Frank Ding wrote:
>>>>>>>>> Hi Alexandr,
>>>>>>>>> I created a new patch v8 @
>>>>>>>>> http://cr.openjdk.java.net/~dingxmin/7189299/webrev.08/. It
>>>>>>>>> uses the newly proposed approach mentioned in my last email.
>>>>>>>>> Could you please help to review it again?
>>>>>>>>>
>>>>>>>>> The patch, of course, passed jtreg test bug7189299.java in
>>>>>>>>> the patch. What's more, I did additional tests for JComboBox,
>>>>>>>>> JTextField and JList in my IDE by comparing listener numbers
>>>>>>>>> observed during debugging with/without my patch. The listener
>>>>>>>>> numbers were doubled without the fix. This proves that v8
>>>>>>>>> patch works for all components. I think I can write those
>>>>>>>>> additional tests as jtreg tests after the patch passes review.
>>>>>>>>>
>>>>>>>>> One notable change is that I had to restrict the scope of
>>>>>>>>> holding a write lock in DefaultStyledDocument because
>>>>>>>>> otherwise, we cannot store the new component created in
>>>>>>>>> FormView.createComponent(). The stack trace is pasted below
>>>>>>>>> for reference.
>>>>>>>>>
>>>>>>>>> Exception in thread "main"
>>>>>>>>> java.lang.reflect.InvocationTargetException
>>>>>>>>> at java.awt.EventQueue.invokeAndWait(EventQueue.java:1270)
>>>>>>>>> at
>>>>>>>>> javax.swing.SwingUtilities.invokeAndWait(SwingUtilities.java:1350)
>>>>>>>>>
>>>>>>>>> at bug7189299.main(bug7189299.java:116)
>>>>>>>>> Caused by: java.lang.IllegalStateException: Attempt to mutate
>>>>>>>>> in notification
>>>>>>>>> at
>>>>>>>>> javax.swing.text.AbstractDocument.writeLock(AbstractDocument.java:1357)
>>>>>>>>>
>>>>>>>>> at
>>>>>>>>> javax.swing.text.html.HTMLDocument.obtainLock(HTMLDocument.java:1708)
>>>>>>>>>
>>>>>>>>> at
>>>>>>>>> javax.swing.text.html.FormView.createComponent(FormView.java:211)
>>>>>>>>> ......(omitted)......
>>>>>>>>> at
>>>>>>>>> javax.swing.plaf.basic.BasicTextUI$RootView.insertUpdate(BasicTextUI.java:1602)
>>>>>>>>> at
>>>>>>>>> javax.swing.plaf.basic.BasicTextUI$UpdateHandler.insertUpdate(BasicTextUI.java:1861)
>>>>>>>>> at
>>>>>>>>> javax.swing.text.AbstractDocument.fireInsertUpdate(AbstractDocument.java:201)
>>>>>>>>>
>>>>>>>>> at
>>>>>>>>> javax.swing.text.DefaultStyledDocument.create(DefaultStyledDocument.java:155)
>>>>>>>>> <------ FormView.createComponent is triggered by this call
>>>>>>>>> which has already held the lock
>>>>>>>>> at
>>>>>>>>> javax.swing.text.html.HTMLDocument.create(HTMLDocument.java:469)
>>>>>>>>> ......(omitted)......
>>>>>>>>>
>>>>>>>>> This change did violate what is documented in
>>>>>>>>> AbstractDocument.writeLock that "This situation violates the
>>>>>>>>> bean event model where order of delivery is not guaranteed and
>>>>>>>>> all listeners should be notified before further mutations are
>>>>>>>>> allowed. " However, without the change of shrinking lock
>>>>>>>>> scope, the component cannot be saved once it is created in
>>>>>>>>> FormView.createComponent(). I found it is even harder to save
>>>>>>>>> it later in code but perhaps there does exist a more
>>>>>>>>> appropriate place to do this. If you have any better
>>>>>>>>> suggestion, I am glad to know.
>>>>>>>>>
>>>>>>>>> Also I searched references to
>>>>>>>>> 'StyleConstants.ComponentAttribute' as you asked. The result
>>>>>>>>> is listed below. Note that the command 'grep' is invoked
>>>>>>>>> under openjdk 8 top directory.
>>>>>>>>> $ grep -R 'ComponentAttribute' .
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/StyleConstants.java:
>>>>>>>>> public static final Object ComponentAttribute = new
>>>>>>>>> CharacterConstants("component");
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/StyleConstants.java:
>>>>>>>>> return (Component) a.getAttribute(ComponentAttribute);
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/StyleConstants.java:
>>>>>>>>> a.addAttribute(ComponentAttribute, c);
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/StyleConstants.java:
>>>>>>>>> Background, ComponentAttribute, IconAttribute,
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/StyledEditorKit.java:
>>>>>>>>> set.removeAttribute(StyleConstants.ComponentAttribute);
>>>>>>>>> Binary file ./jdk/test/sun/tools/jhat/jmap.bin matches
>>>>>>>>>
>>>>>>>>> So ComponentAttribute is not referenced by other classes
>>>>>>>>> except StyledEditorKit. StyleConstants exposes
>>>>>>>>> ComponentAttribute in getComponent() and setComponent()
>>>>>>>>> methods. References to StyleConstants.getComponent and
>>>>>>>>> StyleConstants.setComponent are further searched in repo.
>>>>>>>>>
>>>>>>>>> $ grep -R 'StyleConstants.getComponent' .
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/ComponentView.java: *
>>>>>>>>> the element (by calling StyleConstants.getComponent). A
>>>>>>>>> ./jdk/src/share/classes/javax/swing/text/ComponentView.java:
>>>>>>>>> Component comp = StyleConstants.getComponent(attr);
>>>>>>>>>
>>>>>>>>> $ grep -R 'StyleConstants.setComponent' .
>>>>>>>>> ./jdk/src/share/classes/javax/swing/JTextPane.java:
>>>>>>>>> StyleConstants.setComponent(inputAttributes, c);
>>>>>>>>>
>>>>>>>>> From the facts above, I think we are sufficiently confident to
>>>>>>>>> use ComponentAttribute.
>>>>>>>>>
>>>>>>>>> Please let me know if you have any comments and I can do more
>>>>>>>>> regression tests and provide more jtreg test cases as next step.
>>>>>>>>>
>>>>>>>>> Thanks and regards,
>>>>>>>>> Frank
>>>>>>>>>
>>>>>>>>> On 11/15/2012 5:18 PM, Alexander Scherbatiy wrote:
>>>>>>>>>> On 11/14/2012 10:22 AM, Frank Ding wrote:
>>>>>>>>>>> Hi Alexandr,
>>>>>>>>>>> After a couple of hours investigating the possibility of
>>>>>>>>>>> fixing JComboBox.setModel(null) and
>>>>>>>>>>> JTextComponent.setComponent(null), I found that
>>>>>>>>>>> 1. In JComboBox.setModel, the new model, null in this case,
>>>>>>>>>>> is eventually passed to BasicComboPopup.propertyChange where
>>>>>>>>>>> JList.setModel is called. JList.setModel rejects the null
>>>>>>>>>>> model because of its api restriction. Below I listed the
>>>>>>>>>>> offending call stacks in my IDE. This makes sense as the
>>>>>>>>>>> associated drop down JList needs new model. However, it
>>>>>>>>>>> makes fixing JComboBox.setModel(null) hard or impossible.
>>>>>>>>>>> Exception in thread "main"
>>>>>>>>>>> java.lang.IllegalArgumentException: model must be non null
>>>>>>>>>>> at javax.swing.JList.setModel(JList.java:1674)
>>>>>>>>>>> at
>>>>>>>>>>> javax.swing.plaf.basic.BasicComboPopup$Handler.propertyChange(BasicComboPopup.java:939)
>>>>>>>>>>> at
>>>>>>>>>>> java.beans.PropertyChangeSupport.fire(PropertyChangeSupport.java:335)
>>>>>>>>>>>
>>>>>>>>>>> at
>>>>>>>>>>> java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:327)
>>>>>>>>>>> at
>>>>>>>>>>> java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:263)
>>>>>>>>>>> at
>>>>>>>>>>> java.awt.Component.firePropertyChange(Component.java:8413)
>>>>>>>>>>> at javax.swing.JComboBox.setModel(JComboBox.java:322)
>>>>>>>>>>>
>>>>>>>>>>> 2. JTextComponent.setComponent(null) can be fixed but code
>>>>>>>>>>> change in BasicTextUI is also required.
>>>>>>>>>>>
>>>>>>>>>>> Since setting null model to JComboBox, JList and
>>>>>>>>>>> JTextComponent is impossible or dangerous, just as you
>>>>>>>>>>> mentioned, we could set a non null new model to these UI
>>>>>>>>>>> components just for the purpose of having the side effect of
>>>>>>>>>>> removing listeners from old model. Are you ok with this
>>>>>>>>>>> approach?
>>>>>>>>>> Yes. Please, try this and run the html swing automated
>>>>>>>>>> tests from the test/javax/swing/text/html diroctory to check
>>>>>>>>>> possible regressions.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Alexandr.
>>>>>>>>>>
>>>>>>>>>>> By the way, I will investigate your another question "Could
>>>>>>>>>>> you also check that the StyleConstants.ComponentAttribute
>>>>>>>>>>> property value can't be rewritten by the JDK code or by
>>>>>>>>>>> public methods." and reply soon.
>>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Frank
>>>>>>>>>>>
>>>>>>>>>>> On 11/13/2012 7:49 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>> On 11/13/2012 11:53 AM, Frank Ding wrote:
>>>>>>>>>>>>> Hi Alexandr,
>>>>>>>>>>>>> As for your comment "Could you create an issue that a
>>>>>>>>>>>>> JComboBox.setModel(null) call throws NPE? You could fix it
>>>>>>>>>>>>> before the 7189299 issue. ", I created a bug with internal
>>>>>>>>>>>>> review ID of 2381499 on JComboBox.setModel(null). But
>>>>>>>>>>>>> test shows that JPasswordField.setDocument(null),
>>>>>>>>>>>>> JTextField.setDocument(null), JList.setModel(null) and
>>>>>>>>>>>>> JTextArea.setDocument(null) all throw NPE. Particularly,
>>>>>>>>>>>>> JList.setModel(null) has null check and throws
>>>>>>>>>>>>> IllegalArgumentException("model" must be non null").
>>>>>>>>>>>>> Shall I include those components as well?
>>>>>>>>>>>>
>>>>>>>>>>>> There is the javadoc for the JList setModel() method:
>>>>>>>>>>>> Throws IllegalArgumentException - if model is null. So it
>>>>>>>>>>>> is undesirable to change the public API.
>>>>>>>>>>>> However, it is possible to set a new empty model for
>>>>>>>>>>>> the JList. The list listeners should be removed from the
>>>>>>>>>>>> old model in this case.
>>>>>>>>>>>>
>>>>>>>>>>>> You could have only 2 issues: one for components that
>>>>>>>>>>>> allow to set a null model but throws NPE (like JComboBox)
>>>>>>>>>>>> and another for components that does not allow to set null
>>>>>>>>>>>> model but they do not remove listeners from the old model
>>>>>>>>>>>> in case if a new model is set.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for your guidance in advance.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>> Frank
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 11/8/2012 10:55 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>> On 11/7/2012 10:56 AM, Frank Ding wrote:
>>>>>>>>>>>>>>> Hi Alexandr,
>>>>>>>>>>>>>>> Unfortunately, all the JComponents involved in
>>>>>>>>>>>>>>> FormView.createComponent() have bugs!
>>>>>>>>>>>>>>> I have done tests for all other swing components, i.e.
>>>>>>>>>>>>>>> JCheckBox, JRadioButton, JPasswordField, JTextField,
>>>>>>>>>>>>>>> JList, JComboBox and JTextField. Sadder news is that if
>>>>>>>>>>>>>>> we fix all components in the same way as I did for
>>>>>>>>>>>>>>> JButton, we need to subclass them all, resulting in
>>>>>>>>>>>>>>> JCheckBoxWrapper, JRadioButtonWrapper,
>>>>>>>>>>>>>>> JPasswordFieldWrapper, JTextFieldWrapper, JListWrapper,
>>>>>>>>>>>>>>> JComboBoxWrapper, JTextFieldWrapper plus
>>>>>>>>>>>>>>> JButtonWrapper! This approach becomes unacceptable when
>>>>>>>>>>>>>>> all swing components are affected.
>>>>>>>>>>>>>>> Shall we fix it in other way illustrated below?
>>>>>>>>>>>>>>> 1. Whenever a swing component is created, it is kept in
>>>>>>>>>>>>>>> AttributeSet, as what is now for model.
>>>>>>>>>>>>>>> 2. In FormView.createComponent(), the old swing
>>>>>>>>>>>>>>> component can be retrieved via
>>>>>>>>>>>>>>> attr.getAttribute(StyleConstants.ComponentAttribute).
>>>>>>>>>>>>>>> Note that ComponentAttribute is newly added.
>>>>>>>>>>>>>> This way should be carefully investigated that it
>>>>>>>>>>>>>> does not introduce new memory leaks.
>>>>>>>>>>>>>> Could you also check that the
>>>>>>>>>>>>>> StyleConstants.ComponentAttribute property value can't be
>>>>>>>>>>>>>> rewritten by the JDK code or by public methods.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 3. Before setting shared model to a newly initialized
>>>>>>>>>>>>>>> swing component, call oldComp.setModel(null), delegating
>>>>>>>>>>>>>>> deregistration to setModel method.
>>>>>>>>>>>>>>> 4. Seems some setModel such as AbstractButton.setModel()
>>>>>>>>>>>>>>> can function well when the param, new model, is null
>>>>>>>>>>>>>>> while JComboBox.setModel() will throw NPE in case of
>>>>>>>>>>>>>>> null param.
>>>>>>>>>>>>>> Could you create an issue that a
>>>>>>>>>>>>>> JComboBox.setModel(null) call throws NPE? You could fix
>>>>>>>>>>>>>> it before the 7189299 issue.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> 5. Add null check code in those problematic setModel or
>>>>>>>>>>>>>>> setDocument methods.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Any idea is appreciated. Thanks.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>>> Frank
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 11/6/2012 9:26 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the swing-dev
mailing list