From: Pekka Hyvönen Date: Fri, 14 Mar 2014 14:17:27 +0000 (+0200) Subject: Fixes a memory leak on IE8 in LayoutManagerIE8 (#12688) X-Git-Tag: 7.1.13~16 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0c7cbc73dba97ad0b95cb11f4b4eabb37046fe61;p=vaadin-framework.git Fixes a memory leak on IE8 in LayoutManagerIE8 (#12688) Change-Id: Ieae3b1d82e92fadf5ab517c1c878fc82bcc0ecbd --- diff --git a/client/src/com/vaadin/client/LayoutManager.java b/client/src/com/vaadin/client/LayoutManager.java index 1ced003146..5b27031a29 100644 --- a/client/src/com/vaadin/client/LayoutManager.java +++ b/client/src/com/vaadin/client/LayoutManager.java @@ -74,6 +74,15 @@ public class LayoutManager { this.connection = connection; } + /** + * Returns the application connection for this layout manager. + * + * @return connection + */ + protected ApplicationConnection getConnection() { + return connection; + } + /** * Gets the layout manager associated with the given * {@link ApplicationConnection}. diff --git a/client/src/com/vaadin/client/LayoutManagerIE8.java b/client/src/com/vaadin/client/LayoutManagerIE8.java index a692f126a2..97e3059a22 100644 --- a/client/src/com/vaadin/client/LayoutManagerIE8.java +++ b/client/src/com/vaadin/client/LayoutManagerIE8.java @@ -21,6 +21,7 @@ import java.util.Map; import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Element; +import com.google.gwt.dom.client.Node; import com.google.gwt.user.client.ui.RootPanel; /** @@ -39,6 +40,12 @@ public class LayoutManagerIE8 extends LayoutManager { private Map measuredSizes = new HashMap(); + // this method is needed to test for memory leaks (see + // LayoutMemoryUsageIE8ExtensionConnector) but can be private + private int getMeasuredSizesMapSize() { + return measuredSizes.size(); + } + @Override protected void setMeasuredSize(Element element, MeasuredSize measuredSize) { if (measuredSize != null) { @@ -62,12 +69,16 @@ public class LayoutManagerIE8 extends LayoutManager { @Override protected void cleanMeasuredSizes() { Profiler.enter("LayoutManager.cleanMeasuredSizes"); - Document document = RootPanel.get().getElement().getOwnerDocument(); + + // #12688: IE8 was leaking memory when adding&removing components. + // Uses IE specific logic to figure if an element has been removed from + // DOM or not. For removed elements the measured size is discarded. + Node rootNode = Document.get().getBody(); Iterator i = measuredSizes.keySet().iterator(); while (i.hasNext()) { Element e = i.next(); - if (e.getOwnerDocument() != document) { + if (!rootNode.isOrHasChild(e)) { i.remove(); } } diff --git a/uitest/src/com/vaadin/tests/extensions/LayoutMemoryUsageIE8Extension.java b/uitest/src/com/vaadin/tests/extensions/LayoutMemoryUsageIE8Extension.java new file mode 100644 index 0000000000..c69c2b4d30 --- /dev/null +++ b/uitest/src/com/vaadin/tests/extensions/LayoutMemoryUsageIE8Extension.java @@ -0,0 +1,36 @@ +/* + * 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.extensions; + +import com.vaadin.server.AbstractExtension; +import com.vaadin.ui.UI; + +/** + * Test extension for finding out the size of the measuredSizes map of + * LayoutManagerIE8. + * + * This UI extension uses JSNI to register a JavaScript method + * window.vaadin.getMeasuredSizesCount() that can be used to query the size of + * the internal map of the layout manager. It should only be used on IE8. + * + * @since 7.1.13 + * @author Vaadin Ltd + */ +public class LayoutMemoryUsageIE8Extension extends AbstractExtension { + public void extend(UI target) { + super.extend(target); + } +} diff --git a/uitest/src/com/vaadin/tests/layouts/IE8MeasuredSizeMemoryLeak.java b/uitest/src/com/vaadin/tests/layouts/IE8MeasuredSizeMemoryLeak.java new file mode 100644 index 0000000000..7da9e46653 --- /dev/null +++ b/uitest/src/com/vaadin/tests/layouts/IE8MeasuredSizeMemoryLeak.java @@ -0,0 +1,111 @@ +/* + * 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.layouts; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.tests.extensions.LayoutMemoryUsageIE8Extension; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.HasComponents; +import com.vaadin.ui.Label; +import com.vaadin.ui.VerticalLayout; + +@Widgetset("com.vaadin.tests.widgetset.TestingWidgetSet") +public class IE8MeasuredSizeMemoryLeak extends AbstractTestUI { + + private boolean state = false; + + private HasComponents component1 = new VerticalLayout() { + { + for (int i = 1; i <= 200; i++) { + String idText = "ID:" + i; + Label c = new Label(idText); + c.setId(idText); + addComponent(c); + } + } + }; + + private HasComponents component2 = new VerticalLayout() { + { + for (int i = 201; i <= 400; i++) { + String idText = "ID:" + i; + Label c = new Label(idText); + c.setId(idText); + addComponent(c); + } + } + }; + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server. + * VaadinRequest) + */ + @Override + protected void setup(VaadinRequest request) { + new LayoutMemoryUsageIE8Extension().extend(this); + + VerticalLayout layout = new VerticalLayout(); + setContent(layout); + + final VerticalLayout contentLayout = new VerticalLayout(); + + Button button = new Button("Toggle"); + button.setId("toggle"); + button.addClickListener(new ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + contentLayout.removeAllComponents(); + if (state) { + contentLayout.addComponent(component1); + } else { + contentLayout.addComponent(component2); + } + state = !state; + } + + }); + + layout.addComponent(button); + layout.addComponent(contentLayout); + + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription() + */ + @Override + protected String getTestDescription() { + return "IE8 leaks memory when components are added and removed"; + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber() + */ + @Override + protected Integer getTicketNumber() { + return 12688; + } +} diff --git a/uitest/src/com/vaadin/tests/layouts/IE8MeasuredSizeMemoryLeakTest.java b/uitest/src/com/vaadin/tests/layouts/IE8MeasuredSizeMemoryLeakTest.java new file mode 100644 index 0000000000..c9bd64c881 --- /dev/null +++ b/uitest/src/com/vaadin/tests/layouts/IE8MeasuredSizeMemoryLeakTest.java @@ -0,0 +1,54 @@ +/* + * 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.layouts; + +import java.util.Collections; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.JavascriptExecutor; +import org.openqa.selenium.remote.DesiredCapabilities; + +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class IE8MeasuredSizeMemoryLeakTest extends MultiBrowserTest { + + @Test + public void testMeasuredSizesMapCleaned() { + openTestURL(); + Assert.assertEquals("No extra measured sizes in the beginning", 3, + getMeasuredSizesMapSize()); + vaadinElementById("toggle").click(); + Assert.assertEquals("Measured sizes after single toggle", 204, + getMeasuredSizesMapSize()); + vaadinElementById("toggle").click(); + Assert.assertEquals("Measured sizes cleaned on toggle", 204, + getMeasuredSizesMapSize()); + } + + private int getMeasuredSizesMapSize() { + JavascriptExecutor jsExec = (JavascriptExecutor) getDriver(); + Number result = (Number) jsExec + .executeScript("return window.vaadin.getMeasuredSizesCount();"); + return result.intValue(); + } + + @Override + public List getBrowsersToTest() { + return Collections.singletonList(Browser.IE8.getDesiredCapabilities()); + } +} diff --git a/uitest/src/com/vaadin/tests/widgetset/client/LayoutMemoryUsageIE8ExtensionConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/LayoutMemoryUsageIE8ExtensionConnector.java new file mode 100644 index 0000000000..c92e688520 --- /dev/null +++ b/uitest/src/com/vaadin/tests/widgetset/client/LayoutMemoryUsageIE8ExtensionConnector.java @@ -0,0 +1,45 @@ +/* + * 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.widgetset.client; + +import com.vaadin.client.BrowserInfo; +import com.vaadin.client.LayoutManager; +import com.vaadin.client.LayoutManagerIE8; +import com.vaadin.client.ServerConnector; +import com.vaadin.client.extensions.AbstractExtensionConnector; +import com.vaadin.shared.ui.Connect; +import com.vaadin.tests.extensions.LayoutMemoryUsageIE8Extension; + +@Connect(LayoutMemoryUsageIE8Extension.class) +public class LayoutMemoryUsageIE8ExtensionConnector extends + AbstractExtensionConnector { + + @Override + protected void extend(ServerConnector target) { + if (BrowserInfo.get().isIE8()) { + LayoutManagerIE8 manager = (LayoutManagerIE8) LayoutManager + .get(getConnection()); + configureGetMapSizeJS(manager); + } + } + + private native void configureGetMapSizeJS(LayoutManagerIE8 manager) + /*-{ + $wnd.vaadin.getMeasuredSizesCount = function() { + return manager.@com.vaadin.client.LayoutManagerIE8::getMeasuredSizesMapSize()(); + }; + }-*/; +}