Fix JSONP collisions when using multiple modules on the same page.
Review at http://gwt-code-reviews.appspot.com/130820/show
Issue: 4386
Patch by: jat
Review by: rice
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@7469 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/jsonp/client/JsonpRequest.java b/user/src/com/google/gwt/jsonp/client/JsonpRequest.java
index ad8d5d4..c199339 100644
--- a/user/src/com/google/gwt/jsonp/client/JsonpRequest.java
+++ b/user/src/com/google/gwt/jsonp/client/JsonpRequest.java
@@ -32,32 +32,47 @@
public class JsonpRequest<T> {
/**
- * Each request will be assigned a new id.
+ * A global JS variable that holds the next index to use.
*/
- private static int callbackCounter = 0;
+ @SuppressWarnings("unused") // accessed from JSNI
+ private static final String CALLBACKS_COUNTER_NAME = "__gwt_jsonp_counter__";
/**
- * __gwt_jsonp__ is a global object that contains callbacks of pending requests.
+ * A global JS object that contains callbacks of pending requests.
*/
private static final String CALLBACKS_NAME = "__gwt_jsonp__";
- private static final JavaScriptObject CALLBACKS = createCallbacksObject(CALLBACKS_NAME);
+ private static final JavaScriptObject CALLBACKS = getOrCreateCallbacksObject();
/**
- * Creates a global object to store callbacks of pending requests.
- *
- * @param name The name of the global object.
- * @return The created object.
+ * @return the next ID to use, incrementing the global counter.
*/
- private static native JavaScriptObject createCallbacksObject(String name) /*-{
- return $wnd[name] = new Object();
+ private static native int getAndIncrementCallbackCounter() /*-{
+ var name = @com.google.gwt.jsonp.client.JsonpRequest::CALLBACKS_NAME;
+ var ctr = @com.google.gwt.jsonp.client.JsonpRequest::CALLBACKS_COUNTER_NAME;
+ return $wnd[name][ctr]++;
}-*/;
private static Node getHeadElement() {
return Document.get().getElementsByTagName("head").getItem(0);
}
+ /**
+ * @return a global object to store callbacks of pending requests, creating
+ * it if it doesn't exist.
+ */
+ private static native JavaScriptObject getOrCreateCallbacksObject() /*-{
+ var name = @com.google.gwt.jsonp.client.JsonpRequest::CALLBACKS_NAME;
+ if (!$wnd[name]) {
+ $wnd[name] = new Object();
+ $wnd[name]
+ [@com.google.gwt.jsonp.client.JsonpRequest::CALLBACKS_COUNTER_NAME]
+ = 0;
+ }
+ return $wnd[name];
+ }-*/;
+
private static String nextCallbackId() {
- return "I" + (callbackCounter++);
+ return "I" + getAndIncrementCallbackCounter();
}
private final String callbackId;
@@ -120,6 +135,16 @@
return timeout;
}
+ @Override
+ public String toString() {
+ return "JsonpRequest(id=" + callbackId + ")";
+ }
+
+ // @VisibleForTesting
+ String getCallbackId() {
+ return callbackId;
+ }
+
/**
* Sends a request using the JSONP mechanism.
*
diff --git a/user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java b/user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java
index 8022eb7..0d0cc69 100644
--- a/user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java
+++ b/user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java
@@ -16,7 +16,10 @@
package com.google.gwt.jsonp.client;
import com.google.gwt.core.client.GWT;
+import com.google.gwt.junit.DoNotRunWith;
+import com.google.gwt.junit.Platform;
import com.google.gwt.junit.client.GWTTestCase;
+import com.google.gwt.user.client.Timer;
import com.google.gwt.user.client.rpc.AsyncCallback;
/**
@@ -25,16 +28,6 @@
public class JsonpRequestTest extends GWTTestCase {
/**
- * The maximum amount of time to wait for a response in milliseconds.
- */
- private static final int RESPONSE_DELAY = 2500;
-
- /**
- * The current Id of the async callback.
- */
- protected static int currentId;
-
- /**
* Checks that an error is received.
*/
private class AssertFailureCallback<T> implements AsyncCallback<T> {
@@ -42,19 +35,19 @@
private int id;
public AssertFailureCallback(String expectedMessage) {
- id = ++currentId;
+ id = currentTestId;
this.expectedMessage = expectedMessage;
}
public void onFailure(Throwable throwable) {
- if (id == currentId) {
+ if (id == currentTestId) {
assertEquals(expectedMessage, throwable.getMessage());
finishTest();
}
}
public void onSuccess(T value) {
- if (id == currentId) {
+ if (id == currentTestId) {
fail();
}
}
@@ -64,24 +57,34 @@
* Checks that the received value is as expected.
*/
private class AssertSuccessCallback<T> implements AsyncCallback<T> {
- private T expectedValue;
- private int id;
+ private final T expectedValue;
+ private final int id;
+ private final Counter counter;
private AssertSuccessCallback(T expectedValue) {
- this.id = ++currentId;
+ this(expectedValue, null);
+ }
+
+ private AssertSuccessCallback(T expectedValue, Counter counter) {
+ this.id = currentTestId;
+ this.counter = counter;
this.expectedValue = expectedValue;
}
public void onFailure(Throwable throwable) {
- if (id == currentId) {
+ if (id == currentTestId) {
fail();
}
}
public void onSuccess(T value) {
- if (id == currentId) {
+ if (id == currentTestId) {
assertEquals(expectedValue, value);
- finishTest();
+ if (counter != null) {
+ counter.count();
+ } else {
+ finishTest();
+ }
}
}
}
@@ -100,10 +103,51 @@
}
}
+ /**
+ * A class that counts results and calls finishTest when the right number
+ * have been received.
+ */
+ private class Counter {
+
+ private int count;
+
+ public Counter(int count) {
+ this.count = count;
+ }
+
+ public void count() {
+ if (--count < 0) {
+ fail("Too many results");
+ }
+ if (count == 0) {
+ finishTest();
+ }
+ }
+ }
+
+ /**
+ * The maximum amount of time to wait for a response in milliseconds.
+ */
+ private static final int RESPONSE_DELAY = 2500;
+
+ /**
+ * The ID of the current test.
+ */
+ protected static int currentTestId;
+
private static String echo(String value) {
return GWT.getModuleBaseURL() + "echo?action=SUCCESS&value=" + value;
}
+ private static String echoDelayed(String value) {
+ return echoDelayed(value, 500);
+ }
+
+ private static String echoDelayed(String value, long delayMs) {
+ return GWT.getModuleBaseURL() + "echo?action=SUCCESS&delay=" + delayMs
+ + "&value=" + value;
+ }
+
private static String echoFailure(String error) {
return GWT.getModuleBaseURL() + "echo?action=FAILURE&error=" + error;
}
@@ -131,6 +175,31 @@
Boolean.TRUE));
}
+
+ @DoNotRunWith(Platform.HtmlUnit)
+ // Fails in devmode with HtmlUnit, JS "null" exception
+ public void testCancel() {
+ delayTestFinish(2000);
+ // setup a server request that will delay for 500ms
+ JsonpRequest<String> req = jsonp.requestString(echoDelayed("A", 500),
+ new AssertFailureCallback<String>("A"));
+ // cancel it before it comes back
+ req.cancel();
+ // wait 1s to make sure we don't get a callback
+ new Timer() {
+ @Override
+ public void run() {
+ finishTest();
+ }
+ }.schedule(1000);
+ }
+
+ public void testDelay() {
+ delayTestFinish(RESPONSE_DELAY);
+ JsonpRequest<String> req = jsonp.requestString(echoDelayed("'A'"),
+ new AssertSuccessCallback<String>("A"));
+ }
+
public void testDouble() {
delayTestFinish(RESPONSE_DELAY);
jsonp.requestDouble(echo("123.456"), new AssertSuccessCallback<Double>(
@@ -144,6 +213,20 @@
new AssertFailureCallback<String>("ERROR"));
}
+ @DoNotRunWith(Platform.HtmlUnit)
+ // Hangs indefinitely in devmode with HtmlUnit
+ public void testIds() {
+ delayTestFinish(RESPONSE_DELAY);
+ JsonpRequest<String> reqA = jsonp.requestString(echo("'A'"),
+ new AssertSuccessCallback<String>("A"));
+ JsonpRequest<String> reqB = jsonp.requestString(echo("'B'"),
+ new AssertSuccessCallback<String>("B"));
+ // WARNING: knows the current structure of IDs
+ int idA = Integer.parseInt(reqA.getCallbackId().substring(1));
+ int idB = Integer.parseInt(reqB.getCallbackId().substring(1));
+ assertEquals("Unexpected ID sequence", idA + 1, idB);
+ }
+
public void testInteger() {
delayTestFinish(RESPONSE_DELAY);
jsonp.requestInteger(echo("123"), new AssertSuccessCallback<Integer>(123));
@@ -180,6 +263,19 @@
jsonp.requestString(echo("null"), new AssertSuccessCallback<String>(null));
}
+ @DoNotRunWith(Platform.HtmlUnit)
+ // Hangs indefinitely in devmode with HtmlUnit
+ public void testOverlapped() {
+ delayTestFinish(RESPONSE_DELAY);
+ Counter counter = new Counter(3);
+ JsonpRequest<String> reqA = jsonp.requestString(echoDelayed("'A'", 750),
+ new AssertSuccessCallback<String>("A", counter));
+ JsonpRequest<String> reqB = jsonp.requestString(echoDelayed("'B'", 500),
+ new AssertSuccessCallback<String>("B", counter));
+ JsonpRequest<String> reqC = jsonp.requestString(echo("'C'"),
+ new AssertSuccessCallback<String>("C", counter));
+ }
+
public void testString() {
delayTestFinish(RESPONSE_DELAY);
jsonp.requestString(echo("'Hello'"), new AssertSuccessCallback<String>(
@@ -202,4 +298,9 @@
jsonp = new JsonpRequestBuilder();
jsonp.setTimeout(RESPONSE_DELAY + 500);
}
+
+ @Override
+ protected void gwtTearDown() throws Exception {
+ ++currentTestId;
+ }
}
diff --git a/user/test/com/google/gwt/jsonp/server/EchoServlet.java b/user/test/com/google/gwt/jsonp/server/EchoServlet.java
index 18461dc..7e8cef2 100644
--- a/user/test/com/google/gwt/jsonp/server/EchoServlet.java
+++ b/user/test/com/google/gwt/jsonp/server/EchoServlet.java
@@ -17,7 +17,6 @@
import java.io.IOException;
-import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@@ -47,9 +46,11 @@
@Override
protected void service(HttpServletRequest req, HttpServletResponse res)
- throws ServletException, IOException {
+ throws IOException {
+ res.setContentType("application/javascript");
switch (Action.valueOf(req.getParameter("action"))) {
case SUCCESS: {
+ handleDelay(req);
String callback = req.getParameter("callback");
String value = req.getParameter("value");
if (value == null) {
@@ -61,6 +62,7 @@
}
case FAILURE: {
+ handleDelay(req);
String failureCallback = req.getParameter("failureCallback");
String error = req.getParameter("error");
if (failureCallback != null) {
@@ -80,4 +82,21 @@
// Don't respond anything so that a timeout happens.
}
}
+
+ /**
+ * Handle a delay query parameter if present.
+ *
+ * @param req
+ */
+ private void handleDelay(HttpServletRequest req) {
+ String delayVal = req.getParameter("delay");
+ if (delayVal != null) {
+ int delayMs = Integer.parseInt(delayVal);
+ try {
+ Thread.sleep(delayMs);
+ } catch (InterruptedException e) {
+ log("Interrupted sleep, continuing");
+ }
+ }
+ }
}
