Fixes memory leak in VScrollTable (#14159)

Change-Id: I59596630b71f5a6b78c13bc5dbeaf7ef5dfaccf9

4 files changed, 209 insertions, 6 deletions

diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java
index ef8be5aadc..d88f7426ef 100644
--- a/client/src/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/com/vaadin/client/ui/VScrollTable.java
@@ -588,6 +588,8 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
 
     private boolean hadScrollBars = false;
 
+    private HandlerRegistration addCloseHandler;
+
     public VScrollTable() {
         setMultiSelectMode(MULTISELECT_MODE_DEFAULT);
 
@@ -669,13 +671,14 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         this.client = client;
         // Add a handler to clear saved context menu details when the menu
         // closes. See #8526.
-        client.getContextMenu().addCloseHandler(new CloseHandler<PopupPanel>() {
+        addCloseHandler = client.getContextMenu().addCloseHandler(
+                new CloseHandler<PopupPanel>() {
 
-            @Override
-            public void onClose(CloseEvent<PopupPanel> event) {
-                contextMenu = null;
-            }
-        });
+                    @Override
+                    public void onClose(CloseEvent<PopupPanel> event) {
+                        contextMenu = null;
+                    }
+                });
     }
 
     private void handleBodyContextMenu(ContextMenuEvent event) {
@@ -7929,4 +7932,13 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         // Nothing found.
         return null;
     }
+
+    /**
+     * @since
+     */
+    public void onUnregister() {
+        if (addCloseHandler != null) {
+            addCloseHandler.removeHandler();
+        }
+    }
 }
diff --git a/client/src/com/vaadin/client/ui/table/TableConnector.java b/client/src/com/vaadin/client/ui/table/TableConnector.java
index bea49e5f27..017d1d1024 100644
--- a/client/src/com/vaadin/client/ui/table/TableConnector.java
+++ b/client/src/com/vaadin/client/ui/table/TableConnector.java
@@ -55,6 +55,17 @@ public class TableConnector extends AbstractHasComponentsConnector implements
     /*
      * (non-Javadoc)
      * 
+     * @see com.vaadin.client.ui.AbstractComponentConnector#onUnregister()
+     */
+    @Override
+    public void onUnregister() {
+        super.onUnregister();
+        getWidget().onUnregister();
+    }
+
+    /*
+     * (non-Javadoc)
+     *
      * @see com.vaadin.client.Paintable#updateFromUIDL(com.vaadin.client.UIDL,
      * com.vaadin.client.ApplicationConnection)
      */
diff --git a/uitest/src/com/vaadin/tests/components/table/MemoryLeakTable.java b/uitest/src/com/vaadin/tests/components/table/MemoryLeakTable.java
new file mode 100644
index 0000000000..1b76be3b72
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/MemoryLeakTable.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.table;
+
+import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.Table;
+
+/**
+ * Test UI Class for testing memory leak in table (#14159).
+ *
+ * @since
+ * @author Vaadin Ltd
+ */
+public class MemoryLeakTable extends AbstractTestUI {
+    Button btnAdd = new Button("Add rows");
+    Button btnRemove = new Button("Remove rows");
+    Button btnTenTimes = new Button("Do ten times");
+    Table tbl = new Table();
+    static final int COLS = 15;
+    static final int ROWS = 2000;
+
+    private void addRows() {
+        IndexedContainer idx = new IndexedContainer();
+        for (int i = 0; i < COLS; i++) {
+            idx.addContainerProperty("name " + i, String.class, "value");
+        }
+        for (int i = 0; i < ROWS; i++) {
+            idx.addItem("item" + i);
+        }
+        tbl.setContainerDataSource(idx);
+        addComponent(tbl);
+    }
+
+    private void removeRows() {
+        tbl.removeAllItems();
+        removeComponent(tbl);
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        btnAdd.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                addRows();
+            }
+        });
+        btnRemove.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                removeRows();
+            }
+        });
+        addComponent(btnAdd);
+        addComponent(btnRemove);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Generates table for memory leaking test";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 14159;
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/table/MemoryLeakTableTest.java b/uitest/src/com/vaadin/tests/components/table/MemoryLeakTableTest.java
new file mode 100644
index 0000000000..b4b8d93fbe
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/MemoryLeakTableTest.java
@@ -0,0 +1,95 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.table;
+
+import java.io.IOException;
+import java.util.Random;
+
+import org.junit.Ignore;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.JavascriptExecutor;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.tests.tb3.AbstractTB3Test.RunLocally;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.MultiBrowserTest.Browser;
+
+/**
+ * Test case creating and deleting table component in a loop, testing memory
+ * lead in Table component. This test should not be used in auto testing.
+ *
+ * To test memory consuption. Run test in debug mode. Take memory snapshot in
+ * Profiler in browser before and after the loop. Compare memory consuption.
+ *
+ * @since
+ * @author Vaadin Ltd
+ */
+@RunLocally(Browser.CHROME)
+public class MemoryLeakTableTest extends MultiBrowserTest {
+
+    /**
+     *
+     */
+    private static final int ITERATIONS = 200;
+
+    // To run locally in chrome download ChromeDriver for TB3
+    // Set path to the chrome driver. In
+    // ./work/eclipse-run-selected-test.properties add line
+    // chrome.driver.path=path_to_driver
+
+    // Test is marked as ignore to exclude it from auto testing
+    @Test
+    @Ignore
+    public void memoryTest() throws IOException {
+        // Set breakoint and look memory consuption in Profiler
+        // Mozilla Firefox doesn't provide memory usage profiler, use chrome.
+
+        openTestURL();
+
+        ButtonElement btnAdd = $(ButtonElement.class).get(0);
+
+        for (int i = 0; i < ITERATIONS; i++) {
+            btnAdd.click();
+            ButtonElement btnDel = $(ButtonElement.class).get(1);
+            TableElement tbl = $(TableElement.class).get(0);
+            Random rand = new Random();
+            int scrollValue = rand.nextInt(1500);
+            scrollTable(tbl, scrollValue);
+            try {
+                Thread.sleep(1000);
+            } catch (InterruptedException e) {
+                // TODO Auto-generated catch block
+                e.printStackTrace();
+            }
+            btnDel.click();
+        }
+        // Set breakoint and look memory consuption in Profiler
+        btnAdd = $(ButtonElement.class).get(0);
+    }
+
+    // Scrolls table element
+    // Method scroll in TalbeElement class has a bug
+    //
+    private void scrollTable(TableElement tbl, int value) {
+        WebElement actualElement = tbl.findElement(By
+                .className("v-table-body-wrapper"));
+        JavascriptExecutor js = tbl.getCommandExecutor();
+        js.executeScript("arguments[0].scrollTop = " + value, actualElement);
+    }
+}