From cda7b71146f6748116ad5bbc9050ee7e79a9ce2b Mon Sep 17 00:00:00 2001
From: David Bennett <davbennett@google.com>
Date: Wed, 24 Apr 2013 12:33:08 -0500
Subject: [PATCH] feat($httpBackend): add timeout support for JSONP requests

Documentation implies that timeout works for all requests, though it
only works with XHR. To implement:
- Change $httpBackend to set a timeout for JSONP requests which will
immediately resolve the request when fired.
- Cancel the timeout when requests are completed.
---
 src/ng/httpBackend.js      | 24 ++++++++++-------
 test/ng/httpBackendSpec.js | 53 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js
index a3f6cdc01eb8..69ac597648be 100644
--- a/src/ng/httpBackend.js
+++ b/src/ng/httpBackend.js
@@ -33,6 +33,7 @@ function $HttpBackendProvider() {
 function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, locationProtocol) {
   // TODO(vojta): fix the signature
   return function(method, url, post, callback, headers, timeout, withCredentials, responseType) {
+    var status;
     $browser.$$incOutstandingRequestCount();
     url = url || $browser.url();
 
@@ -42,12 +43,12 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
         callbacks[callbackId].data = data;
       };
 
-      jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
+      var jsonpDone = jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
           function() {
         if (callbacks[callbackId].data) {
           completeRequest(callback, 200, callbacks[callbackId].data);
         } else {
-          completeRequest(callback, -2);
+          completeRequest(callback, status || -2);
         }
         delete callbacks[callbackId];
       });
@@ -58,8 +59,6 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
         if (value) xhr.setRequestHeader(key, value);
       });
 
-      var status;
-
       // In IE6 and 7, this might be called synchronously when xhr.send below is called and the
       // response is in the cache. the promise api will ensure that to the app code the api is
       // always async
@@ -105,13 +104,14 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
       }
 
       xhr.send(post || '');
+    }
 
-      if (timeout > 0) {
-        $browserDefer(function() {
-          status = -1;
-          xhr.abort();
-        }, timeout);
-      }
+    if (timeout > 0) {
+      var timeoutId = $browserDefer(function() {
+        status = -1;
+        jsonpDone && jsonpDone();
+        xhr && xhr.abort();
+      }, timeout);
     }
 
 
@@ -119,6 +119,9 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
       // URL_MATCH is defined in src/service/location.js
       var protocol = (url.match(SERVER_MATCH) || ['', locationProtocol])[1];
 
+      // cancel timeout
+      timeoutId && $browserDefer.cancel(timeoutId);
+
       // fix status code for file protocol (it's always 0)
       status = (protocol == 'file') ? (response ? 200 : 404) : status;
 
@@ -152,5 +155,6 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument,
     }
 
     rawDocument.body.appendChild(script);
+    return doneWrapper;
   }
 }
diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js
index a7935a7c0cc6..da4fed168de7 100644
--- a/test/ng/httpBackendSpec.js
+++ b/test/ng/httpBackendSpec.js
@@ -1,21 +1,36 @@
 describe('$httpBackend', function() {
 
   var $backend, $browser, callbacks,
-      xhr, fakeDocument, callback;
+      xhr, fakeDocument, callback,
+      fakeTimeoutId = 0;
 
   // TODO(vojta): should be replaced by $defer mock
   function fakeTimeout(fn, delay) {
     fakeTimeout.fns.push(fn);
     fakeTimeout.delays.push(delay);
+    fakeTimeout.ids.push(++fakeTimeoutId);
+    return fakeTimeoutId;
   }
 
   fakeTimeout.fns = [];
   fakeTimeout.delays = [];
+  fakeTimeout.ids = [];
   fakeTimeout.flush = function() {
     var len = fakeTimeout.fns.length;
     fakeTimeout.delays = [];
+    fakeTimeout.ids = [];
     while (len--) fakeTimeout.fns.shift()();
   };
+  fakeTimeout.cancel = function(id) {
+    var i = indexOf(fakeTimeout.ids, id);
+    if (i >= 0) {
+      fakeTimeout.fns.splice(i, 1);
+      fakeTimeout.delays.splice(i, 1);
+      fakeTimeout.ids.splice(i, 1);
+      return true;
+    }
+    return false;
+  };
 
 
   beforeEach(inject(function($injector) {
@@ -102,6 +117,27 @@ describe('$httpBackend', function() {
   });
 
 
+  it('should cancel timeout on completion', function() {
+    callback.andCallFake(function(status, response) {
+      expect(status).toBe(200);
+    });
+
+    $backend('GET', '/url', null, callback, {}, 2000);
+    xhr = MockXhr.$$lastInstance;
+    spyOn(xhr, 'abort');
+
+    expect(fakeTimeout.delays[0]).toBe(2000);
+
+    xhr.status = 200;
+    xhr.readyState = 4;
+    xhr.onreadystatechange();
+    expect(callback).toHaveBeenCalledOnce();
+
+    expect(fakeTimeout.delays.length).toBe(0);
+    expect(xhr.abort).not.toHaveBeenCalled();
+  });
+
+
   it('should register onreadystatechange callback before sending', function() {
     // send() in IE6, IE7 is sync when serving from cache
     function SyncXhr() {
@@ -239,6 +275,21 @@ describe('$httpBackend', function() {
     });
 
 
+    it('should abort request on timeout', function() {
+      callback.andCallFake(function(status, response) {
+        expect(status).toBe(-1);
+      });
+
+      $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback, null, 2000);
+      expect(fakeDocument.$$scripts.length).toBe(1);
+      expect(fakeTimeout.delays[0]).toBe(2000);
+
+      fakeTimeout.flush();
+      expect(fakeDocument.$$scripts.length).toBe(0);
+      expect(callback).toHaveBeenCalledOnce();
+    });
+
+
     // TODO(vojta): test whether it fires "async-start"
     // TODO(vojta): test whether it fires "async-end" on both success and error
   });