refactor($http) Use onload/onerror/onabort instead of onreadystatechange

Author: realityking

src/ng/httpBackend.js

@@ -27,11 +27,8 @@ function $HttpBackendProvider() {
 }
 
 function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) {
-  var ABORTED = -1;
-
   // TODO(vojta): fix the signature
   return function(method, url, post, callback, headers, timeout, withCredentials, responseType) {
-    var status;
     $browser.$$incOutstandingRequestCount();
     url = url || $browser.url();
 
@@ -58,44 +55,39 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
         }
       });
 
-      // 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
-      xhr.onreadystatechange = function() {
-        // onreadystatechange might get called multiple times with readyState === 4 on mobile webkit caused by
-        // xhrs that are resolved while the app is in the background (see #5426).
-        // since calling completeRequest sets the `xhr` variable to null, we just check if it's not null before
-        // continuing
-        //
-        // we can't set xhr.onreadystatechange to undefined or delete it because that breaks IE8 (method=PATCH) and
-        // Safari respectively.
-        if (xhr && xhr.readyState == 4) {
-          var responseHeaders = null,
-              response = null,
-              statusText = '';
-
-          if(status !== ABORTED) {
-            responseHeaders = xhr.getAllResponseHeaders();
-
-            // responseText is the old-school way of retrieving response (supported by IE8 & 9)
-            // response/responseType properties were introduced in XHR Level2 spec (supported by IE10)
-            response = ('response' in xhr) ? xhr.response : xhr.responseText;
-          }
+      xhr.onload = function requestComplete() {
+        var statusText = xhr.statusText || '';
 
-          // Accessing statusText on an aborted xhr object will
-          // throw an 'c00c023f error' in IE9 and lower, don't touch it.
-          if (!(status === ABORTED && msie < 10)) {
-            statusText = xhr.statusText;
-          }
+        // responseText is the old-school way of retrieving response (supported by IE8 & 9)
+        // response/responseType properties were introduced in XHR Level2 spec (supported by IE10)
+        var response = ('response' in xhr) ? xhr.response : xhr.responseText;
 
-          completeRequest(callback,
-              status || xhr.status,
-              response,
-              responseHeaders,
-              statusText);
+        // normalize IE9 bug (http://bugs.jquery.com/ticket/1450)
+        var status = xhr.status === 1223 ? 204 : xhr.status;
+
+        // fix status code when it is 0 (0 status is undocumented).
+        // Occurs when accessing file resources or on Android 4.1 stock browser
+        // while retrieving files from application cache.
+        if (status === 0) {
+          status = response ? 200 : urlResolve(url).protocol == 'file' ? 404 : 0;
         }
+
+        completeRequest(callback,
+            status,
+            response,
+            xhr.getAllResponseHeaders(),
+            statusText);
       };
 
+      var requestError = function () {
+        // The response is always empty
+        // See https://xhr.spec.whatwg.org/#request-error-steps and https://fetch.spec.whatwg.org/#concept-network-error
+        completeRequest(callback, -1, null, null, '');
+      };
+
+      xhr.onerror = requestError;
+      xhr.onabort = requestError;
+
       if (withCredentials) {
         xhr.withCredentials = true;
       }
@@ -128,7 +120,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
 
 
     function timeoutRequest() {
-      status = ABORTED;
       jsonpDone && jsonpDone();
       xhr && xhr.abort();
     }
@@ -138,17 +129,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
       timeoutId && $browserDefer.cancel(timeoutId);
       jsonpDone = xhr = null;
 
-      // fix status code when it is 0 (0 status is undocumented).
-      // Occurs when accessing file resources or on Android 4.1 stock browser
-      // while retrieving files from application cache.
-      if (status === 0) {
-        status = response ? 200 : urlResolve(url).protocol == 'file' ? 404 : 0;
-      }
-
-      // normalize IE bug (http://bugs.jquery.com/ticket/1450)
-      status = status === 1223 ? 204 : status;
-      statusText = statusText || '';
-
       callback(status, response, headersString, statusText);
       $browser.$$completeOutstandingRequest(noop);
     }

test/ng/httpBackendSpec.js

@@ -87,8 +87,7 @@ describe('$httpBackend', function() {
     $backend('GET', '/some-url', null, callback);
     xhr = MockXhr.$$lastInstance;
     xhr.statusText = 'OK';
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onload();
     expect(callback).toHaveBeenCalledOnce();
   });
 
@@ -105,9 +104,8 @@ describe('$httpBackend', function() {
     expect(xhr.abort).toHaveBeenCalledOnce();
 
     xhr.status = 0;
-    xhr.readyState = 4;
     xhr.statusText = 'OK';
-    xhr.onreadystatechange();
+    xhr.onload();
     expect(callback).toHaveBeenCalledOnce();
   });
 
@@ -118,8 +116,7 @@ describe('$httpBackend', function() {
 
     $backend('GET', '/some-url', null, callback);
     xhr = MockXhr.$$lastInstance;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onload();
     expect(callback).toHaveBeenCalledOnce();
   });
 
@@ -133,23 +130,7 @@ describe('$httpBackend', function() {
     xhr = MockXhr.$$lastInstance;
 
     xhr.status = 1223;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
-
-    expect(callback).toHaveBeenCalledOnce();
-  });
-
-  // onreadystatechange might by called multiple times
-  // with readyState === 4 on mobile webkit caused by
-  // xhrs that are resolved while the app is in the background (see #5426).
-  it('should not process onreadystatechange callback with readyState == 4 more than once', function() {
-    $backend('GET', 'URL', null, callback);
-    xhr = MockXhr.$$lastInstance;
-
-    xhr.status = 200;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
-    xhr.onreadystatechange();
+    xhr.onload();
 
     expect(callback).toHaveBeenCalledOnce();
   });
@@ -195,8 +176,7 @@ describe('$httpBackend', function() {
     expect(xhr.abort).toHaveBeenCalledOnce();
 
     xhr.status = 0;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onabort();
     expect(callback).toHaveBeenCalledOnce();
   });
 
@@ -215,8 +195,7 @@ describe('$httpBackend', function() {
     expect(xhr.abort).toHaveBeenCalledOnce();
 
     xhr.status = 0;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onabort();
     expect(callback).toHaveBeenCalledOnce();
   });
 
@@ -234,8 +213,7 @@ describe('$httpBackend', function() {
     expect(xhr.abort).toHaveBeenCalledOnce();
 
     xhr.status = 0;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onabort();
     expect(callback).toHaveBeenCalledOnce();
   }));
 
