/hg/icedtea-web: Reduced usage of weak references in PolicyEditor

aazores at icedtea.classpath.org aazores at icedtea.classpath.org
Thu Jun 12 20:49:08 UTC 2014


changeset dd5a97f2b37f in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=dd5a97f2b37f
author: Andrew Azores <aazores at redhat.com>
date: Thu Jun 12 16:48:55 2014 -0400

	Reduced usage of weak references in PolicyEditor

	2014-06-12  Andrew Azores  <aazores at redhat.com>

	    Reduced usage of weak references in PolicyEditor
	    * netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java
	    (weakThis): removed in favour of CustomPolicyViewer.this
	    * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
	    (weakThis): used only in showChangesSavedDialog and showCouldNotSaveDialog,
	    other uses changed to PolicyEditor.this


diffstat:

 ChangeLog                                                               |   9 +
 netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java |   5 +-
 netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java       |  51 ++++++---
 3 files changed, 42 insertions(+), 23 deletions(-)

diffs (237 lines):

diff -r 48debbfe8672 -r dd5a97f2b37f ChangeLog
--- a/ChangeLog	Wed Jun 11 15:59:54 2014 -0400
+++ b/ChangeLog	Thu Jun 12 16:48:55 2014 -0400
@@ -1,3 +1,12 @@
+2014-06-12  Andrew Azores  <aazores at redhat.com>
+
+	Reduced usage of weak references in PolicyEditor
+	* netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java
+	(weakThis): removed in favour of CustomPolicyViewer.this
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
+	(weakThis): used only in showChangesSavedDialog and showCouldNotSaveDialog,
+	other uses changed to PolicyEditor.this
+
 2014-06-11  Andrew Azores  <aazores at redhat.com>
 
 	CustomPolicyViewer refactored - methods extracted for unit testing and no
diff -r 48debbfe8672 -r dd5a97f2b37f netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java
--- a/netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java	Wed Jun 11 15:59:54 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java	Thu Jun 12 16:48:55 2014 -0400
@@ -44,7 +44,6 @@
 import java.awt.event.ActionListener;
 import java.awt.event.WindowAdapter;
 import java.awt.event.WindowEvent;
-import java.lang.ref.WeakReference;
 import java.util.Collection;
 import java.util.Objects;
 import java.util.TreeSet;
@@ -75,7 +74,6 @@
     private final JButton addButton = new JButton(), removeButton = new JButton(), closeButton = new JButton();
     private final JLabel listLabel = new JLabel();
     private final ActionListener addButtonAction, removeButtonAction, closeButtonAction;
-    private final WeakReference<CustomPolicyViewer> weakThis = new WeakReference<>(this);
     private final PolicyEditor parent;
     private final String codebase;
 
@@ -101,7 +99,7 @@
             @Override
             public void actionPerformed(final ActionEvent e) {
                 final String prefill = R("PECPType") + " " + R("PECPTarget") + " [" + R("PECPActions") + "]";
-                String string = JOptionPane.showInputDialog(weakThis.get(), R("PECPPrompt"), prefill);
+                String string = JOptionPane.showInputDialog(CustomPolicyViewer.this, R("PECPPrompt"), prefill);
                 if (string == null || string.isEmpty()) {
                     return;
                 }
@@ -210,7 +208,6 @@
     }
 
     private void quit() {
-        weakThis.clear();
         parent.customPolicyViewerClosing();
         dispose();
     }
diff -r 48debbfe8672 -r dd5a97f2b37f netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Wed Jun 11 15:59:54 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Thu Jun 12 16:48:55 2014 -0400
@@ -173,7 +173,14 @@
             addCodebaseButton = new JButton(), removeCodebaseButton = new JButton();
     private final JFileChooser fileChooser;
     private CustomPolicyViewer cpViewer = null;
-    private final WeakReference<PolicyEditor> weakThis = new WeakReference<>(this);
+
+    /**
+     * See showChangesSavedDialog/showCouldNotSaveDialog. This weak reference is needed because
+     * there is a modal child dialog which can sometimes appear after the editor has been closed
+     * and disposed. In this case, its parent should be set to 'null', but otherwise the parent
+     * should be the editor so that the dialog is modal.
+     */
+    private final WeakReference<PolicyEditor> parentPolicyEditor = new WeakReference<>(this);
     private Map<PolicyEditorPermissions, Boolean> editorPermissionsClipboard = null;
     private Set<CustomPermission> customPermissionsClipboard = null;
 
