Fix handler creation to happen on init (#11172)

Fix handler creation to happen on init

diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/ComponentRendererConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/ComponentRendererConnector.java
index 649cec0b36620d799609c0a6e2e4aa758e053d4d..df8ace1a4e9f9811fb22c939b553973562d7a625 100644 (file)
--- a/client/src/main/java/com/vaadin/client/connectors/grid/ComponentRendererConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/grid/ComponentRendererConnector.java
@@ -25,6 +25,7 @@ import com.google.gwt.user.client.ui.SimplePanel;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.ConnectorMap;
+import com.vaadin.client.ServerConnector;
 import com.vaadin.client.renderers.Renderer;
 import com.vaadin.client.renderers.WidgetRenderer;
 import com.vaadin.client.ui.AbstractComponentConnector;
@@ -43,11 +44,17 @@ import com.vaadin.ui.renderers.ComponentRenderer;
  */
 @Connect(ComponentRenderer.class)
 public class ComponentRendererConnector
-    extends AbstractGridRendererConnector<String> {
+        extends AbstractGridRendererConnector<String> {
 
     private HashSet<String> knownConnectors = new HashSet<>();
     private HandlerRegistration handlerRegistration;
 
+    @Override
+    public void setParent(ServerConnector parent) {
+        super.setParent(parent);
+        createConnectorHierarchyChangeHandler();
+    }
+
     @Override
     protected Renderer<String> createRenderer() {
         return new WidgetRenderer<String, SimplePanel>() {
@@ -61,12 +68,12 @@ public class ComponentRendererConnector
 
             @Override
             public void render(RendererCellReference cell, String connectorId,
-                               SimplePanel widget) {
-                createConnectorHierarchyChangeHandler();
+                    SimplePanel widget) {
+                assert handlerRegistration != null : "HirarchyChangeHandler should not be null when rendering.";
                 Widget connectorWidget = null;
                 if (connectorId != null) {
                     ComponentConnector connector = (ComponentConnector) ConnectorMap
-                        .get(getConnection()).getConnector(connectorId);
+                            .get(getConnection()).getConnector(connectorId);
                     if (connector != null) {
                         connectorWidget = connector.getWidget();
                         knownConnectors.add(connectorId);
@@ -95,23 +102,26 @@ public class ComponentRendererConnector
 
     /**
      * Adds a listener for grid hierarchy changes to find detached connectors
-     * previously handled by this renderer in order to detach from DOM their widgets
-     * before {@link AbstractComponentConnector#onUnregister()} is invoked
-     * otherwise an error message is logged.
+     * previously handled by this renderer in order to detach from DOM their
+     * widgets before {@link AbstractComponentConnector#onUnregister()} is
+     * invoked otherwise an error message is logged.
      */
     private void createConnectorHierarchyChangeHandler() {
-        if (handlerRegistration == null) {
-            handlerRegistration = getGridConnector().addConnectorHierarchyChangeHandler(event -> {
-                Iterator<String> iterator = knownConnectors.iterator();
-                while (iterator.hasNext()) {
-                    ComponentConnector connector = (ComponentConnector) ConnectorMap.get(getConnection()).getConnector(iterator.next());
-                    if (connector != null && connector.getParent() == null) {
-                        connector.getWidget().removeFromParent();
-                        iterator.remove();
+        assert handlerRegistration == null : "Trying to re-initialize HierarchyChangeHandler";
+        handlerRegistration = getGridConnector()
+                .addConnectorHierarchyChangeHandler(event -> {
+                    Iterator<String> iterator = knownConnectors.iterator();
+                    while (iterator.hasNext()) {
+                        ComponentConnector connector = (ComponentConnector) ConnectorMap
+                                .get(getConnection())
+                                .getConnector(iterator.next());
+                        if (connector != null
+                                && connector.getParent() == null) {
+                            connector.getWidget().removeFromParent();
+                            iterator.remove();
+                        }
                     }
-                }
-            });
-        }
+                });
     }
 
     private void unregisterHierarchyHandler() {

diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridMultiSelect.java b/server/src/main/java/com/vaadin/ui/components/grid/GridMultiSelect.java
index bf3c9d02d3a37f8ab3bed7483310d75111db58cc..b4c4dbc236b98c8a925cd3e299ed2ab3de1627a6 100644 (file)
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridMultiSelect.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridMultiSelect.java
@@ -84,7 +84,7 @@ public class GridMultiSelect<T> implements MultiSelect<T> {
     /**
      * Selects the given item. If another item was already selected, that item
      * is deselected.
-     * 
+     *
      * @param item
      *            the item to select
      */

diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridSingleSelect.java b/server/src/main/java/com/vaadin/ui/components/grid/GridSingleSelect.java
index 41804ad617c446277f6b3dd64a69110f3159e952..d2bb13da62abe93e68f119c9140255d231a9d81f 100644 (file)
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridSingleSelect.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridSingleSelect.java
@@ -211,7 +211,7 @@ public class GridSingleSelect<T> implements SingleSelect<T> {
     /**
      * Selects the given item. If another item was already selected, that item
      * is deselected.
-     * 
+     *
      * @param item
      *            the item to select
      */

diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java
index 8c8810f7a8233cc494380ae6c335bb35d9edc225..aaa2d98027a02967c647be57829ef5393bab4ebf 100644 (file)
--- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java
@@ -24,9 +24,9 @@ public class GridComponentsVisibility extends AbstractTestUIWithLog {
     protected void setup(VaadinRequest request) {
         Grid<String> grid = new Grid<>();
         grid.addColumn(string -> new Label(string), new ComponentRenderer())
-            .setId("label").setCaption("Column 0");
+                .setId("label").setCaption("Column 0");
         grid.getDefaultHeaderRow().getCell("label")
-            .setComponent(new Label("Label"));
+                .setComponent(new Label("Label"));
         grid.addComponentColumn(string -> {
             if (textFields.containsKey(string)) {
                 log("Reusing old text field for: " + string);
@@ -41,7 +41,7 @@ public class GridComponentsVisibility extends AbstractTestUIWithLog {
         grid.addColumn(string -> {
 
             Button button = new Button("Click Me!",
-                event -> toggleFieldVisibility(string));
+                    event -> toggleFieldVisibility(string));
             button.setId(string.replace(' ', '_').toLowerCase(Locale.ROOT));
             return button;
         }, new ComponentRenderer()).setId("button").setCaption("Button");
@@ -49,13 +49,13 @@ public class GridComponentsVisibility extends AbstractTestUIWithLog {
         grid.setRowHeight(40);
 
         grid.getDefaultHeaderRow().join("textField", "button")
-            .setText("Other Components");
+                .setText("Other Components");
 
         addComponent(grid);
         grid.setSizeFull();
 
         grid.setItems(IntStream.range(0, 5).boxed()
-            .map(i -> "Row " + (i + (counter * 1000))));
+                .map(i -> "Row " + (i + (counter * 1000))));
 
     }
 

diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java
index 14fa82a58ead77015470b2e8f984acf9eb321d13..3e8f80649954e4b19943f1defda7dc922382c6e3 100644 (file)
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java
@@ -23,7 +23,8 @@ public class GridComponentsVisibilityTest extends MultiBrowserTest {
 
     @Test
     public void changingVisibilityOfComponentShouldNotThrowClientSideExceptions() {
-        testHideComponent(grid -> ThreadLocalRandom.current().nextInt(1, (int) grid.getRowCount() - 1));
+        testHideComponent(grid -> ThreadLocalRandom.current().nextInt(1,
+                (int) grid.getRowCount() - 1));
     }
 
     @Test
@@ -31,12 +32,13 @@ public class GridComponentsVisibilityTest extends MultiBrowserTest {
         testHideComponent(grid -> (int) grid.getRowCount() - 1);
     }
 
-    private void testHideComponent(Function<GridElement, Integer> rowUnderTestSupplier) {
+    private void testHideComponent(
+            Function<GridElement, Integer> rowUnderTestSupplier) {
         openTestURL("debug");
         GridElement grid = $(GridElement.class).first();
         int rowUnderTest = rowUnderTestSupplier.apply(grid);
         assertTrue("Text field should be visible", grid.getCell(rowUnderTest, 1)
-            .isElementPresent(TextFieldElement.class));
+                .isElementPresent(TextFieldElement.class));
         assertOtherConnectorsArePresent(grid, rowUnderTest);
 
         clearDebugMessages();
@@ -57,34 +59,40 @@ public class GridComponentsVisibilityTest extends MultiBrowserTest {
         clickVisibilityToggleButton(grid, rowUnderTest);
     }
 
-    private void assertOnlyTextFieldOnTestedRowIsNotPresent(GridElement grid, int rowUnderTest) {
-        assertFalse("Text field should not be visible", grid.getCell(rowUnderTest, 1)
-            .isElementPresent(TextFieldElement.class));
+    private void assertOnlyTextFieldOnTestedRowIsNotPresent(GridElement grid,
+            int rowUnderTest) {
+        assertFalse("Text field should not be visible",
+                grid.getCell(rowUnderTest, 1)
+                        .isElementPresent(TextFieldElement.class));
         assertOtherConnectorsArePresent(grid, rowUnderTest);
     }
 
-    private void assertAllTextFieldsArePresent(GridElement grid, int rowUnderTest) {
+    private void assertAllTextFieldsArePresent(GridElement grid,
+            int rowUnderTest) {
         assertTrue("Text field should be visible", grid.getCell(rowUnderTest, 1)
-            .isElementPresent(TextFieldElement.class));
+                .isElementPresent(TextFieldElement.class));
         assertOtherConnectorsArePresent(grid, rowUnderTest);
     }
 
     private void assertNotClientSideErrors() {
         assertNoErrorNotifications();
-        // Should not log "Widget is still attached to the DOM ..." error message
+        // Should not log "Widget is still attached to the DOM ..." error
+        // message
         assertNoDebugMessage(Level.SEVERE);
     }
 
-    private void clickVisibilityToggleButton(GridElement grid, int rowUnderTest) {
-        grid.getRow(rowUnderTest).getCell(2).findElement(By.className("v-button")).click();
+    private void clickVisibilityToggleButton(GridElement grid,
+            int rowUnderTest) {
+        grid.getRow(rowUnderTest).getCell(2)
+                .findElement(By.className("v-button")).click();
     }
 
-    private void assertOtherConnectorsArePresent(GridElement grid, int rowUnderTest) {
+    private void assertOtherConnectorsArePresent(GridElement grid,
+            int rowUnderTest) {
         IntStream.range(1, (int) grid.getRowCount())
-            .filter(row -> row != rowUnderTest)
-            .forEach(row ->
-                assertTrue("Text field should be visible", grid.getCell(row, 1)
-                    .isElementPresent(TextFieldElement.class))
-            );
+                .filter(row -> row != rowUnderTest)
+                .forEach(row -> assertTrue("Text field should be visible",
+                        grid.getCell(row, 1)
+                                .isElementPresent(TextFieldElement.class)));
     }
 }

diff --git a/uitest/src/test/java/com/vaadin/tests/push/BasicPushTest.java b/uitest/src/test/java/com/vaadin/tests/push/BasicPushTest.java
index 1264affcf2a4b98768529b4608b1c8d176a5e20e..91408fa4b4ccce4d3295d882a77c5f8ce90f8342 100644 (file)
--- a/uitest/src/test/java/com/vaadin/tests/push/BasicPushTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/push/BasicPushTest.java
@@ -70,14 +70,13 @@ public abstract class BasicPushTest extends MultiBrowserTest {
 
     protected void waitUntilClientCounterChanges(final int expectedValue) {
         waitUntil(input -> {
-                    try {
-                        return BasicPushTest
-                                .getClientCounter(BasicPushTest.this) == expectedValue;
-                    } catch (NoSuchElementException e) {
-                        return false;
-                    }
-                },
-                10);
+            try {
+                return BasicPushTest
+                        .getClientCounter(BasicPushTest.this) == expectedValue;
+            } catch (NoSuchElementException e) {
+                return false;
+            }
+        }, 10);
     }
 
     protected void waitUntilServerCounterChanges() {