From 812bf26b3c5b2182947f4dd059914281321e47c9 Mon Sep 17 00:00:00 2001 From: Mads Date: Thu, 13 Mar 2014 20:18:35 +0100 Subject: [PATCH] Fixed UI exceptions caused by modifications to the UI outside the event thread. --- .../ripme/ripper/AbstractRipper.java | 72 ++++----- .../com/rarchives/ripme/ui/MainWindow.java | 153 +++++++++--------- .../rarchives/ripme/ui/RipStatusHandler.java | 14 ++ 3 files changed, 127 insertions(+), 112 deletions(-) create mode 100644 src/main/java/com/rarchives/ripme/ui/RipStatusHandler.java diff --git a/src/main/java/com/rarchives/ripme/ripper/AbstractRipper.java b/src/main/java/com/rarchives/ripme/ripper/AbstractRipper.java index 952f8420..cb737410 100644 --- a/src/main/java/com/rarchives/ripme/ripper/AbstractRipper.java +++ b/src/main/java/com/rarchives/ripme/ripper/AbstractRipper.java @@ -1,5 +1,7 @@ package com.rarchives.ripme.ripper; +import com.rarchives.ripme.ui.MainWindow; +import com.rarchives.ripme.ui.RipStatusHandler; import java.io.File; import java.io.IOException; import java.lang.reflect.Constructor; @@ -17,6 +19,7 @@ import org.apache.log4j.Logger; import com.rarchives.ripme.ui.RipStatusMessage; import com.rarchives.ripme.ui.RipStatusMessage.STATUS; import com.rarchives.ripme.utils.Utils; +import java.util.Collections; public abstract class AbstractRipper extends Observable @@ -30,11 +33,11 @@ public abstract class AbstractRipper protected URL url; protected File workingDir; protected DownloadThreadPool threadPool; - protected Observer observer = null; + protected RipStatusHandler observer = null; - protected Map itemsPending = new HashMap(); - protected Map itemsCompleted = new HashMap(); - protected Map itemsErrored = new HashMap(); + protected Map itemsPending = Collections.synchronizedMap(new HashMap()); + protected Map itemsCompleted = Collections.synchronizedMap(new HashMap()); + protected Map itemsErrored = Collections.synchronizedMap(new HashMap()); protected boolean completed = true; public abstract void rip() throws IOException; @@ -59,7 +62,7 @@ public abstract class AbstractRipper this.threadPool = new DownloadThreadPool(); } - public void setObserver(Observer obs) { + public void setObserver(RipStatusHandler obs) { this.observer = obs; } @@ -162,7 +165,6 @@ public abstract class AbstractRipper public void retrievingSource(URL url) { RipStatusMessage msg = new RipStatusMessage(STATUS.LOADING_RESOURCE, url); observer.update(this, msg); - observer.notifyAll(); } /** @@ -179,13 +181,11 @@ public abstract class AbstractRipper try { String path = Utils.removeCWD(saveAs); RipStatusMessage msg = new RipStatusMessage(STATUS.DOWNLOAD_COMPLETE, path); - synchronized(observer) { - itemsPending.remove(url); - itemsCompleted.put(url, saveAs); - observer.update(this, msg); - observer.notifyAll(); - checkIfComplete(); - } + itemsPending.remove(url); + itemsCompleted.put(url, saveAs); + observer.update(this, msg); + + checkIfComplete(); } catch (Exception e) { logger.error("Exception while updating observer: ", e); } @@ -200,13 +200,11 @@ public abstract class AbstractRipper if (observer == null) { return; } - synchronized(observer) { - itemsPending.remove(url); - itemsErrored.put(url, reason); - observer.update(this, new RipStatusMessage(STATUS.DOWNLOAD_ERRORED, url + " : " + reason)); - observer.notifyAll(); - checkIfComplete(); - } + itemsPending.remove(url); + itemsErrored.put(url, reason); + observer.update(this, new RipStatusMessage(STATUS.DOWNLOAD_ERRORED, url + " : " + reason)); + + checkIfComplete(); } /** @@ -219,12 +217,12 @@ public abstract class AbstractRipper if (observer == null) { return; } - synchronized(observer) { - itemsPending.remove(url); - itemsErrored.put(url, message); - observer.update(this, new RipStatusMessage(STATUS.DOWNLOAD_WARN, url + " : " + message)); - observer.notifyAll(); - } + + itemsPending.remove(url); + itemsErrored.put(url, message); + observer.update(this, new RipStatusMessage(STATUS.DOWNLOAD_WARN, url + " : " + message)); + + checkIfComplete(); } @@ -235,16 +233,13 @@ public abstract class AbstractRipper if (observer == null) { return; } - synchronized (observer) { - if (!completed && itemsPending.size() == 0) { - completed = true; - logger.info(" Rip completed!"); - observer.update(this, - new RipStatusMessage( - STATUS.RIP_COMPLETE, - workingDir)); - observer.notifyAll(); - } + + if (!completed && itemsPending.isEmpty()) { + completed = true; + logger.info(" Rip completed!"); + + RipStatusMessage msg = new RipStatusMessage(STATUS.RIP_COMPLETE, workingDir); + observer.update(this, msg); } } @@ -325,10 +320,7 @@ public abstract class AbstractRipper if (observer == null) { return; } - synchronized (observer) { - observer.update(this, new RipStatusMessage(status, message)); - observer.notifyAll(); - } + observer.update(this, new RipStatusMessage(status, message)); } /** diff --git a/src/main/java/com/rarchives/ripme/ui/MainWindow.java b/src/main/java/com/rarchives/ripme/ui/MainWindow.java index 756d86f5..43edc92e 100644 --- a/src/main/java/com/rarchives/ripme/ui/MainWindow.java +++ b/src/main/java/com/rarchives/ripme/ui/MainWindow.java @@ -46,7 +46,7 @@ import com.rarchives.ripme.utils.Utils; /** * Everything UI-related starts and ends here. */ -public class MainWindow implements Runnable { +public class MainWindow implements Runnable, RipStatusHandler { private static final Logger logger = Logger.getLogger(MainWindow.class); @@ -101,7 +101,7 @@ public class MainWindow implements Runnable { mainFrame.setVisible(true); } - public static void status(String text) { + public synchronized static void status(String text) { statusLabel.setText(text); mainFrame.pack(); } @@ -276,24 +276,14 @@ public class MainWindow implements Runnable { } private void appendLog(final String text, final Color color) { - try { - SwingUtilities.invokeAndWait(new Runnable() { - @Override - public void run() { - SimpleAttributeSet sas = new SimpleAttributeSet(); - StyleConstants.setForeground(sas, color); - StyledDocument sd = logText.getStyledDocument(); - try { - sd.insertString(sd.getLength(), text + "\n", sas); - } catch (BadLocationException e) { } - logText.setCaretPosition(logText.getText().length()); - } - }); - } catch (InterruptedException e) { - e.printStackTrace(); - } catch (InvocationTargetException e) { - e.printStackTrace(); - } + SimpleAttributeSet sas = new SimpleAttributeSet(); + StyleConstants.setForeground(sas, color); + StyledDocument sd = logText.getStyledDocument(); + try { + sd.insertString(sd.getLength(), text + "\n", sas); + } catch (BadLocationException e) { } + + logText.setCaretPosition(sd.getLength()); } private void loadHistory() { @@ -359,7 +349,7 @@ public class MainWindow implements Runnable { try { AbstractRipper ripper = AbstractRipper.getRipper(url); ripTextfield.setText(ripper.getURL().toExternalForm()); - ripper.setObserver(new RipStatusHandler()); + ripper.setObserver((RipStatusHandler) this); Thread t = new Thread(ripper); t.start(); return t; @@ -375,64 +365,83 @@ public class MainWindow implements Runnable { ripAlbum(ripTextfield.getText()); } } + + private class StatusEvent implements Runnable { + private final AbstractRipper ripper; + private final RipStatusMessage msg; - class RipStatusHandler implements Observer { - public void update(Observable observable, Object object) { - RipStatusMessage msg = (RipStatusMessage) object; - - int completedPercent = ((AbstractRipper) observable).getCompletionPercentage(); - statusProgress.setValue(completedPercent); - status( ((AbstractRipper)observable).getStatusText() ); - - switch(msg.getStatus()) { - case LOADING_RESOURCE: - case DOWNLOAD_STARTED: - appendLog( "Downloading: " + (String) msg.getObject(), Color.BLACK); - break; - case DOWNLOAD_COMPLETE: - appendLog( "Completed: " + (String) msg.getObject(), Color.GREEN); - break; - case DOWNLOAD_ERRORED: - appendLog( "Error: " + (String) msg.getObject(), Color.RED); - break; - - case DOWNLOAD_WARN: - appendLog( "Warn: " + (String) msg.getObject(), Color.ORANGE); - break; - - case RIP_COMPLETE: - if (!historyListModel.contains(ripTextfield.getText())) { - historyListModel.addElement(ripTextfield.getText()); - } - saveHistory(); - ripButton.setEnabled(true); - ripTextfield.setEnabled(true); - statusProgress.setValue(100); - statusLabel.setVisible(false); - openButton.setVisible(true); - File f = (File) msg.getObject(); - String prettyFile = Utils.removeCWD(f); - openButton.setText("Open " + prettyFile); - appendLog( "Rip complete, saved to " + prettyFile, Color.GREEN); - openButton.setActionCommand(f.toString()); - openButton.addActionListener(new ActionListener() { - @Override - public void actionPerformed(ActionEvent event) { - try { - Desktop.getDesktop().open(new File(event.getActionCommand())); - } catch (Exception e) { - logger.error(e); - } - } - }); - mainFrame.pack(); - } + public StatusEvent(AbstractRipper ripper, RipStatusMessage msg) { + this.ripper = ripper; + this.msg = msg; } + + public void run() { + handleEvent(this); + } + } + + private void handleEvent(StatusEvent evt) { + RipStatusMessage msg = evt.msg; + + int completedPercent = evt.ripper.getCompletionPercentage(); + statusProgress.setValue(completedPercent); + status( evt.ripper.getStatusText() ); + + switch(msg.getStatus()) { + case LOADING_RESOURCE: + case DOWNLOAD_STARTED: + appendLog( "Downloading: " + (String) msg.getObject(), Color.BLACK); + break; + case DOWNLOAD_COMPLETE: + appendLog( "Completed: " + (String) msg.getObject(), Color.GREEN); + break; + case DOWNLOAD_ERRORED: + appendLog( "Error: " + (String) msg.getObject(), Color.RED); + break; + + case DOWNLOAD_WARN: + appendLog( "Warn: " + (String) msg.getObject(), Color.ORANGE); + break; + + case RIP_COMPLETE: + if (!historyListModel.contains(ripTextfield.getText())) { + historyListModel.addElement(ripTextfield.getText()); + } + saveHistory(); + ripButton.setEnabled(true); + ripTextfield.setEnabled(true); + statusProgress.setValue(100); + statusLabel.setVisible(false); + openButton.setVisible(true); + File f = (File) msg.getObject(); + String prettyFile = Utils.removeCWD(f); + openButton.setText("Open " + prettyFile); + appendLog( "Rip complete, saved to " + prettyFile, Color.GREEN); + openButton.setActionCommand(f.toString()); + openButton.addActionListener(new ActionListener() { + @Override + public void actionPerformed(ActionEvent event) { + try { + Desktop.getDesktop().open(new File(event.getActionCommand())); + } catch (Exception e) { + logger.error(e); + } + } + }); + mainFrame.pack(); + } + } + + public void update(AbstractRipper ripper, RipStatusMessage message) { + StatusEvent event = new StatusEvent(ripper, message); + SwingUtilities.invokeLater(event); } /** Simple TextPane that allows horizontal scrolling. */ class JTextPaneNoWrap extends JTextPane { private static final long serialVersionUID = 1L; + + @Override public boolean getScrollableTracksViewportWidth() { return false; } diff --git a/src/main/java/com/rarchives/ripme/ui/RipStatusHandler.java b/src/main/java/com/rarchives/ripme/ui/RipStatusHandler.java new file mode 100644 index 00000000..d99d039d --- /dev/null +++ b/src/main/java/com/rarchives/ripme/ui/RipStatusHandler.java @@ -0,0 +1,14 @@ + +package com.rarchives.ripme.ui; + +import com.rarchives.ripme.ripper.AbstractRipper; + +/** + * + * @author Mads + */ +public interface RipStatusHandler { + + public void update(AbstractRipper ripper, RipStatusMessage message); + +}