<Swing Dev> [14] RFR JDK-8190763 - Class cast exception on (CompoundEdit) UndoableEditEvent.getEdit()

[Sergey Bylokhov]

Thank you for clarification, the fix looks fine.

On 11/14/19 11:19 pm, Tejpal Rebari wrote:
> Hi Sergey,
> 
>> This version actually raised another question, do we really need DefaultDocumentEventUndoableWrapper
>> which transfers everything to the dde?
> 
> Without using DefaultDocumentEventUndoableWrapper the test javax/swing/undo/UndoManager/AbstractDocumentUndoConcurrentTest.java
> Would fail.
> 
> The DefaultDocumentEventUndoableWrapper was introduced as a fix to JDK-8030702 to avoid deadlock between subclass of AbstractDocument and undoManager.
> 
> It has two methods lockEdit() and unlockEdit()  which is not there in DefaultDocumentEvent.
> These method are there in UndoableEditLockSupport which was also introduced as a fix of JDK-8030702.
> 
> Regards
> Tejpal
> 
> 
>>
>>> Regards
>>> Tejpal
>>>>
>>>> On 11/11/19 11:04 pm, Pankaj Bansal wrote:
>>>>> Looks good to me
>>>>> -Pankaj
>>>>> *From:*Prasanta Sadhukhan
>>>>> *Sent:* Tuesday, November 12, 2019 11:30 AM
>>>>> *To:* Tejpal Rebari; Pankaj Bansal
>>>>> *Cc:* swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net> <mailto:swing-dev at openjdk.java.net>
>>>>> *Subject:* Re: <Swing Dev> [14] RFR JDK-8190763 - Class cast exception on (CompoundEdit) UndoableEditEvent.getEdit()
>>>>> Please remove ununsed imports from the test(no need for new webrev for me), other than that, looks ok to me.
>>>>> Regards
>>>>> Prasanta
>>>>> On 12-Nov-19 11:24 AM, Tejpal Rebari wrote:
>>>>>    Hi Prasanta,
>>>>>    I have added try-finally in the main and called frame.dispose from finally clause.
>>>>>    Please find the updated webrev
>>>>> http://cr.openjdk.java.net/~trebari/swing/8190763/webrev3/
>>>>>    Regards
>>>>>    Tejpal
>>>>>        On 11-Nov-2019, at 4:34 PM, Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com <mailto:prasanta.sadhukhan at oracle.com><mailto:prasanta.sadhukhan at oracle.com>> wrote:
>>>>>        dispose is called from createTextArea when CCE is thrown so ideally the EDT should be inside dispose() function. I guess it will be clean if you remove dispose() from CCE and add frame.dispose() under try-finally clause in main() itself. I guess frame.setIconImage() also is not needed.
>>>>>        Also, please add @Override annotation to run() method of EDT.
>>>>>        On 11-Nov-19 3:07 PM, Tejpal Rebari wrote:
>>>>>            Hi all,
>>>>>            I have removed GridBagLayout() ,GridBagConstraints and author tag ,
>>>>>            moved dispose() call to EDT
>>>>>            and changed the test name to TestCCEOnEditEvet.
>>>>>            Please find the updated webrev
>>>>> http://cr.openjdk.java.net/~trebari/swing/8190763/webrev2/
>>>>>            Regards
>>>>>            Tejpal
>>>>>                On 11-Nov-2019, at 12:02 PM, Pankaj Bansal <pankaj.b.bansal at oracle.com <mailto:pankaj.b.bansal at oracle.com><mailto:pankaj.b.bansal at oracle.com>> wrote:
>>>>>                The fix look good to me.
>>>>>                Some points about test case.
>>>>>                1.No need for author tag. This is not followed now.
>>>>>                2.The test case name should be something meaningful instead of bug***.
>>>>>                3.You should not need the GridBagConstraints code anywhere to reproduce the issue.  You should remove this.
>>>>>                Regards,
>>>>>                Pankaj
>>>>>                *From:*Prasanta Sadhukhan
>>>>>                *Sent:*Monday, November 11, 2019 11:47 AM
>>>>>                *To:*Tejpal Rebari
>>>>>                *Cc:*swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net><mailto:swing-dev at openjdk.java.net>
>>>>>                *Subject:*Re: <Swing Dev> [14] RFR JDK-8190763 - Class cast exception on (CompoundEdit) UndoableEditEvent.getEdit()
>>>>>                Fix looks ok to me. Regarding the test, dispose() should be called under EDT, just as you have done for createAndShowGUI().Also, I don't think createGridBagLayout() code is necessary to recreate the issue.
>>>>>                Regards
>>>>>                Prasanta
>>>>>                On 08-Nov-19 3:56 PM, Tejpal Rebari wrote:
>>>>>                    Hi Prasanta ,
>>>>>                    I have added a  test to the fix.
>>>>>                    Updated webrev :http://cr.openjdk.java.net/~trebari/swing/8190763/webrev1/
>>>>>                    Regards
>>>>>                    Tejpal
>>>>>                        On 31-Oct-2019, at 1:58 PM, Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com <mailto:prasanta.sadhukhan at oracle.com><mailto:prasanta.sadhukhan at oracle.com>> wrote:
>>>>>                        Hi tejpal,
>>>>>                        Can you add a testcase to the fix? I guess one is there in the JBS itself.
>>>>>                        Regards
>>>>>                        Prasanta
>>>>>                        On 24-Oct-19 10:31 AM, Tejpal Rebari wrote:
>>>>>                            Hi All,
>>>>>                            Please review the following fix for jdk14.
>>>>>                            Bug:https://bugs.openjdk.java.net/browse/JDK-8190763
>>>>>                            Webrev:http://cr.openjdk.java.net/~trebari/swing/8190763/webrev0/
>>>>>                            Issue : This issue is a regression ofhttps://bugs.openjdk.java.net/browse/JDK-8030702.
>>>>>                            When UndoableEditEvent.getEdit() is casted to CompoundEdit, it throws class cast exception.
>>>>>                            Fix :  In JDK-8030702 <https://bugs.openjdk.java.net/browse/JDK-8030702>  a new class DefaultDocumentEventUndoableWrapper was introduced.
>>>>>                            UndoableEditEvent.getEdit() returns an object of DefaultDocumentEventUndoableWrapper and when it is casted to CompoundEdit it throws class cast exception because
>>>>>                            it doesn’t inherit CompoundEdit.
>>>>>                            Before the fix of JDK-8030702 <https://bugs.openjdk.java.net/browse/JDK-8030702>  , UndoableEditEvent.getEdit() used to return an object of DefaultDocumentEvent which inherits CompoundEdit so the class cast exception was not thrown.
>>>>>                            The solution is to make DefaultDocumentEventUndoableWrapper a subclass of DefaultDocumentEvent.
>>>>>                            Testing : I have tested it on Mac, Windows and Ubuntu.
>>>>>                            Regards
>>>>>                            Tejpal
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>
>>
>> -- 
>> Best regards, Sergey.
> 


-- 
Best regards, Sergey.