Fixed tree memory leak when removing expanded items from Tree (based on new Filterable API introduced by #11234) #11053

Change-Id: I397124cbfa355417717d2e81bf67b15b202bf16a

2 files changed, 31 insertions, 5 deletions

diff --git a/server/src/com/vaadin/ui/Tree.java b/server/src/com/vaadin/ui/Tree.java
index a6dbea51ba..34cfbaf61b 100644
--- a/server/src/com/vaadin/ui/Tree.java
+++ b/server/src/com/vaadin/ui/Tree.java
@@ -861,6 +861,37 @@ public class Tree extends AbstractSelect implements Container.Hierarchical,
 
     }
 
+    @Override
+    public void containerItemSetChange(
+            com.vaadin.data.Container.ItemSetChangeEvent event) {
+        super.containerItemSetChange(event);
+        if (getContainerDataSource() instanceof Filterable) {
+            boolean hasFilters = ((Filterable) getContainerDataSource())
+                    .hasContainerFilters();
+            if (!hasFilters) {
+                /*
+                 * If Container is not filtered then the itemsetchange is caused
+                 * by either adding or removing items to the container. To
+                 * prevent a memory leak we should cleanup the expanded list
+                 * from items which was removed.
+                 * 
+                 * However, there will still be a leak if the container is
+                 * filtered to show only a subset of the items in the tree and
+                 * later unfiltered items are removed from the container. In
+                 * that case references to the unfiltered item ids will remain
+                 * in the expanded list until the Tree instance is removed and
+                 * the list is destroyed, or the container data source is
+                 * replaced/updated. To force the removal of the removed items
+                 * the application developer needs to a) remove the container
+                 * filters temporarly or b) re-apply the container datasource
+                 * using setContainerDataSource(getContainerDataSource())
+                 */
+                cleanupExpandedItems();
+            }
+        }
+
+    }
+
     /* Expand event and listener */
 
     /**
diff --git a/server/tests/src/com/vaadin/tests/server/component/tree/TreeTest.java b/server/tests/src/com/vaadin/tests/server/component/tree/TreeTest.java
index 634e6a86f3..a0d57c4d59 100644
--- a/server/tests/src/com/vaadin/tests/server/component/tree/TreeTest.java
+++ b/server/tests/src/com/vaadin/tests/server/component/tree/TreeTest.java
@@ -9,7 +9,6 @@ import java.lang.reflect.Field;
 import java.util.HashSet;
 
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import com.vaadin.data.Container;
@@ -84,10 +83,6 @@ public class TreeTest {
                 .getContainerDataSource().getClass()));
     }
 
-    @Ignore("This test tests that item ids which are removed are also "
-            + "removed from the expand list to prevent a memory leak. "
-            + "Fixing the memory leak cannot be done without changing some API (see #11053) "
-            + "so ignoring this test for the 7.0.x series.")
     @Test
     public void testRemoveExpandedItems() throws Exception {
         tree.expandItem("parent");