Don't actively disconnect when the client already has disconnected (#15330)

Change-Id: I26e53f6b07eaccc785bda547e454fa185ad952df

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

@@ -275,12 +275,10 @@ public class AtmospherePushConnection implements PushConnection {
         assert isConnected();
 
         if (resource.isResumed()) {
-            // Calling disconnect may end up invoking it again via
-            // resource.resume and PushHandler.onResume. Bail out here if
-            // the resource is already resumed; this is a bit hacky and should
-            // be implemented in a better way in 7.2.
-            resource = null;
-            state = State.DISCONNECTED;
+            // This can happen for long polling because of
+            // http://dev.vaadin.com/ticket/16919
+            // Once that is fixed, this should never happen
+            connectionLost();
             return;
         }
 
@@ -307,8 +305,23 @@ public class AtmospherePushConnection implements PushConnection {
             getLogger()
                     .log(Level.INFO, "Error when closing push connection", e);
         }
+        connectionLost();
+    }
+
+    /**
+     * Called when the connection to the client has been lost.
+     * 
+     * @since
+     */
+    public void connectionLost() {
         resource = null;
-        state = State.DISCONNECTED;
+        if (state == State.CONNECTED) {
+            // Guard against connectionLost being (incorrectly) called when
+            // state is PUSH_PENDING or RESPONSE_PENDING
+            // (http://dev.vaadin.com/ticket/16919)
+            state = State.DISCONNECTED;
+        }
+
     }
 
     /**

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

@@ -64,7 +64,7 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter {
                 throws IOException {
             super.onStateChange(event);
             if (event.isCancelled() || event.isResumedOnTimeout()) {
-                disconnect(event);
+                connectionLost(event);
             }
         }
 
@@ -329,17 +329,17 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter {
     public void onDisconnect(AtmosphereResourceEvent event) {
         // Log event on trace level
         super.onDisconnect(event);
-        disconnect(event);
+        connectionLost(event);
     }
 
     @Override
     public void onThrowable(AtmosphereResourceEvent event) {
         getLogger().log(Level.SEVERE, "Exception in push connection",
                 event.throwable());
-        disconnect(event);
+        connectionLost(event);
     }
 
-    private void disconnect(AtmosphereResourceEvent event) {
+    private void connectionLost(AtmosphereResourceEvent event) {
         // We don't want to use callWithUi here, as it assumes there's a client
         // request active and does requestStart and requestEnd among other
         // things.
@@ -425,12 +425,8 @@ public class PushHandler extends AtmosphereResourceEventListenerAdapter {
                                     "Connection unexpectedly closed for resource {0} with transport {1}",
                                     new Object[] { id, resource.transport() });
                 }
-                if (pushConnection.isConnected()) {
-                    // disconnect() assumes the push connection is connected but
-                    // this method can currently be called more than once during
-                    // disconnect, depending on the situation
-                    pushConnection.disconnect();
-                }
+
+                pushConnection.connectionLost();
             }
 
         } catch (final Exception e) {