Hi Sean,
Hi Pavel,

I modified the testcase according to your comments. The webrev is http://cr.openjdk.java.net/~zhouyx/7129742/webrev.06/ <http://cr.openjdk.java.net/%7Ezhouyx/7129742/webrev.06/> . Please take a look again.
And now when fastreturn is true the test doesn't stop.

Regards, Pavel

On Thu, Apr 12, 2012 at 10:24 PM, Pavel Porvatov <pavel.porva...@oracle.com <mailto:pavel.porva...@oracle.com>> wrote:

    Hi Sean,

    The fix looks good, but I have several comments about the test:
    1. You shouldn't use Swing components on non-EDT threads, so
    frame.dispose() should be done on the EDT

I made a really stupid mistake... When I was checking the testcase and found frame.dispose() in main method, I added a volatile to the frame variable...

    2. "These exceptions mean the implementation of XTextAreaPeer is
    changed" - I think is XTextAreaPeer is changed, then the test
    should be fixed as well or removed (if the test become
    inapplicable. Therefore in that situation the test should fail but
    not skipped

    Regards, Pavel


    Hi Pavel,

        As awt team has committed the related
    bug(http://hg.openjdk.java.net/jdk8/awt/jdk/rev/96340349e35b), I
    prepared the webrev for this bug(7129742
    <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7129742>) again.

        The new webrev:
    http://cr.openjdk.java.net/~zhouyx/7129742/webrev.05/
    <http://cr.openjdk.java.net/%7Ezhouyx/7129742/webrev.05/>  .

    The sunbug:
    http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7129742
    Previous discussion:
    http://mail.openjdk.java.net/pipermail/swing-dev/2012-March/001923.html


        Please take a look once more, thank you.

    On Thu, Mar 15, 2012 at 8:57 PM, Pavel Porvatov
    <pavel.porva...@oracle.com <mailto:pavel.porva...@oracle.com>> wrote:

        Hi Sean,

        Hi Pavel,

           Thanks for your comments.

        About the 1st question, I modified the testcase a little to
        demonstrate the problem, testcase is pasted at the end.

        If textArea.setEditable(true) is executed, the frame
        disposes, but application doesn't exit.
        If textArea.setEditable(false), the application exits as normal.

        java 1.7.0_01 and latest java8 code are tested.  And
        TextField contains this problem as well. I'll add it to fix
        later.
        Shall I report a separate bug for it ?
        Yes. I think it will be better if you fix the described above
        problem as a separate bug. Note that it should be reviewed by
        the AWT team...



        About the 2nd question, I was driven by the comment "// TODO
        : fix this duplicate code". Is there any strong reason to
        keep it ?
        I'm absolutely agree with removing duplicate code. My second
        question was about added by you
        "jtext.getCaret().setVisible(false)" in the
        XTextAreaPeer.java class. I wrote:
        2. Why don't we need the same code in the XTextFieldPeer class?

        As I can see you've noticed that XTextFieldPeer should be
        fixed as well (in a separate fix with XTextAreaPeer)

        Regards, Pavel



        ///////////////////////////////////  a testcase
         ////////////////////////////////////////////////////////////
        /*
         * Copyright (c) 2012 Oracle and/or its affiliates. All
        rights reserved.
         * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
         *
         * This code is free software; you can redistribute it
        and/or modify it
         * under the terms of the GNU General Public License version
        2 only, as
         * published by the Free Software Foundation.
         *
         * This code is distributed in the hope that it will be
        useful, but WITHOUT
         * ANY WARRANTY; without even the implied warranty of
        MERCHANTABILITY or
         * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
        Public License
         * version 2 for more details (a copy is included in the
        LICENSE file that
         * accompanied this code).
         *
         * You should have received a copy of the GNU General Public
        License version
         * 2 along with this work; if not, write to the Free
        Software Foundation,
         * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
         *
         * Please contact Oracle, 500 Oracle Parkway, Redwood
        Shores, CA 94065 USA
         * or visit www.oracle.com <http://www.oracle.com> if you
        need additional information or have any
         * questions.
         */

        /*
         * Portions Copyright (c) 2012 IBM Corporation
         */

        /* @test
         * @bug
         * @summary editable TextArea blocks gui app from dispose.
         * @author Sean Chou
         */

        import java.awt.FlowLayout;
        import java.awt.Frame;
        import java.awt.TextArea;
        import java.awt.Toolkit;
        import java.lang.reflect.Field;

        import javax.swing.JFrame;
        import javax.swing.JTextArea;
        import javax.swing.SwingUtilities;
        import javax.swing.text.DefaultCaret;

        import sun.awt.SunToolkit;

        public class TextAreaDisposeBug {
            public static volatile boolean passed = false;

            public static Frame frame = null;
            public static TextArea textArea = null;

            public static DefaultCaret caret = null;
            public static Class XTextAreaPeerClzz = null;

            public static void main(String[] args) throws Exception {
                SunToolkit toolkit = (SunToolkit)
        Toolkit.getDefaultToolkit();
                SwingUtilities.invokeAndWait(new Runnable() {
                    @Override
                    public void run() {
                        frame = new JFrame("Test");
                        textArea = new TextArea("editable textArea");
                        textArea.setEditable(true);
        //                textArea.setEditable(false);

                        frame.setLayout(new FlowLayout());
                        frame.add(textArea);

                        frame.pack();
                        frame.setVisible(true);
                    }
                });
                toolkit.realSync();
                SwingUtilities.invokeAndWait(new Runnable() {
                    @Override
                    public void run() {
                        frame.dispose();
                    }
                });
                toolkit.realSync();
            }

        }


        
//////////////////////////////////////////////////////////////////////////////////////////

        /*
         * Copyright (c) 2012 Oracle and/or its affiliates. All
        rights reserved.
         * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
         *
         * This code is free software; you can redistribute it
        and/or modify it
         * under the terms of the GNU General Public License version
        2 only, as
         * published by the Free Software Foundation.
         *
         * This code is distributed in the hope that it will be
        useful, but WITHOUT
         * ANY WARRANTY; without even the implied warranty of
        MERCHANTABILITY or
         * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General
        Public License
         * version 2 for more details (a copy is included in the
        LICENSE file that
         * accompanied this code).
         *
         * You should have received a copy of the GNU General Public
        License version
         * 2 along with this work; if not, write to the Free
        Software Foundation,
         * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
         *
         * Please contact Oracle, 500 Oracle Parkway, Redwood
        Shores, CA 94065 USA
         * or visit www.oracle.com <http://www.oracle.com> if you
        need additional information or have any
         * questions.
         */

        /*
         * Portions Copyright (c) 2012 IBM Corporation
         */


        /* @test
         * @bug
         * @summary editable TextField blocks gui app from dispose.
         * @author Sean Chou
         */

        import java.awt.FlowLayout;
        import java.awt.Frame;
        import java.awt.TextField;
        import java.awt.Toolkit;
        import java.lang.reflect.Field;

        import javax.swing.JFrame;
        import javax.swing.JTextArea;
        import javax.swing.SwingUtilities;
        import javax.swing.text.DefaultCaret;

        import sun.awt.SunToolkit;

        public class TextFieldDisposeBug {
            public static volatile boolean passed = false;

            public static Frame frame = null;
            public static TextField textField = null;

            public static DefaultCaret caret = null;
            public static Class XTextAreaPeerClzz = null;

            public static void main(String[] args) throws Exception {
                SunToolkit toolkit = (SunToolkit)
        Toolkit.getDefaultToolkit();
                SwingUtilities.invokeAndWait(new Runnable() {
                    @Override
                    public void run() {
                        frame = new JFrame("Test");
                        textField = new TextField("editable textArea");
        //                textField.setEditable(true);
                        textField.setEditable(false);

                        frame.setLayout(new FlowLayout());
                        frame.add(textField);

                        frame.pack();
                        frame.setVisible(true);
                    }
                });
                toolkit.realSync();
                SwingUtilities.invokeAndWait(new Runnable() {
                    @Override
                    public void run() {
                        frame.dispose();
                    }
                });
                toolkit.realSync();
            }

        }


        On Tue, Mar 13, 2012 at 9:49 PM, Pavel Porvatov
        <pavel.porva...@oracle.com
        <mailto:pavel.porva...@oracle.com>> wrote:

            Hi Sean,

            I have a couple questions about the following code in
            the XTextAreaPeer.java class
            +        // visible caret has a timer thread, which must
            be stopped
            +        jtext.getCaret().setVisible(false);

            1. Why this code wasn't needed before your fix, when
            caret was visible for enabled text area?
            2. Why don't we need the same code in the XTextFieldPeer
            class?

            About the bug7129742 test:
            Because of the passed field is used from different
            threads you must use synchronization or mark the field
            as a volatile.

            Regards, Pavel


            Hi Alexander,

                XTextFieldPeer and   XTextAreaPeer have a same
            inner class XAWTCaret, and in  XTextAreaPeer  there is
            also a comment:
            "// TODO : fix this duplicate code "   before XAWTCaret
            . So I removed the XAWTCaret in XTextFieldPeer and
            changed the
            XAWTCaret  into a static class, so XTextFieldPeer can
            use XAWTCaret from XTextAreaPeer  .

                As XAWTCaret is only used in the following
            method  in both XTextAreaPeer and  XTextFieldPeer .
            protected Caret createCaret() {
             return new XAWTCaret();
             }
                I think this modification should not bring side effect.

            On Mon, Mar 12, 2012 at 1:27 AM, Alexander Potochkin
            <alexander.potoch...@oracle.com
            <mailto:alexander.potoch...@oracle.com>> wrote:

                Hello Sean

                Could you give more details about your changes in
                XTextFieldPeer?

                Thanks
                alexp

                    Hi all,

                      I updated the patch to as suggested and
                    simplified the testcase .
                    Would anyone like to take a look again ? Thanks.

                     The webrev is at :
                    http://cr.openjdk.java.net/~zhouyx/7129742/webrev.04/
                    <http://cr.openjdk.java.net/%7Ezhouyx/7129742/webrev.04/>
                    <http://cr.openjdk.java.net/%7Ezhouyx/7129742/webrev.04/>



                     Previous discussion at :
                    
http://mail.openjdk.java.net/pipermail/swing-dev/2012-February/001913.html

-- Best Regards,
                    Sean Chou





-- Best Regards,
            Sean Chou





-- Best Regards,
        Sean Chou





-- Best Regards,
    Sean Chou





--
Best Regards,
Sean Chou


Reply via email to