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
