Skip to content
Permalink
Browse files

Fixes #8205. Mitigates memory usage by recycling jsonp callback names…

… the safest possible way (no kittens were harmed in the making of this). Doesn't even try to delete window properties (would necessitate a try/catch for IE which makes the cost in size prohibitive). Unit tests added.
  • Loading branch information...
jaubourg committed Apr 20, 2012
1 parent 3e6f94c commit 8ebb2f4793d2c464888b5f0dba97fb5c9d0c9541
Showing with 55 additions and 4 deletions.
  1. +12 −2 src/ajax/jsonp.js
  2. +43 −2 test/unit/ajax.js
@@ -1,13 +1,16 @@
(function( jQuery ) {

var jsc = jQuery.now(),
jsre = /(\=)\?(&|$)|\?\?/i;
jsre = /(\=)\?(&|$)|\?\?/i,
jscallbacks = [];

// Default jsonp settings
jQuery.ajaxSetup({
jsonp: "callback",
jsonpCallback: function() {
return jQuery.expando + "_" + ( jsc++ );
var callback = jscallbacks.pop() || ( jQuery.expando + "_" + ( jsc++ ) );
this[ callback ] = true;
return callback;
}
});

@@ -53,6 +56,13 @@ jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, jqXHR ) {
jqXHR.always(function() {
// Set callback back to previous value
window[ jsonpCallback ] = previous;
// Save back as free
if ( s[ jsonpCallback ] ) {
// make sure that re-using the options doesn't screw things around
s.jsonpCallback = originalSettings.jsonpCallback;
// save the callback name for future use
jscallbacks.push( jsonpCallback );
}
// Call if it was a function and we have a response
if ( responseContainer && jQuery.isFunction( previous ) ) {
window[ jsonpCallback ]( responseContainer[ 0 ] );
@@ -1309,10 +1309,10 @@ test("jQuery.getScript(String, Function) - no callback", function() {
jQuery.each( [ "Same Domain", "Cross Domain" ] , function( crossDomain , label ) {

test("jQuery.ajax() - JSONP, " + label, function() {
expect(20);
expect(24);

var count = 0;
function plus(){ if ( ++count == 18 ) start(); }
function plus(){ if ( ++count == 20 ) start(); }

stop();

@@ -1330,6 +1330,25 @@ jQuery.each( [ "Same Domain", "Cross Domain" ] , function( crossDomain , label )
}
});

jQuery.ajax({
url: "data/jsonp.php",
dataType: "jsonp",
crossDomain: crossDomain,
success: function(data){
ok( data.data, ( this.alreadyDone ? "this re-used" : "first request" ) + ": JSON results returned (GET, no callback)" );
if ( !this.alreadyDone ) {
this.alreadyDone = true;
jQuery.ajax( this );
} else {
plus();
}
},
error: function(data){
ok( false, "Ajax error JSON (GET, no callback)" );
plus();
}
});

jQuery.ajax({
url: "data/jsonp.php?callback=?",
dataType: "jsonp",
@@ -1564,6 +1583,28 @@ jQuery.each( [ "Same Domain", "Cross Domain" ] , function( crossDomain , label )
}
});

//#8205
jQuery.ajax({
url: "data/jsonp.php",
dataType: "jsonp",
crossDomain: crossDomain,
beforeSend: function() {
this.callback = this.jsonpCallback;
}
}).pipe(function() {
var previous = this;
strictEqual( previous.jsonpCallback, undefined, "jsonpCallback option is set back to default in callbacks" );
jQuery.ajax({
url: "data/jsonp.php",
dataType: "jsonp",
crossDomain: crossDomain,
beforeSend: function() {
strictEqual( this.jsonpCallback, previous.callback, "JSONP callback name is re-used" );
return false;
}
});
}).always( plus );

});
});

1 comment on commit 8ebb2f4

@rwaldron

This comment has been minimized.

Copy link
Member

commented on 8ebb2f4 Apr 20, 2012

I dig this :)

Please sign in to comment.
You can’t perform that action at this time.