From 008dd4cf731522543c80ea81c0d5dca35f85c90b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Thu, 15 Nov 2012 09:40:42 +0200 Subject: [PATCH] Handle canceled or ignored XHR requests after beforeunload (#8891) * Not automatically testable as it depends exact timings Change-Id: I53bccce675520ab2c7bb007766546b4cd3fe6e53 --- .../vaadin/client/ApplicationConnection.java | 88 ++++++++++++- .../application/NavigateWithOngoingXHR.java | 116 ++++++++++++++++++ 2 files changed, 200 insertions(+), 4 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/application/NavigateWithOngoingXHR.java 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); + } + +} -- 2.39.5