fix($http): properly increment/decrement $browser.outstandingRequestCount

Calling `$http` will not increment `$browser.outstandingRequestCount` until after all (potentially) asynchronous request interceptors have been processed and will decrement it before any (potentially) asynchronous response interceptors have been processed. This can lead to `$browser.notifyWhenNoOutstandingRequests` firing prematurely, which can be problematic in end-to-end tests. This commit fixes this, by synchronizing the increment/decrement operations with `$http`'s internal interceptor promise chain. Fixes #13782 Closes #13862 Closes #14921 BREAKING CHANGE: HTTP requests now update the outstanding request count synchronously. Previously the request count would not have been updated until the request to the server is actually in flight. Now the request count is updated before the async interceptor is called. The new behaviour is correct but it may change the expected behaviour in a small number of e2e test cases where an async request interceptor is being used.
2016-01-27 17:44:47 -06:00
parent 56d456360b
commit 4f6f2bce4a
6 changed files with 93 additions and 5 deletions
@@ -374,8 +374,8 @@ function $HttpProvider() {
   **/
  var interceptorFactories = this.interceptors = [];

-  this.$get = ['$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector',
-      function($httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) {
+  this.$get = ['$browser', '$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector',
+      function($browser, $httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) {

    var defaultCache = $cacheFactory('$http');

@@ -981,7 +981,7 @@ function $HttpProvider() {
      };

      var chain = [serverRequest, undefined];
-      var promise = $q.when(config);
+      var promise = initiateOutstandingRequest(config);

      // apply interceptors
      forEach(reversedInterceptors, function(interceptor) {
@@ -1000,6 +1000,8 @@ function $HttpProvider() {
        promise = promise.then(thenFn, rejectFn);
      }

+      promise.finally(completeOutstandingRequest);
+
      if (useLegacyPromise) {
        promise.success = function(fn) {
          assertArgFn(fn, 'fn');
@@ -1025,6 +1027,15 @@ function $HttpProvider() {

      return promise;

+      function initiateOutstandingRequest(config) {
+        $browser.$$incOutstandingRequestCount();
+        return $q.when(config);
+      }
+
+      function completeOutstandingRequest() {
+        $browser.$$completeOutstandingRequest(noop);
+      }
+
      function transformResponse(response) {
        // make a copy since the response must be cacheable
        var resp = extend({}, response);
@@ -55,7 +55,6 @@ function $HttpBackendProvider() {
 function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) {
  // TODO(vojta): fix the signature
  return function(method, url, post, callback, headers, timeout, withCredentials, responseType, eventHandlers, uploadEventHandlers) {
-    $browser.$$incOutstandingRequestCount();
    url = url || $browser.url();

    if (lowercase(method) === 'jsonp') {
@@ -162,7 +161,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
      jsonpDone = xhr = null;

      callback(status, response, headersString, statusText);
-      $browser.$$completeOutstandingRequest(noop);
    }
  };

@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html ng-app="test">
+  <body>
+    <div ng-controller="TestController">
+      <p>{{text}}</p>
+    </div>
+
+    <script src="angular.js"></script>
+    <script src="script.js"></script>
+  </body>
+</html>
@@ -0,0 +1,28 @@
+angular.
+  module('test', []).
+  config(function($httpProvider) {
+    $httpProvider.interceptors.push(function($q) {
+      return {
+        request: function(config) {
+          return $q(function(resolve) {
+            setTimeout(resolve, 100, config);
+          });
+        },
+        response: function(response) {
+          return $q(function(resolve) {
+            setTimeout(resolve, 100, response);
+          });
+        }
+      };
+    });
+  }).
+  controller('TestController', function($cacheFactory, $http, $scope) {
+    var url = '/some/url';
+
+    var cache = $cacheFactory('test');
+    cache.put(url, 'Hello, world!');
+
+    $http.
+      get(url, {cache: cache}).
+      then(function(response) { $scope.text = response.data; });
+  });
@@ -0,0 +1,9 @@
+describe('$http', function() {
+  beforeEach(function() {
+    loadFixture('http');
+  });
+
+  it('should correctly update the outstanding request count', function() {
+    expect(element(by.binding('text')).getText()).toBe('Hello, world!');
+  });
+});
@@ -1896,6 +1896,37 @@ describe('$http', function() {
        expect(paramSerializer({foo: 'foo', bar: ['bar', 'baz']})).toEqual('bar=bar&bar=baz&foo=foo');
      });
    });
+
+    describe('$browser\'s outstandingRequestCount', function() {
+      var incOutstandingRequestCountSpy, completeOutstandingRequestSpy;
+
+      beforeEach(inject(function($browser) {
+        incOutstandingRequestCountSpy
+          = spyOn($browser, '$$incOutstandingRequestCount').andCallThrough();
+        completeOutstandingRequestSpy
+          = spyOn($browser, '$$completeOutstandingRequest').andCallThrough();
+      }));
+
+      it('should update $browser outstandingRequestCount on success', function() {
+        $httpBackend.when('GET').respond(200);
+
+        expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled();
+        $http.get('');
+        expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce();
+        $httpBackend.flush();
+        expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce();
+      });
+
+      it('should update $browser outstandingRequestCount on error', function() {
+        $httpBackend.when('GET').respond(500);
+
+        expect(incOutstandingRequestCountSpy).not.toHaveBeenCalled();
+        $http.get('');
+        expect(incOutstandingRequestCountSpy).toHaveBeenCalledOnce();
+        $httpBackend.flush();
+        expect(completeOutstandingRequestSpy).toHaveBeenCalledOnce();
+      });
+    });
  });