@@ -250,8 +228,7 @@ describe('$httpBackend', function() {
     spyOn(xhr, 'abort');
 
     xhr.status = 200;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onload();
     expect(callback).toHaveBeenCalledOnce();
 
     $timeout.flush();
@@ -271,42 +248,14 @@ describe('$httpBackend', function() {
     expect(fakeTimeout.delays[0]).toBe(2000);
 
     xhr.status = 200;
-    xhr.readyState = 4;
-    xhr.onreadystatechange();
+    xhr.onload();
     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() {
-      xhr = this;
-      this.open = this.setRequestHeader = noop;
-
-      this.send = function() {
-        this.status = 200;
-        this.responseText = 'response';
-        this.readyState = 4;
-        this.onreadystatechange();
-      };
-
-      this.getAllResponseHeaders = valueFn('');
-    }
-
-    callback.andCallFake(function(status, response) {
-      expect(status).toBe(200);
-      expect(response).toBe('response');
-    });
-
-    $backend = createHttpBackend($browser, function() { return new SyncXhr(); });
-    $backend('GET', '/url', null, callback);
-    expect(callback).toHaveBeenCalledOnce();
-  });
-
-
   it('should set withCredentials', function() {
     $backend('GET', '/some.url', null, callback, {}, null, true);
     expect(MockXhr.$$lastInstance.withCredentials).toBe(true);
@@ -326,8 +275,7 @@ describe('$httpBackend', function() {
       });
 
       xhrInstance.response = {some: 'object'};
-      xhrInstance.readyState = 4;
-      xhrInstance.onreadystatechange();
+      xhrInstance.onload();
 
       expect(callback).toHaveBeenCalledOnce();
     });
@@ -347,8 +295,7 @@ describe('$httpBackend', function() {
       });
 
       xhrInstance.responseText = responseText;
-      xhrInstance.readyState = 4;
-      xhrInstance.onreadystatechange();
+      xhrInstance.onload();
 
       expect(callback).toHaveBeenCalledOnce();
     });
@@ -436,8 +383,7 @@ describe('$httpBackend', function() {
       xhr = MockXhr.$$lastInstance;
       xhr.status = status;
       xhr.responseText = content;
-      xhr.readyState = 4;
-      xhr.onreadystatechange();
+      xhr.onload();
     }