Tidy up how windows are added/removed.

- Don't have FrameContainers add themselves to the WindowManager in
  the constructor.

- Don't make FrameContainers keep a reference to WindowManager just
  to remove themselves when they're closed

- Instead have WindowManager register a FrameClosed listener and use
  that to remove windows itself.

Change-Id: Iadbca8a28f7ef3372df7fda722a3cbba2a0fbad6
Depends-On: I0f60ac23438313cab49cdb86c089938f5ee44abc
Reviewed-on: http://gerrit.dmdirc.com/2842
Reviewed-by: Greg Holmes <greg@dmdirc.com>
Automatic-Compile: DMDirc Build Manager

src/com/dmdirc/Channel.java

@@ -35,7 +35,6 @@ import com.dmdirc.parser.interfaces.ChannelClientInfo;
 import com.dmdirc.parser.interfaces.ChannelInfo;
 import com.dmdirc.parser.interfaces.ClientInfo;
 import com.dmdirc.ui.Colour;
-import com.dmdirc.ui.WindowManager;
 import com.dmdirc.ui.core.components.WindowComponent;
 import com.dmdirc.ui.input.TabCompleter;
 import com.dmdirc.ui.input.TabCompleterFactory;
@@ -106,19 +105,17 @@ public class Channel extends MessageTarget implements ConfigChangeListener {
      * @param configMigrator The config migrator which provides the config for this channel.
      * @param tabCompleterFactory The factory to use to create tab completers.
      * @param messageSinkManager The sink manager to use to despatch messages.
-     * @param windowManager Window management
      */
     public Channel(
             final Server newServer,
             final ChannelInfo newChannelInfo,
             final ConfigProviderMigrator configMigrator,
             final TabCompleterFactory tabCompleterFactory,
-            final MessageSinkManager messageSinkManager,
-            final WindowManager windowManager) {
+            final MessageSinkManager messageSinkManager) {
         super("channel-inactive", newChannelInfo.getName(),
                 Styliser.stipControlCodes(newChannelInfo.getName()),
                 configMigrator.getConfigProvider(),
-                new ChannelCommandParser(newServer), messageSinkManager, windowManager,
+                new ChannelCommandParser(newServer), messageSinkManager,
                 Arrays.asList(WindowComponent.TEXTAREA.getIdentifier(),
                     WindowComponent.INPUTFIELD.getIdentifier(),
                     WindowComponent.TOPICBAR.getIdentifier(),

src/com/dmdirc/CustomWindow.java

@@ -23,7 +23,6 @@
 package com.dmdirc;
 
 import com.dmdirc.config.IdentityManager;
-import com.dmdirc.ui.WindowManager;
 import com.dmdirc.ui.core.components.WindowComponent;
 
 import java.util.Arrays;
@@ -43,9 +42,7 @@ public class CustomWindow extends FrameContainer {
     public CustomWindow(final String name, final String title,
             final FrameContainer parent) {
         super("custom", name, title, parent.getConfigManager(),
-                Arrays.asList(WindowComponent.TEXTAREA.getIdentifier()), parent.getWindowManager());
-
-        parent.getWindowManager().addWindow(parent, this);
+                Arrays.asList(WindowComponent.TEXTAREA.getIdentifier()));
     }
 
     /**
@@ -53,14 +50,11 @@ public class CustomWindow extends FrameContainer {
      *
      * @param name The name of this custom window
      * @param title The parent of this custom window
-     * @param windowManager For window management
      */
-    public CustomWindow(final String name, final String title, final WindowManager windowManager) {
+    public CustomWindow(final String name, final String title) {
         super("custom", name, title,
                 IdentityManager.getIdentityManager().getGlobalConfiguration(),
-                Arrays.asList(WindowComponent.TEXTAREA.getIdentifier()), windowManager);
-
-        windowManager.addWindow(this);
+                Arrays.asList(WindowComponent.TEXTAREA.getIdentifier()));
     }
 
     /** {@inheritDoc} */

src/com/dmdirc/FrameContainer.java

@@ -106,9 +106,6 @@ public abstract class FrameContainer {
     /** Object used to synchronise styliser access. */
     private final Object documentSync = new Object();
 
-    /** Window management. */
-    private final WindowManager windowManager;
-
     /**
      * Instantiate new frame container.
      *
@@ -117,15 +114,12 @@ public abstract class FrameContainer {
      * @param title The title of this container
      * @param config The config manager for this container
      * @param components The UI components that this frame requires
-     * @param windowManager Window manager to register from
      * @since 0.6.4
      */
     protected FrameContainer(
             final String icon, final String name,
             final String title, final AggregateConfigProvider config,
-            final Collection<String> components,
-            final WindowManager windowManager) {
-        this.windowManager = windowManager;
+            final Collection<String> components) {
         this.configManager = config;
         this.name = name;
         this.title = title;
@@ -223,17 +217,6 @@ public abstract class FrameContainer {
         return new IconManager(getConfigManager(), urlBuilder);
     }
 
-    /**
-     * Gets the window manager associated with this container.
-     *
-     * @return This container's window manager.
-     * @deprecated Window managers should be supplied to classes that need them.
-     */
-    @Deprecated
-    public WindowManager getWindowManager() {
-        return windowManager;
-    }
-
     /**
      * Retrieves the {@link IRCDocument} used to store this frame's content.
      *
@@ -323,8 +306,6 @@ public abstract class FrameContainer {
         }
 
         windowClosing();
-
-        windowManager.removeWindow(this);
     }
 
     /**

src/com/dmdirc/GlobalWindow.java

@@ -61,24 +61,20 @@ public class GlobalWindow extends WritableFrameContainer {
      *
      * @param config The ConfigManager to retrieve settings from.
      * @param parser The command parser to use to parse input.
-     * @param windowManager The window manager.
      * @param tabCompleterFactory The factory to use to create tab completers.
      * @param messageSinkManager The sink manager to use to despatch messages.
      */
     public GlobalWindow(
             final AggregateConfigProvider config,
             final CommandParser parser,
-            final WindowManager windowManager,
             final TabCompleterFactory tabCompleterFactory,
             final MessageSinkManager messageSinkManager) {
-        super("icon", "Global", "(Global)", config, parser, messageSinkManager, windowManager,
+        super("icon", "Global", "(Global)", config, parser, messageSinkManager,
                 Arrays.asList(
                         WindowComponent.TEXTAREA.getIdentifier(),
                         WindowComponent.INPUTFIELD.getIdentifier()));
 
         tabCompleter = tabCompleterFactory.getTabCompleter(config, CommandType.TYPE_GLOBAL);
-
-        windowManager.addWindow(this);
     }
 
     /** {@inheritDoc} */
@@ -176,9 +172,11 @@ public class GlobalWindow extends WritableFrameContainer {
                 if (globalConfig.getOptionBool("general", "showglobalwindow")) {
                     if (globalWindow == null) {
                         globalWindow = new GlobalWindow(globalConfig,
-                                new GlobalCommandParser(globalConfig, commandControllerProvider.get()),
-                                windowManagerProvider.get(), tabCompleterFactory,
+                                new GlobalCommandParser(globalConfig,
+                                commandControllerProvider.get()),
+                                tabCompleterFactory,
                                 messageSinkManagerProvider.get());
+                        windowManagerProvider.get().addWindow(globalWindow);
                     }
                 } else {
                     if (globalWindow != null) {

src/com/dmdirc/MessageTarget.java

@@ -25,7 +25,6 @@ package com.dmdirc;
 import com.dmdirc.commandparser.parsers.CommandParser;
 import com.dmdirc.interfaces.config.AggregateConfigProvider;
 import com.dmdirc.messages.MessageSinkManager;
-import com.dmdirc.ui.WindowManager;
 
 import java.util.Collection;
 
@@ -45,14 +44,13 @@ public abstract class MessageTarget extends WritableFrameContainer {
      * @param parser The command parser for this container
      * @param messageSinkManager The sink manager to use to despatch messages.
      * @param components The UI components that this frame requires
-     * @param windowManager Window manager
      * @since 0.6.4
      */
     public MessageTarget(final String icon, final String name,
             final String title, final AggregateConfigProvider config,
             final CommandParser parser, final MessageSinkManager messageSinkManager,
-            final WindowManager windowManager, final Collection<String> components) {
-        super(icon, name, title, config, parser, messageSinkManager, windowManager, components);
+            final Collection<String> components) {
+        super(icon, name, title, config, parser, messageSinkManager, components);
     }
 
     /**

src/com/dmdirc/Query.java

@@ -39,7 +39,6 @@ import com.dmdirc.parser.interfaces.callbacks.NickChangeListener;
 import com.dmdirc.parser.interfaces.callbacks.PrivateActionListener;
 import com.dmdirc.parser.interfaces.callbacks.PrivateMessageListener;
 import com.dmdirc.parser.interfaces.callbacks.QuitListener;
-import com.dmdirc.ui.WindowManager;
 import com.dmdirc.ui.core.components.WindowComponent;
 import com.dmdirc.ui.input.TabCompleter;
 import com.dmdirc.ui.input.TabCompleterFactory;
@@ -74,16 +73,14 @@ public class Query extends MessageTarget implements PrivateActionListener,
      * @param newServer The server object that this Query belongs to
      * @param tabCompleterFactory The factory to use to create tab completers.
      * @param messageSinkManager The sink manager to use to despatch messages.
-     * @param windowManager Window management
      */
     public Query(final Server newServer, final String newHost,
             final TabCompleterFactory tabCompleterFactory,
-            final MessageSinkManager messageSinkManager,
-            final WindowManager windowManager) {
+            final MessageSinkManager messageSinkManager) {
         super("query", newServer.parseHostmask(newHost)[0],
                 newServer.parseHostmask(newHost)[0],
                 newServer.getConfigManager(), new QueryCommandParser(newServer),
-                messageSinkManager, windowManager,
+                messageSinkManager,
                 Arrays.asList(
                         WindowComponent.TEXTAREA.getIdentifier(),
                         WindowComponent.INPUTFIELD.getIdentifier()));

src/com/dmdirc/Raw.java

@@ -30,7 +30,6 @@ import com.dmdirc.parser.common.CallbackNotFoundException;
 import com.dmdirc.parser.interfaces.Parser;
 import com.dmdirc.parser.interfaces.callbacks.DataInListener;
 import com.dmdirc.parser.interfaces.callbacks.DataOutListener;
-import com.dmdirc.ui.WindowManager;
 import com.dmdirc.ui.core.components.WindowComponent;
 import com.dmdirc.ui.input.TabCompleter;
 
@@ -52,13 +51,11 @@ public class Raw extends WritableFrameContainer
      *
      * @param newServer the server to monitor
      * @param messageSinkManager The sink manager to use to despatch messages.
-     * @param windowManager Window management
      */
-    public Raw(final Server newServer, final MessageSinkManager messageSinkManager,
-            final WindowManager windowManager) {
+    public Raw(final Server newServer, final MessageSinkManager messageSinkManager) {
         super("raw", "Raw", "(Raw log)", newServer.getConfigManager(),
                 new ServerCommandParser(newServer.getConfigManager()),
-                messageSinkManager, windowManager,
+                messageSinkManager,
                 Arrays.asList(
                     WindowComponent.TEXTAREA.getIdentifier(),
                     WindowComponent.INPUTFIELD.getIdentifier()));

src/com/dmdirc/Server.java

@@ -240,7 +240,6 @@ public class Server extends WritableFrameContainer implements ConfigChangeListen
                 configMigrator.getConfigProvider(),
                 commandParser,
                 messageSinkManager,
-                windowManager,
                 Arrays.asList(
                     WindowComponent.TEXTAREA.getIdentifier(),
                     WindowComponent.INPUTFIELD.getIdentifier(),
@@ -567,8 +566,7 @@ public class Server extends WritableFrameContainer implements ConfigChangeListen
         final String lnick = converter.toLowerCase(nick);
 
         if (!queries.containsKey(lnick)) {
-            final Query newQuery = new Query(this, host, tabCompleterFactory,
-                    messageSinkManager, windowManager);
+            final Query newQuery = new Query(this, host, tabCompleterFactory, messageSinkManager);
 
             windowManager.addWindow(this, newQuery, focus);
             ActionManager.getActionManager().triggerEvent(
@@ -608,7 +606,7 @@ public class Server extends WritableFrameContainer implements ConfigChangeListen
     @Override
     public void addRaw() {
         if (raw == null) {
-            raw = new Raw(this, messageSinkManager, windowManager);
+            raw = new Raw(this, messageSinkManager);
             windowManager.addWindow(raw, this);
 
             try {
@@ -668,7 +666,7 @@ public class Server extends WritableFrameContainer implements ConfigChangeListen
             final ConfigProviderMigrator channelConfig = identityFactory.createMigratableConfig(
                     getProtocol(), getIrcd(), getNetwork(), getAddress(), chan.getName());
             final Channel newChan = new Channel(this, chan, channelConfig,
-                    tabCompleterFactory, messageSinkManager, windowManager);
+                    tabCompleterFactory, messageSinkManager);
 
             windowManager.addWindow(this, newChan, focus);
             ActionManager.getActionManager().triggerEvent(CoreActionType.CHANNEL_OPENED, null,

src/com/dmdirc/WritableFrameContainer.java

@@ -28,7 +28,6 @@ import com.dmdirc.interfaces.actions.ActionType;
 import com.dmdirc.interfaces.config.AggregateConfigProvider;
 import com.dmdirc.messages.MessageSinkManager;
 import com.dmdirc.parser.common.CompositionState;
-import com.dmdirc.ui.WindowManager;
 import com.dmdirc.ui.input.TabCompleter;
 
 import java.util.ArrayList;
@@ -65,7 +64,6 @@ public abstract class WritableFrameContainer extends FrameContainer {
      * @param parser The command parser for this container
      * @param messageSinkManager The sink manager to use to despatch messages
      * @param components The UI components that this frame requires
-     * @param windowManager For window management
      * @since 0.6.4
      */
     public WritableFrameContainer(
@@ -75,9 +73,8 @@ public abstract class WritableFrameContainer extends FrameContainer {
             final AggregateConfigProvider config,
             final CommandParser parser,
             final MessageSinkManager messageSinkManager,
-            final WindowManager windowManager,
             final Collection<String> components) {
-        super(icon, name, title, config, components, windowManager);
+        super(icon, name, title, config, components);
 
         this.commandParser = parser;
         this.messageSinkManager = messageSinkManager;

src/com/dmdirc/commandparser/commands/global/OpenWindow.java

@@ -100,10 +100,13 @@ public class OpenWindow extends Command implements IntelligentCommand {
                 ? args.getArgumentsAsString(start + 1) : args.getArguments()[start];
 
             if (window == null) {
+                CustomWindow newWindow;
                 if (parent == null) {
-                    new CustomWindow(args.getArguments()[start], title, windowManager);
+                    newWindow = new CustomWindow(args.getArguments()[start], title);
+                    windowManager.addWindow(newWindow);
                 } else {
-                    new CustomWindow(args.getArguments()[start], title, parent);
+                    newWindow = new CustomWindow(args.getArguments()[start], title, parent);
+                    windowManager.addWindow(parent, newWindow);
                 }
             } else {
                 sendLine(origin, args.isSilent(), FORMAT_ERROR,

src/com/dmdirc/messages/CustomWindowMessageSink.java

@@ -67,6 +67,7 @@ public class CustomWindowMessageSink implements MessageSink {
         if (targetWindow == null) {
             targetWindow = new CustomWindow(patternMatches[0],
                     patternMatches[0], source.getServer());
+            windowManager.addWindow(source.getServer(), targetWindow);
         }
 
         targetWindow.addLine(messageType, date, args);

src/com/dmdirc/ui/WindowManager.java

@@ -25,6 +25,7 @@ package com.dmdirc.ui;
 import com.dmdirc.CustomWindow;
 import com.dmdirc.FrameContainer;
 import com.dmdirc.Precondition;
+import com.dmdirc.interfaces.FrameCloseListener;
 import com.dmdirc.interfaces.ui.FrameListener;
 import com.dmdirc.logger.Logger;
 import com.dmdirc.util.collections.ListenerList;
@@ -50,6 +51,9 @@ public class WindowManager {
     /** A list of frame listeners. */
     private final ListenerList listeners = new ListenerList();
 
+    /** Listener for frame close events. */
+    private final FrameCloseListener closeListener = new CloseListener();
+
     /**
      * Creates a new instance of {@link WindowManager}.
      */
@@ -148,6 +152,8 @@ public class WindowManager {
         rootWindows.add(window);
 
         fireAddWindow(window, focus);
+
+        window.addCloseListener(closeListener);
     }
 
     /**
@@ -186,6 +192,8 @@ public class WindowManager {
         parent.addChild(child);
 
         fireAddWindow(parent, child, focus);
+
+        child.addCloseListener(closeListener);
     }
 
     /**
@@ -351,4 +359,17 @@ public class WindowManager {
         }
     }
 
+    /**
+     * Frame close listener that removes the window from this manager.
+     */
+    private class CloseListener implements FrameCloseListener {
+
+        /** {@inheritDoc} */
+        @Override
+        public void windowClosing(final FrameContainer window) {
+            removeWindow(window);
+        }
+
+    }
+
 }

test/com/dmdirc/WritableFrameContainerTest.java

@@ -27,7 +27,6 @@ import com.dmdirc.config.ConfigBinder;
 import com.dmdirc.harness.TestWritableFrameContainer;
 import com.dmdirc.interfaces.config.AggregateConfigProvider;
 import com.dmdirc.messages.MessageSinkManager;
-import com.dmdirc.ui.WindowManager;
 
 import java.util.Arrays;
 
@@ -43,7 +42,6 @@ public class WritableFrameContainerTest {
 
     @Mock private AggregateConfigProvider acp;
     @Mock private ServerManager serverManager;
-    @Mock private WindowManager windowManager;
     @Mock private MessageSinkManager messageSinkManager;
     private CommandManager commands;
 
@@ -61,7 +59,7 @@ public class WritableFrameContainerTest {
     @Test
     public void testGetNumLines() {
         final WritableFrameContainer container10
-                = new TestWritableFrameContainer(10, acp, commands, messageSinkManager, windowManager);
+                = new TestWritableFrameContainer(10, acp, commands, messageSinkManager);
 
         final int res0a = container10.getNumLines("");
         final int res0b = container10.getNumLines("\r");
@@ -91,7 +89,7 @@ public class WritableFrameContainerTest {
     @Test
     public void testSplitLine() {
         final WritableFrameContainer container10
-                = new TestWritableFrameContainer(10, acp, commands, messageSinkManager, windowManager);
+                = new TestWritableFrameContainer(10, acp, commands, messageSinkManager);
         final String[][][] tests = new String[][][]{
             {{""}, {""}},
             {{"0123456789"}, {"0123456789"}},

test/com/dmdirc/harness/TestWritableFrameContainer.java

@@ -28,7 +28,6 @@ import com.dmdirc.commandparser.CommandManager;
 import com.dmdirc.commandparser.parsers.GlobalCommandParser;
 import com.dmdirc.interfaces.config.AggregateConfigProvider;
 import com.dmdirc.messages.MessageSinkManager;
-import com.dmdirc.ui.WindowManager;
 import com.dmdirc.ui.input.TabCompleter;
 
 import java.util.Collections;
@@ -39,9 +38,9 @@ public class TestWritableFrameContainer extends WritableFrameContainer {
 
     public TestWritableFrameContainer(final int lineLength,
             final AggregateConfigProvider cm, final CommandManager commandManager,
-            final MessageSinkManager messageSinkManager, final WindowManager windowManager) {
+            final MessageSinkManager messageSinkManager) {
         super("raw", "Raw", "(Raw)", cm,
-                new GlobalCommandParser(cm, commandManager), messageSinkManager, windowManager,
+                new GlobalCommandParser(cm, commandManager), messageSinkManager,
                 Collections.<String>emptySet());
 
         this.lineLength = lineLength;