]> source.dussan.org Git - vaadin-framework.git/commitdiff
Eliminate .v-caption memory leak (#13346)
authorHenri Sara <hesara@vaadin.com>
Fri, 14 Mar 2014 10:20:08 +0000 (12:20 +0200)
committerVaadin Code Review <review@vaadin.com>
Fri, 14 Mar 2014 10:28:36 +0000 (10:28 +0000)
Change-Id: I6577dabaaf5d9fa4c73158d3391dfcd28dd0629e

client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java
uitest/src/com/vaadin/tests/components/orderedlayout/CaptionLeak.java [new file with mode: 0644]
uitest/src/com/vaadin/tests/components/orderedlayout/CaptionLeakTest.java [new file with mode: 0644]

index 8ee1921ed72443f48f9c8999e1c8f6df8cbce266..ce73627c88f47127904b0baa98a6a6706195c903 100644 (file)
@@ -267,8 +267,7 @@ public abstract class AbstractOrderedLayoutConnector extends
 
         if (slot.hasCaption()) {
             CaptionPosition pos = slot.getCaptionPosition();
-            getLayoutManager().addElementResizeListener(
-                    slot.getCaptionElement(), slotCaptionResizeListener);
+            slot.setCaptionResizeListener(slotCaptionResizeListener);
             if (child.isRelativeHeight()
                     && (pos == CaptionPosition.TOP || pos == CaptionPosition.BOTTOM)) {
                 getWidget().updateCaptionOffset(slot.getCaptionElement());
diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/CaptionLeak.java b/uitest/src/com/vaadin/tests/components/orderedlayout/CaptionLeak.java
new file mode 100644 (file)
index 0000000..7d347d6
--- /dev/null
@@ -0,0 +1,62 @@
+package com.vaadin.tests.components.orderedlayout;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.ComponentContainer;
+import com.vaadin.ui.CssLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Panel;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.UI;
+import com.vaadin.ui.VerticalLayout;
+
+/**
+ * HorizontalLayout and VerticalLayout should not leak caption elements via
+ * listeners when removing components from a layout.
+ * 
+ * @since 7.1.13
+ * @author Vaadin Ltd
+ */
+public class CaptionLeak extends UI {
+
+    public static final String USAGE = "Open this UI with ?debug and count"
+            + " measured non-connector elements after setting leaky and non leaky"
+            + " content.";
+
+    @Override
+    public void init(VaadinRequest req) {
+        final VerticalLayout root = new VerticalLayout();
+        setContent(root);
+        Label usage = new Label(USAGE);
+        Panel parent = new Panel();
+        Button setLeakyContent = makeButton("Set leaky content", parent,
+                VerticalLayout.class);
+        Button setNonLeakyContent = makeButton("Set non leaky content", parent,
+                CssLayout.class);
+        root.addComponents(usage, setLeakyContent, setNonLeakyContent, parent);
+    }
+
+    private Button makeButton(String caption, final Panel parent,
+            final Class<? extends ComponentContainer> targetClass) {
+        Button btn = new Button(caption);
+        btn.setId(caption);
+        btn.addClickListener(new Button.ClickListener() {
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                try {
+                    ComponentContainer target = targetClass.newInstance();
+                    for (int i = 0; i < 61; i++) {
+                        target.addComponent(new TextField("Test"));
+                    }
+                    parent.setContent(target);
+                } catch (InstantiationException e) {
+                    throw new RuntimeException(e);
+                } catch (IllegalAccessException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        });
+        return btn;
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/CaptionLeakTest.java b/uitest/src/com/vaadin/tests/components/orderedlayout/CaptionLeakTest.java
new file mode 100644 (file)
index 0000000..142ca00
--- /dev/null
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2000-2013 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.orderedlayout;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class CaptionLeakTest extends MultiBrowserTest {
+
+    @Test
+    public void testCaptionLeak() throws Exception {
+        setDebug(true);
+        openTestURL();
+
+        // this should be present
+        // 3 general non-connector elements, none accumulated on click
+        getDriver()
+                .findElement(
+                        By.xpath("//span[text() = 'Measured 3 non connector elements']"));
+
+        getDriver().findElement(By.xpath("//button[@title = 'Clear log']"))
+                .click();
+        getDriver().findElement(By.id("Set leaky content")).click();
+
+        getDriver()
+                .findElement(
+                        By.xpath("//span[text() = 'Measured 3 non connector elements']"));
+
+        // nothing accumulates over clicks
+        getDriver().findElement(By.xpath("//button[@title = 'Clear log']"))
+                .click();
+        getDriver().findElement(By.id("Set leaky content")).click();
+        getDriver()
+                .findElement(
+                        By.xpath("//span[text() = 'Measured 3 non connector elements']"));
+    }
+
+    @Test
+    public void testNoCaptionLeak() throws Exception {
+        setDebug(true);
+        openTestURL();
+
+        getDriver().findElement(By.xpath("//button[@title = 'Clear log']"))
+                .click();
+        getDriver().findElement(By.id("Set non leaky content")).click();
+
+        // this should be present
+        // 3 general non-connector elements, none accumulated on click
+        getDriver()
+                .findElement(
+                        By.xpath("//span[text() = 'Measured 3 non connector elements']"));
+    }
+}