From 4f4ddbc3e06283cc83a9d2d1a7523d1adad6d47b Mon Sep 17 00:00:00 2001
From: Sebastian Stenzel <sebastian.stenzel@gmail.com>
Date: Mon, 9 Nov 2020 13:49:28 +0100
Subject: [PATCH] allow GCing of Stats Window while remaining reusability and
 preventing opening multiple windows per vault

---
 .../VaultDetailUnlockedController.java          | 17 ++++++++---------
 .../ui/stats/VaultStatisticsComponent.java      | 14 ++++++++++++--
 .../ui/stats/VaultStatisticsController.java     | 11 ++++++-----
 3 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnlockedController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnlockedController.java
index 6c20e9490..1806d9e5e 100644
--- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnlockedController.java
+++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailUnlockedController.java
@@ -18,20 +18,19 @@ public class VaultDetailUnlockedController implements FxController {
 
 	private final ReadOnlyObjectProperty<Vault> vault;
 	private final VaultService vaultService;
-	private final LoadingCache<Vault, VaultStatisticsComponent> vaultStatisticsWindows;
-	private final VaultStatisticsComponent.Builder vaultStatisticsWindow;
+	private final LoadingCache<Vault, VaultStatisticsComponent> vaultStats;
+	private final VaultStatisticsComponent.Builder vaultStatsBuilder;
 
 	@Inject
-	public VaultDetailUnlockedController(ObjectProperty<Vault> vault, VaultService vaultService, VaultStatisticsComponent.Builder vaultStatisticsWindow) {
+	public VaultDetailUnlockedController(ObjectProperty<Vault> vault, VaultService vaultService, VaultStatisticsComponent.Builder vaultStatsBuilder) {
 		this.vault = vault;
 		this.vaultService = vaultService;
-		this.vaultStatisticsWindows = CacheBuilder.newBuilder().build(CacheLoader.from(this::provideVaultStatisticsComponent));
-		//TODO make the binding a weak Binding via weakValues
-		this.vaultStatisticsWindow = vaultStatisticsWindow;
+		this.vaultStats = CacheBuilder.newBuilder().weakValues().build(CacheLoader.from(this::buildVaultStats));
+		this.vaultStatsBuilder = vaultStatsBuilder;
 	}
 
-	private VaultStatisticsComponent provideVaultStatisticsComponent(Vault vault) {
-		return vaultStatisticsWindow.vault(vault).build();
+	private VaultStatisticsComponent buildVaultStats(Vault vault) {
+		return vaultStatsBuilder.vault(vault).build();
 	}
 
 	@FXML
@@ -47,7 +46,7 @@ public class VaultDetailUnlockedController implements FxController {
 
 	@FXML
 	public void showVaultStatistics() {
-		vaultStatisticsWindows.getUnchecked(vault.get()).showVaultStatisticsWindow();
+		vaultStats.getUnchecked(vault.get()).showVaultStatisticsWindow();
 	}
 
 	/* Getter/Setter */
diff --git a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsComponent.java b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsComponent.java
index 8d48ef52e..aa7b410c2 100644
--- a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsComponent.java
+++ b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsComponent.java
@@ -3,12 +3,22 @@ package org.cryptomator.ui.stats;
 import dagger.BindsInstance;
 import dagger.Lazy;
 import dagger.Subcomponent;
-import javafx.scene.Scene;
-import javafx.stage.Stage;
 import org.cryptomator.common.vaults.Vault;
 import org.cryptomator.ui.common.FxmlFile;
 import org.cryptomator.ui.common.FxmlScene;
 
+import javafx.scene.Scene;
+import javafx.stage.Stage;
+
+/**
+ * For each vault there can be up to one statistics component.
+ * <p>
+ * <b>Important:</b> Outside of {@link org.cryptomator.ui.stats}, this component should be weakly referenced,
+ * as it include memory-intensive UI nodes.
+ * <p>
+ * While the stats window is visible, this component is strongly referenced by the window's main controller.
+ * As soon as the window is closed, the full objectgraph becomes eligible for GC.
+ */
 @VaultStatisticsScoped
 @Subcomponent(modules = {VaultStatisticsModule.class})
 public interface VaultStatisticsComponent {
diff --git a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsController.java b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsController.java
index b149f9559..f8937e90f 100644
--- a/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsController.java
+++ b/main/ui/src/main/java/org/cryptomator/ui/stats/VaultStatisticsController.java
@@ -28,6 +28,7 @@ public class VaultStatisticsController implements FxController {
 	private static final int IO_SAMPLING_STEPS = 30;
 	private static final double IO_SAMPLING_INTERVAL = 1;
 
+	private final VaultStatisticsComponent component; // keep a strong reference to the component (see component's javadoc)
 	private final VaultStats stats;
 	private final Series<Number, Number> readData;
 	private final Series<Number, Number> writeData;
@@ -54,7 +55,8 @@ public class VaultStatisticsController implements FxController {
 	public NumberAxis writeChartYAxis;
 
 	@Inject
-	public VaultStatisticsController(@VaultStatisticsWindow Stage window, @VaultStatisticsWindow Vault vault) {
+	public VaultStatisticsController(VaultStatisticsComponent component, @VaultStatisticsWindow Stage window, @VaultStatisticsWindow Vault vault) {
+		this.component = component;
 		this.stats = vault.getStats();
 		this.readData = new Series<>();
 		this.writeData = new Series<>();
@@ -77,11 +79,10 @@ public class VaultStatisticsController implements FxController {
 		ioAnimation.setCycleCount(Animation.INDEFINITE);
 		ioAnimation.play();
 
-		// make sure to stop animating,
+		// make sure to stop animating while window is closed
 		// otherwise a global timer (GC root) will keep a strong reference to animation
-		window.setOnHiding(evt -> {
-			ioAnimation.stop();
-		});
+		window.setOnHiding(evt -> ioAnimation.stop());
+		window.setOnShowing(evt -> ioAnimation.play());
 	}
 
 	@FXML