Check for optimizations when looking for missing updates (#18317)

A recently merged patch leaves out information from hierarchyInfo for
empty connectors with state changes. This must be taken into account
when looking for disappeared connectors that do not cause any hierarchy
change to be sent.

Change-Id: I9ae7150341a83798141d0a2806ee81cafe7c2f9a

server/src/com/vaadin/server/communication/ConnectorHierarchyWriter.java

@@ -91,16 +91,19 @@ public void write(UI ui, Writer writer, Set<String> stateUpdateConnectors)
         }
         // Dummy assert just for conditionally storing away data that will be
         // used by the real assert later on
-        assert storeSentHierarchy(hierarchyInfo);
+        assert storeSentHierarchy(hierarchyInfo, stateUpdateConnectors);
 
         writer.write(JsonUtil.stringify(hierarchyInfo));
     }
 
-    private boolean storeSentHierarchy(JsonObject hierarchyInfo) {
+    private boolean storeSentHierarchy(JsonObject hierarchyInfo,
+            Set<String> stateUpdateConnectors) {
         VaadinRequest request = VaadinService.getCurrentRequest();
         if (request != null) {
             request.setAttribute(ConnectorHierarchyWriter.class.getName()
                     + ".hierarchyInfo", hierarchyInfo);
+            request.setAttribute(ConnectorHierarchyWriter.class.getName()
+                    + ".stateUpdateConnectors", stateUpdateConnectors);
         }
 
         // Always true, we're just setting up for another assert

server/src/com/vaadin/ui/ConnectorTracker.java

@@ -368,7 +368,34 @@ private boolean isRemovalSentToClient(ClientConnector connector) {
             }
 
             if (!hierachyInfo.hasKey(firstVisibleParent.getConnectorId())) {
-                return false;
+                /*
+                 * No hierarchy change about to be sent, but this might be
+                 * because of an optimization that omits explicit hierarchy
+                 * changes for empty connectors that have state changes.
+                 */
+                if (hasVisibleChild(firstVisibleParent)) {
+                    // Not the optimization case if the parent has visible
+                    // children
+                    return false;
+                }
+
+                attributeName = ConnectorHierarchyWriter.class.getName()
+                        + ".stateUpdateConnectors";
+                Object stateUpdateConnectorsObj = request
+                        .getAttribute(attributeName);
+                if (stateUpdateConnectorsObj instanceof Set<?>) {
+                    Set<?> stateUpdateConnectors = (Set<?>) stateUpdateConnectorsObj;
+                    if (!stateUpdateConnectors.contains(firstVisibleParent
+                            .getConnectorId())) {
+                        // Not the optimization case if the parent is not marked
+                        // as dirty
+                        return false;
+                    }
+                } else {
+                    getLogger().warning(
+                            "Request attribute " + attributeName
+                                    + " is not a Set");
+                }
             }
         } else {
             getLogger().warning(
@@ -379,6 +406,18 @@ private boolean isRemovalSentToClient(ClientConnector connector) {
         return true;
     }
 
+    private static boolean hasVisibleChild(ClientConnector parent) {
+        Iterator<? extends ClientConnector> iterator = AbstractClientConnector
+                .getAllChildrenIterable(parent).iterator();
+        while (iterator.hasNext()) {
+            ClientConnector child = iterator.next();
+            if (LegacyCommunicationManager.isConnectorVisibleToClient(child)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private ClientConnector findFirstVisibleParent(ClientConnector connector) {
         while (connector != null) {
             connector = connector.getParent();

uitest/src/com/vaadin/tests/application/MissingHierarchyDetection.java

@@ -27,7 +27,10 @@
 public class MissingHierarchyDetection extends AbstractTestUI {
 
     private boolean isChildRendered = true;
-    private BrokenCssLayout layout = new BrokenCssLayout();
+    private BrokenCssLayout brokenLayout = new BrokenCssLayout();
+
+    private CssLayout normalLayout = new CssLayout(new Label(
+            "Normal layout child"));
 
     public class BrokenCssLayout extends CssLayout implements SelectiveRenderer {
         public BrokenCssLayout() {
@@ -45,7 +48,8 @@ public boolean isRendered(Component childComponent) {
 
     @Override
     protected void setup(VaadinRequest request) {
-        addComponent(layout);
+        addComponent(brokenLayout);
+        addComponent(normalLayout);
         addComponent(new Button("Toggle properly", new Button.ClickListener() {
             @Override
             public void buttonClick(ClickEvent event) {
@@ -64,7 +68,12 @@ public void buttonClick(ClickEvent event) {
     private void toggle(boolean properly) {
         isChildRendered = !isChildRendered;
         if (properly) {
-            layout.markAsDirtyRecursive();
+            brokenLayout.markAsDirtyRecursive();
         }
+
+        normalLayout.getComponent(0).setVisible(isChildRendered);
+        // Must also have a state change of the layout to trigger special case
+        // related to optimizations
+        normalLayout.setCaption("With child: " + isChildRendered);
     }
 }