[rfc][icedtea-web] Cache viewer cleanup

Jakob Wisor gitne at gmx.de
Sun Sep 15 17:17:43 PDT 2013


Hello there!

I have checked and corrected the code's formatting, and followed Jiri's 
reasonable advice.

Jiri Vanek wrote:
> On 07/31/2013 01:47 AM, Jacob Wisor wrote:
>> Hello,
>>
>> * Added closing of cache viewer by ESC key
>> * Added proper dis-/enabling of cache viewer's buttons
>> * Added busy mouse cursor indicator when populating the cache viewer
>>
>> Everything is done on purpose on the AWT thread as before. Although I am personally not in favor of anonymous classes, I have tried to stick to the prevalent coding style.
>>
>> Unfortunately, I was unable to test the cache viewer's behavior because I do not know how to put synthetic cache entries into the cache. Can anyone help me with that? What does a the cache look like when filled with resources? Is there any index file that keeps track of the cache?
>>
>> Regards,
>> Jacob
>>
>>
>> ESC closing of Cache viewer + cleanup.patch
>>
>>
>> diff --git a/netx/net/sourceforge/jnlp/controlpanel/CachePane.java b/netx/net/sourceforge/jnlp/controlpanel/CachePane.java
>> --- a/netx/net/sourceforge/jnlp/controlpanel/CachePane.java
>> +++ b/netx/net/sourceforge/jnlp/controlpanel/CachePane.java
>> @@ -19,13 +19,18 @@
>>   
>>   import java.awt.BorderLayout;
>>   import java.awt.Component;
>> +import java.awt.Cursor;
>>   import java.awt.Dimension;
>> +import java.awt.EventQueue;
>>   import java.awt.FlowLayout;
>>   import java.awt.GridBagConstraints;
>>   import java.awt.GridBagLayout;
>>   import java.awt.GridLayout;
>> +import java.awt.SystemColor;
>> +import java.awt.Toolkit;
>>   import java.awt.event.ActionEvent;
>>   import java.awt.event.ActionListener;
>> +import java.awt.event.WindowEvent;
>>   import java.io.File;
>>   import java.io.FileNotFoundException;
>>   import java.io.IOException;
>> @@ -45,6 +50,8 @@
>>   import javax.swing.JScrollPane;
>>   import javax.swing.JTable;
>>   import javax.swing.ListSelectionModel;
>> +import javax.swing.event.ListSelectionEvent;
>> +import javax.swing.event.ListSelectionListener;
>>   import javax.swing.table.DefaultTableModel;
>>   import javax.swing.table.TableRowSorter;
>>   
>> @@ -57,19 +64,20 @@
>>   import net.sourceforge.jnlp.util.PropertiesFile;
>>   
>>   public class CachePane extends JPanel {
>> -
>>       JDialog parent;
>>       DeploymentConfiguration config;
>>       private String location;
>>       private JComponent defaultFocusComponent;
>>       DirectoryNode root;
>> -    String[] columns = { Translator.R("CVCPColName"),
>> +    String[] columns = {
>> +	        Translator.R("CVCPColName"),
>>               Translator.R("CVCPColPath"),
>>               Translator.R("CVCPColType"),
>>               Translator.R("CVCPColDomain"),
>>               Translator.R("CVCPColSize"),
>>               Translator.R("CVCPColLastModified") };
>>       JTable cacheTable;
>> +	private JButton deleteButton, refreshButton, doneButton;
>>   
>>       /**
>>        * Creates a new instance of the CachePane.
>> @@ -102,13 +110,22 @@
>>   
>>           cacheTable = new JTable(model);
>>           cacheTable.getSelectionModel().setSelectionMode(ListSelectionModel.SINGLE_SELECTION);
>> +		cacheTable.getSelectionModel().addListSelectionListener(new ListSelectionListener() {
>> +            final public void valueChanged(ListSelectionEvent listSelectionEvent) {
>> +                // If no row has been selected, disable the delete button, else enable it
>> +                if (cacheTable.getSelectionModel().isSelectionEmpty())
>> +                    // Disable delete button, since nothing selected
>> +                    deleteButton.setEnabled(false);
>> +                else
>> +                    // Enable delete button, since something selected
>> +                    deleteButton.setEnabled(true);
>> +            }
>> +        });
> 
> 
> Oh yes, this small enhancement is highly appreciated!
> 
>>           cacheTable.setAutoResizeMode(JTable.AUTO_RESIZE_NEXT_COLUMN);
>>           cacheTable.setPreferredScrollableViewportSize(new Dimension(600, 200));
>>           cacheTable.setFillsViewportHeight(true);
>>           JScrollPane scrollPane = new JScrollPane(cacheTable);
>>   
>> -        populateTable();
>> -
>>           TableRowSorter<DefaultTableModel> tableSorter = new TableRowSorter<DefaultTableModel>(model);
>>           tableSorter.setComparator(4, new Comparator<Long>() { // Comparator for size column.
>>               @Override
>> @@ -138,7 +155,6 @@
>>           topPanel.add(scrollPane, c);
>>           this.add(topPanel, BorderLayout.CENTER);
>>           this.add(createButtonPanel(), BorderLayout.SOUTH);
>> -
>>       }
>>   
>>       /**
>> @@ -153,7 +169,7 @@
>>   
>>           List<JButton> buttons = new ArrayList<JButton>();
>>   
>> -        JButton deleteButton = new JButton(Translator.R("CVCPButDelete"));
>> +        this.deleteButton = new JButton(Translator.R("CVCPButDelete"));
>>           deleteButton.addActionListener(new ActionListener() {
>>               @Override
>>               public void actionPerformed(ActionEvent e) {
>> @@ -216,22 +232,29 @@
>>                   pf.store();
>>               }
>>           });
>> +		deleteButton.setEnabled(false);
>>           buttons.add(deleteButton);
>>   
>> -        JButton refreshButton = new JButton(Translator.R("CVCPButRefresh"));
>> +        this.refreshButton = new JButton(Translator.R("CVCPButRefresh"));
>>           refreshButton.addActionListener(new ActionListener() {
>>               @Override
>>               public void actionPerformed(ActionEvent e) {
>> -                populateTable();
>> +                // Disable all its controls when performing cacheTable refresh (populating)
>> +                deleteButton.setEnabled(false);
>> +                refreshButton.setEnabled(false);
>> +                doneButton.setEnabled(false);
>> +                // Populate cacheTable on AWT thread after this action event has been handled
>> +				invokeLaterPopulateTable();
>>               }
>>           });
>> +		refreshButton.setEnabled(false);
>>           buttons.add(refreshButton);
>>   
>> -        JButton doneButton = new JButton(Translator.R("ButDone"));
>> +        this.doneButton = new JButton(Translator.R("ButDone"));
>>           doneButton.addActionListener(new ActionListener() {
>>               @Override
>>               public void actionPerformed(ActionEvent e) {
>> -                parent.dispose();
>> +                Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent(new WindowEvent(parent, WindowEvent.WINDOW_CLOSING));
>>               }
>>           });
>>   
>> @@ -250,6 +273,7 @@
>>           }
>>   
>>           doneButton.setPreferredSize(new Dimension(wantedWidth, wantedHeight));
>> +		doneButton.setEnabled(false);
>>           rightPanel.add(doneButton);
>>           buttonPanel.add(leftPanel);
>>           buttonPanel.add(rightPanel);
>> @@ -257,14 +281,48 @@
>>           return buttonPanel;
>>       }
>>   
>> +	/**
>> +	 * Posts an event to the event queue to later invoke populating the
>> +	 * {@link CachePane#cacheTable}.
>> +	 * @see CachePane#populateTable
>> +	 */
>> +	final void invokeLaterPopulateTable() {
>> +		EventQueue.invokeLater(new Runnable() {
>> +			public void run() {
>> +				populateTable();
>> +
>> +				// Disable cacheTable when no data to display, so no events are generated
>> +				if (cacheTable.getModel().getRowCount() == 0) {
>> +					cacheTable.setEnabled(false);
>> +					cacheTable.setBackground(SystemColor.control);
>> +					// No data in cacheTable, so nothing to delete
>> +					deleteButton.setEnabled(false);
>> +				} else {
>> +					cacheTable.setEnabled(true);
>> +					cacheTable.setBackground(SystemColor.window);
>> +				}
>> +
> 
> Those should be in finally block
>> +				refreshButton.setEnabled(true);
>> +				doneButton.setEnabled(true);

Done. Please check whether I have done it correctly or the way you have meant.

>> +			}
>> +		});
>> +	}
>> +
>>       /**
>>        * Populate the table with fresh data. Any manual updates to the cache
>>        * directory will be updated in the table.
>>        */
>>       private void populateTable() {
>> -        ((DefaultTableModel) cacheTable.getModel()).setRowCount(0); //Clears the table
>> +        // Populating the cacheTable may take a while, so indicate busy by cursor
>> +        parent.getContentPane().setCursor(Cursor.getPredefinedCursor(Cursor.WAIT_CURSOR));
>> +
>> +        DefaultTableModel defaultTableModel;
>> +        (defaultTableModel = (DefaultTableModel)cacheTable.getModel()).setRowCount(0); //Clears the table
>>           for (Object[] v : generateData(root))
>> -            ((DefaultTableModel) cacheTable.getModel()).addRow(v);
>> +            defaultTableModel.addRow(v);
>> +
>> +        // Reset cursor
> Those should be in finally block
>> +        parent.getContentPane().setCursor(Cursor.getDefaultCursor());

Done. Please check whether I have done it correctly or the way you have meant.

>>       }
>>   
>>       /**
>> @@ -305,4 +363,4 @@
>>               defaultFocusComponent.requestFocusInWindow();
>>           }
>>       }
>> -}
>> +}
>> \ No newline at end of file
>> diff --git a/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java b/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>> --- a/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>> +++ b/netx/net/sourceforge/jnlp/controlpanel/CacheViewer.java
>> @@ -22,7 +22,10 @@
>>   import java.awt.Frame;
>>   import java.awt.GridBagConstraints;
>>   import java.awt.GridBagLayout;
>> +import java.awt.KeyEventDispatcher;
>> +import java.awt.KeyboardFocusManager;
>>   import java.awt.Toolkit;
>> +import java.awt.event.KeyEvent;
>>   import java.awt.event.WindowAdapter;
>>   import java.awt.event.WindowEvent;
>>   
>> @@ -53,8 +56,9 @@
>>        */
>>       public CacheViewer(DeploymentConfiguration config) {
>>           super((Frame) null, dialogTitle, true); // Don't need a parent.
>> +        if ((this.config = config) == null)
>> +		    throw new IllegalArgumentException("config: " + config);
>>           setIconImages(ImageResources.INSTANCE.getApplicationImages());
>> -        this.config = config;
>>   
>>           /* Prepare for adding components to dialog box */
>>           Container contentPane = getContentPane();
>> @@ -70,11 +74,13 @@
>>           contentPane.add(topPanel, c);
>>   
>>           pack();
>> +		this.topPanel.invokeLaterPopulateTable();
>>   
>>           /* Set focus to default button when first activated */
>>           WindowAdapter adapter = new WindowAdapter() {
>>               private boolean gotFocus = false;
>>   
>> +            @Override
>>               public void windowGainedFocus(WindowEvent we) {
>>                   // Once window gets focus, set initial focus
>>                   if (!gotFocus) {
>> @@ -85,6 +91,29 @@
>>           };
>>           addWindowFocusListener(adapter);
>>   
>> +        // Add a KeyEventDispatcher to dispatch events when this CacheViewer has focus
>> +		final CacheViewer cacheViewer = this;
>> +        KeyboardFocusManager.getCurrentKeyboardFocusManager().addKeyEventDispatcher(new KeyEventDispatcher() {
>> +			/**
>> +			 * Dispatches mainly the <code>VK_ESCAPE</code> key event to close the
>> +			 * <code>CacheViewer</code> dialog.
>> +			 * @see java.awt.KeyEventDispatcher
>> +			 */
>> +			public boolean dispatchKeyEvent(final KeyEvent keyEvent) {
>> +				// Check if Esc key has been pressed
>> +				if (keyEvent.getKeyCode() == KeyEvent.VK_ESCAPE && keyEvent.getID() == KeyEvent.KEY_PRESSED) {
>> +					// Exclude this key event from further processing
>> +					keyEvent.consume();
>> +					// Remove this low-level KeyEventDispatcher
>> +					KeyboardFocusManager.getCurrentKeyboardFocusManager().removeKeyEventDispatcher(this);
>> +					// Post close event to CacheViewer dialog
>> +					Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent(new WindowEvent(cacheViewer, WindowEvent.WINDOW_CLOSING));
>> +					return true;
>> +				}
>> +				return false;
>> +			}
>> +		});
>> +
>>           initialized = true;
>>       }
>>   
>> @@ -116,4 +145,4 @@
>>       private void centerDialog() {
>>           ScreenFinder.centerWindowsToCurrentScreen(this);
>>       }
>> -}
>> +}
>> \ No newline at end of file
> 
> Please mention both Cache viewer cleanup and  CerViewr viewer cleanup in news.

Will do but they are separate patches.

Happy reviewing!
Jacob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ESC closing of Cache viewer + cleanup.patch
Type: text/x-patch
Size: 11552 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130916/6549ae96/ESCclosingofCacheviewercleanup.patch 


More information about the distro-pkg-dev mailing list