Fix making components visible by push (#9934)

Fix connector tracker cleanup for the case where a component is
hidden by a request and is made visible again by push.

This fixes a regression caused by #9305.

Fixes #9905

8 files changed, 173 insertions, 54 deletions

diff --git a/server/src/main/java/com/vaadin/server/VaadinSession.java b/server/src/main/java/com/vaadin/server/VaadinSession.java
index 671b35ae66..26b9cfa7a1 100644
--- a/server/src/main/java/com/vaadin/server/VaadinSession.java
+++ b/server/src/main/java/com/vaadin/server/VaadinSession.java
@@ -1028,7 +1028,7 @@ public class VaadinSession implements HttpSessionBindingListener, Serializable {
                         }
                     }
                     try {
-                        ui.getConnectorTracker().cleanConnectorMap();
+                        ui.getConnectorTracker().cleanConnectorMap(false);
                     } catch (AssertionError | Exception e) {
                         getLogger().log(Level.SEVERE,
                                 "Exception while cleaning connector map for ui "
diff --git a/server/src/main/java/com/vaadin/server/communication/UidlWriter.java b/server/src/main/java/com/vaadin/server/communication/UidlWriter.java
index d27fc3f0ae..92b6e8e8bd 100644
--- a/server/src/main/java/com/vaadin/server/communication/UidlWriter.java
+++ b/server/src/main/java/com/vaadin/server/communication/UidlWriter.java
@@ -321,6 +321,7 @@ public class UidlWriter implements Serializable {
             writePerformanceData(ui, writer);
         } finally {
             uiConnectorTracker.setWritingResponse(false);
+            uiConnectorTracker.cleanConnectorMap(true);
         }
     }
 
diff --git a/server/src/main/java/com/vaadin/ui/ConnectorTracker.java b/server/src/main/java/com/vaadin/ui/ConnectorTracker.java
index c41c0d0fd5..54fb5c5822 100644
--- a/server/src/main/java/com/vaadin/ui/ConnectorTracker.java
+++ b/server/src/main/java/com/vaadin/ui/ConnectorTracker.java
@@ -71,7 +71,7 @@ public class ConnectorTracker implements Serializable {
 
     /**
      * Connectors that have been unregistered and should be cleaned up the next
-     * time {@link #cleanConnectorMap()} is invoked unless they have been
+     * time {@link #cleanConnectorMap(boolean)} is invoked unless they have been
      * registered again.
      */
     private final Set<ClientConnector> unregisteredConnectors = new HashSet<>();
@@ -264,12 +264,28 @@ public class ConnectorTracker implements Serializable {
 
     /**
      * Cleans the connector map from all connectors that are no longer attached
+     * to the application if there are dirty connectors or the force flag is
+     * true. This should only be called by the framework.
+     *
+     * @param force
+     *            {@code true} to force cleaning
+     * @since
+     */
+    public void cleanConnectorMap(boolean force) {
+        if (force || !dirtyConnectors.isEmpty()) {
+            cleanConnectorMap();
+        }
+    }
+
+    /**
+     * Cleans the connector map from all connectors that are no longer attached
      * to the application. This should only be called by the framework.
+     *
+     * @deprecated use {@link #cleanConnectorMap(boolean)} instead
      */
+    @Deprecated
     public void cleanConnectorMap() {
-        if (!unregisteredConnectors.isEmpty()) {
-            removeUnregisteredConnectors();
-        }
+        removeUnregisteredConnectors();
 
         cleanStreamVariables();
 
diff --git a/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java b/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java
index 2b85318ea4..0c91814f89 100644
--- a/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java
+++ b/uitest/src/main/java/com/vaadin/tests/application/MissingHierarchyDetection.java
@@ -15,15 +15,7 @@
  */
 package com.vaadin.tests.application;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.logging.Handler;
-import java.util.logging.LogRecord;
-import java.util.logging.Logger;
-
 import com.vaadin.server.VaadinRequest;
-import com.vaadin.server.VaadinSession;
 import com.vaadin.tests.components.AbstractReindeerTestUIWithLog;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
@@ -39,8 +31,6 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog {
 
     private CssLayout normalLayout = new CssLayout(
             new Label("Normal layout child"));
-    private List<LogRecord> pendingErrors = Collections
-            .synchronizedList(new ArrayList<>());
 
     public class BrokenCssLayout extends CssLayout
             implements SelectiveRenderer {
@@ -59,29 +49,6 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog {
 
     @Override
     protected void setup(VaadinRequest request) {
-        // Catch log messages so we can see if there is an error
-        Logger vaadinSessionLogger = Logger
-                .getLogger(VaadinSession.class.getName());
-        vaadinSessionLogger.addHandler(new Handler() {
-
-            @Override
-            public void publish(LogRecord record) {
-                if (record.getThrown() instanceof AssertionError) {
-                    pendingErrors.add(record);
-                    vaadinSessionLogger.removeHandler(this);
-                }
-            }
-
-            @Override
-            public void flush() {
-
-            }
-
-            @Override
-            public void close() throws SecurityException {
-
-            }
-        });
         addComponent(brokenLayout);
         addComponent(normalLayout);
         addComponent(new Button("Toggle properly", new Button.ClickListener() {
@@ -97,16 +64,6 @@ public class MissingHierarchyDetection extends AbstractReindeerTestUIWithLog {
                         toggle(false);
                     }
                 }));
-        addComponent(new Button("Check for errors", new Button.ClickListener() {
-            @Override
-            public void buttonClick(ClickEvent event) {
-                if (!pendingErrors.isEmpty()) {
-                    log(pendingErrors.remove(0).getThrown().getMessage());
-                } else {
-                    log("No errors");
-                }
-            }
-        }));
     }
 
     private void toggle(boolean properly) {
diff --git a/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java b/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java
index ae3667ff29..8e00357b56 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/table/TableRemovedQuicklySendsInvalidRpcCalls.java
@@ -91,6 +91,11 @@ public class TableRemovedQuicklySendsInvalidRpcCalls
         }
 
         @Override
+        public void cleanConnectorMap(boolean force) {
+            tracker.cleanConnectorMap(force);
+        }
+
+        @Override
         public void cleanConnectorMap() {
             tracker.cleanConnectorMap();
         }
diff --git a/uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java b/uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java
new file mode 100644
index 0000000000..2bc93317e8
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/push/MakeComponentVisibleWithPush.java
@@ -0,0 +1,104 @@
+package com.vaadin.tests.push;

+

+import java.util.ArrayList;

+import java.util.List;

+

+import com.vaadin.annotations.Push;

+import com.vaadin.server.VaadinRequest;

+import com.vaadin.tests.components.AbstractTestUI;

+import com.vaadin.ui.Button;

+import com.vaadin.ui.Grid;

+import com.vaadin.ui.UI;

+import com.vaadin.ui.VerticalLayout;

+

+@Push

+public class MakeComponentVisibleWithPush extends AbstractTestUI {

+

+    private VerticalLayout rootLayout;

+    private Grid<Person> grid;

+    private SearchThread searchThread;

+    private List<Person> data;

+

+    @Override

+    protected void setup(VaadinRequest request) {

+        data = new ArrayList<>();

+

+        rootLayout = new VerticalLayout();

+        setContent(rootLayout);

+

+        grid = new Grid<Person>();

+        grid.addColumn(Person::getName);

+        grid.setVisible(false);

+        rootLayout.addComponent(grid);

+

+        Button doUpdateButton = new Button("Do Update", event -> {

+            try {

+                doUpdate();

+            } catch (InterruptedException e) {

+            }

+        });

+

+        rootLayout.addComponent(doUpdateButton);

+    }

+

+    private void doUpdate() throws InterruptedException {

+

+        cancelSuggestThread();

+

+        grid.setVisible(false);

+        grid.setItems(data);

+

+        UI ui = UI.getCurrent();

+        searchThread = new SearchThread(ui);

+        searchThread.start();

+

+    }

+

+    class SearchThread extends Thread {

+        private UI ui;

+

+        public SearchThread(UI ui) {

+            this.ui = ui;

+        }

+

+        @Override

+        public void run() {

+            data = new ArrayList<Person>(data);

+            data.add(new Person("Person " + (data.size() + 1)));

+

+            if (!searchThread.isInterrupted()) {

+                ui.access(() -> {

+                    grid.setItems(data);

+                    grid.setVisible(true);

+                });

+            }

+        }

+

+    }

+

+    private void cancelSuggestThread() {

+

+        if ((searchThread != null) && !searchThread.isInterrupted()) {

+            searchThread.interrupt();

+            searchThread = null;

+        }

+    }

+

+    class Person {

+

+        private String name;

+

+        public Person(String name) {

+            this.name = name;

+        }

+

+        public String getName() {

+            return name;

+        }

+

+        public void setName(String name) {

+            this.name = name;

+        }

+    }

+

+}

diff --git a/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java b/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java
index 80eb0735da..64b999b7d5 100644
--- a/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/application/MissingHierarchyDetectionTest.java
@@ -44,11 +44,6 @@ public class MissingHierarchyDetectionTest extends SingleBrowserTest {
         ButtonElement toggleImproperly = $(ButtonElement.class)
                 .caption("Toggle improperly").first();
         toggleImproperly.click();
-
-        $(ButtonElement.class).caption("Check for errors").first().click();
-        Assert.assertTrue(
-                "No error was logged for the missing hierarchy change event",
-                getLogRow(0).contains(
-                        "is no longer visible to the client, but no corresponding hierarchy change was sent."));
+        assertSystemNotification();
     }
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java b/uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java
new file mode 100644
index 0000000000..80bedff193
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/push/MakeComponentVisibleWithPushTest.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2000-2016 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.push;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class MakeComponentVisibleWithPushTest extends SingleBrowserTest {
+
+    @Test
+    public void showingHiddenComponentByPushWorks() {
+        setDebug(true);
+        openTestURL();
+
+        $(ButtonElement.class).first().click();
+        Assert.assertEquals("Unexpected row count", 1,
+                $(GridElement.class).first().getRowCount());
+        $(ButtonElement.class).first().click();
+        Assert.assertEquals("Unexpected row count", 2,
+                $(GridElement.class).first().getRowCount());
+
+        assertNoErrorNotifications();
+    }
+}