[RFC][IcedTea-Web]: Moved creation of swing thread.

[Omair Majid]

On 12/17/2010 02:47 PM, Andrew Su wrote:
> Hello,
>
> I noticed a minor issue with updating the fields when using the
> advanced proxy dialog, fields in the original network panel was not
> being updated when closing the dialog box. Cause: dialog ran in a new
> thread, code for updating finished before finishing dialog.
>
> The attached patch fixes this issue by moving the swing threading out
> to where the call happens.
>
> Questions? Comments? Concerns?
> Cheers,
>    Andrew
>
>
> 20101217_moved_swing_threading.patch
>
>
> diff -r 7d25178e476d ChangeLog
> --- a/ChangeLog	Fri Dec 17 14:32:17 2010 -0500
> +++ b/ChangeLog	Fri Dec 17 14:44:37 2010 -0500
> @@ -1,3 +1,11 @@
> +2010-12-17  Andrew Su<asu at redhat.com>
> +
> +	* netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsDialog.java:
> +	(showAdvancedProxySettingsDialog): Removed creation of swing thread.
> +	* netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java:
> +	(addComponents): Added creation of swing thread for opening proxy
> +	dialog.
> +
>   2010-12-17  Andrew Su<asu at redhat.com>
>
>   	* netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsPane.java
> diff -r 7d25178e476d netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsDialog.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsDialog.java	Fri Dec 17 14:32:17 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/controlpanel/AdvancedProxySettingsDialog.java	Fri Dec 17 14:44:37 2010 -0500
> @@ -120,16 +120,12 @@
>        */
>       public static void showAdvancedProxySettingsDialog(final DeploymentConfiguration config) throws Exception {
>           setSystemLookAndFeel();

As I said before, please remove these redundant calls to 
UIManager.setLookAndFeel().

> -        SwingUtilities.invokeLater(new Runnable() {
> -            @Override
> -            public void run() {
> -                AdvancedProxySettingsDialog psd = new AdvancedProxySettingsDialog(config);
> -                psd.setResizable(false);
> -                psd.centerDialog();
> -                psd.setVisible(true);
> -                psd.dispose();
> -            }
> -        });
> +
> +        AdvancedProxySettingsDialog psd = new AdvancedProxySettingsDialog(config);
> +        psd.setResizable(false);
> +        psd.centerDialog();
> +        psd.setVisible(true);
> +        psd.dispose();
>

This is supposed to be a modal dialog, right? In that case, you may want 
to have an additional argument for the parent window/dialog, and create 
this dialog as modal (see JDialog.setModalityType) so that it blocks the 
parent. This should fix the bug.

>       }
>
> diff -r 7d25178e476d netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java
> --- a/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Fri Dec 17 14:32:17 2010 -0500
> +++ b/netx/net/sourceforge/jnlp/controlpanel/NetworkSettingsPanel.java	Fri Dec 17 14:44:37 2010 -0500
> @@ -42,6 +42,7 @@
>   import javax.swing.JPanel;
>   import javax.swing.JRadioButton;
>   import javax.swing.JTextField;
> +import javax.swing.SwingUtilities;
>
>   import net.sourceforge.jnlp.config.DeploymentConfiguration;
>   import net.sourceforge.jnlp.runtime.Translator;
> @@ -115,22 +116,27 @@
>           JLabel portLabel = new JLabel(Translator.R("NSPort") + ":");
>           final JTextField addressField = new JTextField(config.getProperty(properties[1]), 10);
>           addressField.getDocument().addDocumentListener(new DocumentAdapter(config, properties[1]));
> -
> +
>           final JTextField portField = new JTextField(config.getProperty(properties[2]), 3);
>           portField.getDocument().addDocumentListener(new DocumentAdapter(config, properties[1]));
> -
> +
>           // Create the button which allows setting of other types of proxy.
>           JButton advancedProxyButton = new JButton(Translator.R("NSAdvanced") + "...");
>           advancedProxyButton.addActionListener(new ActionListener() {
>               @Override
>               public void actionPerformed(ActionEvent e) {
> -                try {
> -                    AdvancedProxySettingsDialog.showAdvancedProxySettingsDialog(config);
> -                    addressField.setText(config.getProperty(properties[1]));
> -                    portField.setText(config.getProperty(properties[2]));
> -                } catch (Exception e1) {
> -                    e1.printStackTrace();
> -                }
> +                SwingUtilities.invokeLater(new Runnable() {
> +                    @Override
> +                    public void run() {
> +                        try {
> +                            AdvancedProxySettingsDialog.showAdvancedProxySettingsDialog(config);
> +                            addressField.setText(config.getProperty(properties[1]));
> +                            portField.setText(config.getProperty(properties[2]));
> +                        } catch (Exception e1) {
> +                            e1.printStackTrace();
> +                        }
> +                    }
> +                });
>               }
>           });
>

AFAIK, listeners are run in the swing event dispatch thread. There 
should be no need to use SwingUtilities.invokeLater() here.

Cheers,
Omair