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

5 files changed, 151 insertions, 49 deletions

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();
+    }
+}