Handle canceled or ignored XHR requests after beforeunload (#8891)

* Not automatically testable as it depends exact timings

Change-Id: I53bccce675520ab2c7bb007766546b4cd3fe6e53

2 files changed, 200 insertions, 4 deletions

diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java
index c8206f96f6..23906f0e02 100644
--- a/client/src/com/vaadin/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/client/ApplicationConnection.java
@@ -56,6 +56,9 @@ import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Element;
 import com.google.gwt.user.client.Timer;
+import com.google.gwt.user.client.Window;
+import com.google.gwt.user.client.Window.ClosingEvent;
+import com.google.gwt.user.client.Window.ClosingHandler;
 import com.google.gwt.user.client.ui.HasWidgets;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.ApplicationConfiguration.ErrorMessage;
@@ -170,6 +173,23 @@ public class ApplicationConnection {
 
     private boolean hasActiveRequest = false;
 
+    /**
+     * Some browsers cancel pending XHR requests when a request that might
+     * navigate away from the page starts (indicated by a beforeunload event).
+     * In that case, we should just send the request again without displaying
+     * any error.
+     */
+    private boolean retryCanceledActiveRequest = false;
+
+    /**
+     * Webkit will ignore outgoing requests while waiting for a response to a
+     * navigation event (indicated by a beforeunload event). When this happens,
+     * we should keep trying to send the request every now and then until there
+     * is a response or until it throws an exception saying that it is already
+     * being sent.
+     */
+    private boolean webkitMaybeIgnoringRequests = false;
+
     protected boolean cssLoaded = false;
 
     /** Parameters for this application connection loaded from the web-page */
@@ -378,6 +398,21 @@ public class ApplicationConnection {
         showLoadingIndicator();
 
         scheduleHeartbeat();
+
+        Window.addWindowClosingHandler(new ClosingHandler() {
+            @Override
+            public void onWindowClosing(ClosingEvent event) {
+                /*
+                 * Set some flags to avoid potential problems with XHR requests,
+                 * see javadocs of the flags for details
+                 */
+                if (hasActiveRequest()) {
+                    retryCanceledActiveRequest = true;
+                }
+
+                webkitMaybeIgnoringRequests = true;
+            }
+        });
     }
 
     /**
@@ -668,9 +703,21 @@ public class ApplicationConnection {
 
                     switch (statusCode) {
                     case 0:
-                        handleCommunicationError(
-                                "Invalid status code 0 (server down?)",
-                                statusCode);
+                        if (retryCanceledActiveRequest) {
+                            /*
+                             * Request was most likely canceled because the
+                             * browser is maybe navigating away from the page.
+                             * Just send the request again without displaying
+                             * any error in case the navigation isn't carried
+                             * through.
+                             */
+                            retryCanceledActiveRequest = false;
+                            doUidlRequest(uri, payload, synchronous);
+                        } else {
+                            handleCommunicationError(
+                                    "Invalid status code 0 (server down?)",
+                                    statusCode);
+                        }
                         return;
 
                     case 401:
@@ -814,9 +861,38 @@ public class ApplicationConnection {
         rb.setRequestData(payload);
         rb.setCallback(requestCallback);
 
-        rb.send();
+        final Request request = rb.send();
+        if (webkitMaybeIgnoringRequests && BrowserInfo.get().isWebkit()) {
+            final int retryTimeout = 250;
+            new Timer() {
+                @Override
+                public void run() {
+                    // Use native js to access private field in Request
+                    if (resendRequest(request) && webkitMaybeIgnoringRequests) {
+                        // Schedule retry if still needed
+                        schedule(retryTimeout);
+                    }
+                }
+            }.schedule(retryTimeout);
+        }
     }
 
+    private static native boolean resendRequest(Request request)
+    /*-{
+        var xhr = request.@com.google.gwt.http.client.Request::xmlHttpRequest
+        if (xhr.readyState != 1) {
+            // Progressed to some other readyState -> no longer blocked
+            return false;
+        }
+        try {
+            xhr.send();
+            return true;
+        } catch (e) {
+            // send throws exception if it is running for real
+            return false;
+        }
+    }-*/;
+
     int cssWaits = 0;
 
     /**
@@ -977,6 +1053,10 @@ public class ApplicationConnection {
         // the call. Active requests used to be tracked with an integer counter,
         // so setting it after used to work but not with the #8505 changes.
         hasActiveRequest = false;
+
+        retryCanceledActiveRequest = false;
+        webkitMaybeIgnoringRequests = false;
+
         if (applicationRunning) {
             checkForPendingVariableBursts();
             runPostRequestHooks(configuration.getRootPanelId());
diff --git a/uitest/src/com/vaadin/tests/application/NavigateWithOngoingXHR.java b/uitest/src/com/vaadin/tests/application/NavigateWithOngoingXHR.java
new file mode 100644
index 0000000000..e11a18d895
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/application/NavigateWithOngoingXHR.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2012 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.application;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+import com.vaadin.server.ExternalResource;
+import com.vaadin.server.RequestHandler;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinResponse;
+import com.vaadin.server.VaadinServiceSession;
+import com.vaadin.shared.ui.progressindicator.ProgressIndicatorServerRpc;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Link;
+import com.vaadin.ui.ProgressIndicator;
+
+public class NavigateWithOngoingXHR extends AbstractTestUI {
+    private final RequestHandler slowRequestHandler = new RequestHandler() {
+        @Override
+        public boolean handleRequest(VaadinServiceSession session,
+                VaadinRequest request, VaadinResponse response)
+                throws IOException {
+            if ("/slowRequestHandler".equals(request.getRequestPathInfo())) {
+                // Make the navigation request last longer to keep the
+                // communication error visible
+                // System.out.println("Got slow content request");
+                try {
+                    Thread.sleep(5000);
+                } catch (InterruptedException e) {
+                    e.printStackTrace();
+                }
+
+                if (request.getParameter("download") != null) {
+                    response.setHeader("Content-Disposition", "attachment");
+                }
+
+                response.setContentType("text/plain");
+                PrintWriter writer = response.getWriter();
+                writer.println("Loaded slowly");
+                writer.close();
+
+                // System.out.println("Finished slow content request");
+
+                return true;
+            }
+            return false;
+        }
+    };
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        addComponent(new ProgressIndicator() {
+            {
+                registerRpc(new ProgressIndicatorServerRpc() {
+                    @Override
+                    public void poll() {
+                        // System.out.println("Pausing poll request");
+                        try {
+                            // Make the XHR request last longer to make it
+                            // easier to click the link at the right moment.
+                            Thread.sleep(1000);
+                        } catch (InterruptedException e) {
+                            e.printStackTrace();
+                        }
+                        // System.out.println("Continuing poll request");
+                    }
+                });
+                setPollingInterval(3000);
+            }
+        });
+
+        // Hacky URLs that are might not work in all deployment scenarios
+        addComponent(new Link("Navigate away", new ExternalResource(
+                "slowRequestHandler")));
+        addComponent(new Link("Start download", new ExternalResource(
+                "slowRequestHandler?download")));
+    }
+
+    @Override
+    public void attach() {
+        super.attach();
+        getSession().addRequestHandler(slowRequestHandler);
+    }
+
+    @Override
+    public void detach() {
+        getSession().removeRequestHandler(slowRequestHandler);
+        super.detach();
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Navigating away from a Vaadin page while there's an ongoing XHR request should not cause a communication error to be displayed";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(8891);
+    }
+
+}