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

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Thu Feb 7 12:06:57 UTC 2013


   The fix looks good for me.

   Thanks,
   Alexandr.


On 1/29/2013 9:31 AM, Frank Ding wrote:
> 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