@@ -247,7 +254,7 @@
             @Override
             public void actionPerformed(final ActionEvent event) {
                 if (policyFile.getFile() == null) {
-                    final int choice = fileChooser.showOpenDialog(weakThis.get());
+                    final int choice = fileChooser.showOpenDialog(PolicyEditor.this);
                     if (choice == JFileChooser.APPROVE_OPTION) {
                         policyFile.setFile(fileChooser.getSelectedFile());
                     }
@@ -284,10 +291,10 @@
             @Override
             public void actionPerformed(final ActionEvent e) {
                 if (changesMade) {
-                    final int save = JOptionPane.showConfirmDialog(weakThis.get(), R("PESaveChanges"));
+                    final int save = JOptionPane.showConfirmDialog(PolicyEditor.this, R("PESaveChanges"));
                     if (save == JOptionPane.YES_OPTION) {
                         if (policyFile.getFile() == null) {
-                            final int choice = fileChooser.showSaveDialog(weakThis.get());
+                            final int choice = fileChooser.showSaveDialog(PolicyEditor.this);
                             if (choice == JFileChooser.APPROVE_OPTION) {
                                 policyFile.setFile(fileChooser.getSelectedFile());
                             } else if (choice == JFileChooser.CANCEL_OPTION) {
@@ -299,7 +306,7 @@
                         return;
                     }
                 }
-                final int choice = fileChooser.showOpenDialog(weakThis.get());
+                final int choice = fileChooser.showOpenDialog(PolicyEditor.this);
                 if (choice == JFileChooser.APPROVE_OPTION) {
                     policyFile.setFile(fileChooser.getSelectedFile());
                     openAndParsePolicyFile();
@@ -310,7 +317,7 @@
         saveAsButtonAction = new ActionListener() {
             @Override
             public void actionPerformed(final ActionEvent e) {
-                final int choice = fileChooser.showSaveDialog(weakThis.get());
+                final int choice = fileChooser.showSaveDialog(PolicyEditor.this);
                 if (choice == JFileChooser.APPROVE_OPTION) {
                     policyFile.setFile(fileChooser.getSelectedFile());
                     changesMade = true;
@@ -328,7 +335,7 @@
                 }
                 String newCodebase = "";
                 while (!validateCodebase(newCodebase) || policyFile.getCopyOfPermissions().containsKey(newCodebase)) {
-                    newCodebase = JOptionPane.showInputDialog(weakThis.get(), R("PERenameCodebase"), oldCodebase);
+                    newCodebase = JOptionPane.showInputDialog(PolicyEditor.this, R("PERenameCodebase"), oldCodebase);
                     if (newCodebase == null) {
                         return;
                     }
@@ -349,7 +356,7 @@
             public void actionPerformed(final ActionEvent e) {
                 String newCodebase = "";
                 while (!validateCodebase(newCodebase) || policyFile.getCopyOfPermissions().containsKey(newCodebase)) {
-                    newCodebase = JOptionPane.showInputDialog(weakThis.get(), R("PEPasteCodebase"), "http://");
+                    newCodebase = JOptionPane.showInputDialog(PolicyEditor.this, R("PEPasteCodebase"), "http://");
                     if (newCodebase == null) {
                         return;
                     }
@@ -540,6 +547,7 @@
      */
     private static void policyEditorWindowQuit(final Window window) {
         final PolicyEditor editor = ((PolicyEditorWindow) window).getPolicyEditor();
+        editor.parentPolicyEditor.clear();
         if (editor.changesMade) {
             final int save = JOptionPane.showConfirmDialog(window, R("PESaveChanges"));
             if (save == JOptionPane.YES_OPTION) {
@@ -556,7 +564,6 @@
                 return;
             }
         }
-        editor.weakThis.clear();
         editor.setClosed();
         window.dispose();
     }
@@ -780,7 +787,7 @@
             public void run() {
                 String codebase = "";
                 while (!validateCodebase(codebase)) {
-                    codebase = JOptionPane.showInputDialog(weakThis.get(), R("PECodebasePrompt"), "http://");
+                    codebase = JOptionPane.showInputDialog(PolicyEditor.this, R("PECodebasePrompt"), "http://");
                     if (codebase == null) {
                         return;
                     }
@@ -1249,11 +1256,11 @@
         }
         final OpenFileResult ofr = FileUtils.testFilePermissions(policyFile.getFile());
         if (ofr == OpenFileResult.FAILURE || ofr == OpenFileResult.NOT_FILE) {
-            FileUtils.showCouldNotOpenFilepathDialog(weakThis.get(), policyFile.getFile().getPath());
+            FileUtils.showCouldNotOpenFilepathDialog(PolicyEditor.this, policyFile.getFile().getPath());
             return;
         }
         if (ofr == OpenFileResult.CANT_WRITE) {
-            FileUtils.showReadOnlyDialog(weakThis.get());
+            FileUtils.showReadOnlyDialog(PolicyEditor.this);
         }
 
         final Window parentWindow = SwingUtilities.getWindowAncestor(this);
@@ -1275,11 +1282,11 @@
                     policyFile.openAndParsePolicyFile();
                 } catch (final FileNotFoundException fnfe) {
                     OutputController.getLogger().log(fnfe);
-                    FileUtils.showCouldNotOpenDialog(weakThis.get(), R("PECouldNotOpen"));
+                    FileUtils.showCouldNotOpenDialog(PolicyEditor.this, R("PECouldNotOpen"));
                 } catch (final IOException ioe) {
                     OutputController.getLogger().log(ioe);
                     OutputController.getLogger().log(OutputController.Level.ERROR_ALL, R("RCantOpenFile", policyFile.getFile().getPath()));
-                    FileUtils.showCouldNotOpenDialog(weakThis.get(), R("PECouldNotOpen"));
+                    FileUtils.showCouldNotOpenDialog(PolicyEditor.this, R("PECouldNotOpen"));
                 }
                 return null;
             }
@@ -1362,10 +1369,13 @@
      * Show a dialog informing the user that their changes have been saved.
      */
     private void showChangesSavedDialog() {
+        // This dialog is often displayed when closing the editor, and so PolicyEditor
+        // may already be disposed when this dialog appears. Give a weak reference so
+        // that this dialog doesn't prevent GC of the editor
         SwingUtilities.invokeLater(new Runnable() {
             @Override
             public void run() {
-                JOptionPane.showMessageDialog(weakThis.get(), R("PEChangesSaved"));
+                JOptionPane.showMessageDialog(parentPolicyEditor.get(), R("PEChangesSaved"));
             }
         });
     }
@@ -1374,10 +1384,13 @@
      * Show a dialog informing the user that their changes could not be saved.
      */
     private void showCouldNotSaveDialog() {
+        // This dialog is often displayed when closing the editor, and so PolicyEditor
+        // may already be disposed when this dialog appears. Give a weak reference so
+        // that this dialog doesn't prevent GC of the editor
         SwingUtilities.invokeLater(new Runnable() {
             @Override
             public void run() {
-                JOptionPane.showMessageDialog(weakThis.get(), R("PECouldNotSave"), R("Error"), JOptionPane.ERROR_MESSAGE);
+                JOptionPane.showMessageDialog(parentPolicyEditor.get(), R("PECouldNotSave"), R("Error"), JOptionPane.ERROR_MESSAGE);
             }
         });
     }
@@ -1396,7 +1409,7 @@
             changed = policyFile.hasChanged();
         } catch (FileNotFoundException e) {
             OutputController.getLogger().log(e);
-            JOptionPane.showMessageDialog(weakThis.get(), R("PEFileMissing"), R("PEFileModified"), JOptionPane.WARNING_MESSAGE);
+            JOptionPane.showMessageDialog(PolicyEditor.this, R("PEFileMissing"), R("PEFileModified"), JOptionPane.WARNING_MESSAGE);
             return JOptionPane.NO_OPTION;
         } catch (IOException e) {
             OutputController.getLogger().log(e);
@@ -1410,7 +1423,7 @@
                 OutputController.getLogger().log(e);
                 policyFilePath = policyFile.getFile().getPath();
             }
-            return JOptionPane.showConfirmDialog(weakThis.get(), R("PEFileModifiedDetail", policyFilePath,
+            return JOptionPane.showConfirmDialog(PolicyEditor.this, R("PEFileModifiedDetail", policyFilePath,
                     R("PEFileModified"), JOptionPane.YES_NO_CANCEL_OPTION));
         } else if (!changesMade) {
             //Return without saving or reloading
@@ -1505,4 +1518,4 @@
         return map;
     }
 
-}
+}
\ No newline at end of file


More information about the distro-pkg-dev mailing list