Hello Leif
Looks good to me
I had a look at the PolicyTool code
and indeed it is better to leave the IO processing as is at this moment.
Thanks
alexp
Hi All,
A new webrev is available at:
http://cr.openjdk.java.net/~weijun/7025699/webrev.01/
The following new changes were made:
1. Added call to SwingUtilities.invokeLater() in main() to run the GUI
initialization on the Event Dispatch Thread.
2. Minor corrections to initial size and placement of the main window.
3. More mnemonics defined for buttons and labeled fields on the Policy
Entry and KeyStore dialogs.
Note that we have decided to not implement the use of SwingWorker for
File I/O in this release. The main reasons are that it is not covered
in the scope of the bug report, the blocking can not actually be
considered a regression compared to the AWT version, and it would
require enough refactoring of the code to risk affecting the general
logic and cause new bugs.
I am the Contributor for this bug fix, and Max (Weijun) will be the
Committer.
Thanks,
Leif
On 2013-10-15 10:00, Leif Samuelsson wrote:
Thanks for the good feedback. We will make sure to move to move the
GUI instantiation to the EDT and the file I/O to a worker thread.
All other operations are event driven and therefore occur directly
on the EDT.
Leif
On 2013-10-15 09:56, alexander potochkin wrote:
Hello
Well, performing I/O or other blocking operations on EDT can only
freeze the app's GUI for the period of blocking. If
developers/users are OK with that, this is fine with me too.
I don't think an I/O blocking operation on EDT is acceptable. It can
freeze the whole application and give a bad experience to the users.
Please consider using the SwingWorker API
http://docs.oracle.com/javase/tutorial/uiswing/concurrency/worker.html
However, you should still call all Swing APIs (including creating
your components/windows) on the EDT. And I don't see this is being
done in your current code. As a minimum, the displayToolWindow()
method should be dispatched on the event thread. I haven't examined
the code closely, but if there are any other GUI updates from
separate threads, those should also be moved to the EDT. See the
following tutorial for details:
http://docs.oracle.com/javase/tutorial/uiswing/concurrency/dispatch.html
Indeed, rewriting an AWT app as a Swing app is not only about
changing Labels to JLabels
Please make sure that all Swing code are used on the event
dispatching thread only.
Thanks
alexp
--
best regards,
Anthony
On 10/15/2013 06:13 PM, Weijun Wang wrote:
Policy Tool is a GUI editor for the plain text policy file. The
only I/O
is loading the policy file from disk and it should be quite small,
so I
think there won't be a problem here.
Thanks
Max
On 10/15/13 8:08 PM, Anthony Petrov wrote:
Hi Max,
I don't have expertise in this code so I haven't reviewed the fix
thoroughly. I'd like to point out one thing though: unlike AWT,
Swing is
a single-threaded GUI toolkit. While in AWT you can create
components/windows and call APIs on any thread, in Swing everything
GUI-related must be performed on the Event Dispatch Thread (EDT)
only.
Any long, non-GUI-related operations (like I/O, computations, etc.)
should be dispatched on other threads (with a SwingWorker, for
example).
I don't see a single SwingUtilities.invokeLater/invokeAndWait()
call in
PolicyTool.java, so I thought I'd ask whether you're aware of the
threading limitations imposed by Swing and how those are going to be
addressed?
--
best regards,
Anthony
On 10/12/2013 04:49 AM, Weijun Wang wrote:
Hi All
Please review the fix at
http://cr.openjdk.java.net/~weijun/7025699/webrev.00/
The fix includes porting PolicyTool from AWT to Swing, defining
mnemonics for menu items and buttons, and adding keyboard
shortcuts for
the File -> New/Open/Save items. Several tests are updated also.
Thanks
Max
