refactor($http): remove unneeded call to $apply

Author: thorn0

src/ng/http.js

@@ -308,34 +308,6 @@ function $HttpProvider() {
     paramSerializer: '$httpParamSerializer'
   };
 
-  var useApplyAsync = false;
-  /**
-   * @ngdoc method
-   * @name $httpProvider#useApplyAsync
-   * @description
-   *
-   * Configure $http service to combine processing of multiple http responses received at around
-   * the same time via {@link ng.$rootScope.Scope#$applyAsync $rootScope.$applyAsync}. This can result in
-   * significant performance improvement for bigger applications that make many HTTP requests
-   * concurrently (common during application bootstrap).
-   *
-   * Defaults to false. If no value is specified, returns the current configured value.
-   *
-   * @param {boolean=} value If true, when requests are loaded, they will schedule a deferred
-   *    "apply" on the next tick, giving time for subsequent requests in a roughly ~10ms window
-   *    to load and share the same digest cycle.
-   *
-   * @returns {boolean|Object} If a value is specified, returns the $httpProvider for chaining.
-   *    otherwise, returns the current configured value.
-   **/
-  this.useApplyAsync = function(value) {
-    if (isDefined(value)) {
-      useApplyAsync = !!value;
-      return this;
-    }
-    return useApplyAsync;
-  };
-
   var useLegacyPromise = true;
   /**
    * @ngdoc method
@@ -375,8 +347,8 @@ function $HttpProvider() {
    **/
   var interceptorFactories = this.interceptors = [];
 
-  this.$get = ['$httpBackend', '$$cookieReader', '$cacheFactory', '$rootScope', '$q', '$injector',
-      function($httpBackend, $$cookieReader, $cacheFactory, $rootScope, $q, $injector) {
+  this.$get = ['$httpBackend', '$$cookieReader', '$cacheFactory', '$q', '$injector',
+      function($httpBackend, $$cookieReader, $cacheFactory, $q, $injector) {
 
     var defaultCache = $cacheFactory('$http');
 
@@ -404,7 +376,6 @@ function $HttpProvider() {
      * @name $http
      * @requires ng.$httpBackend
      * @requires $cacheFactory
-     * @requires $rootScope
      * @requires $q
      * @requires $injector
      *
@@ -1189,7 +1160,7 @@ function $HttpProvider() {
      * Makes the request.
      *
      * !!! ACCESSES CLOSURE VARS:
-     * $httpBackend, defaults, $log, $rootScope, defaultCache, $http.pendingRequests
+     * $httpBackend, defaults, $log, defaultCache, $http.pendingRequests
      */
     function sendReq(config, reqData) {
       var deferred = $q.defer(),
@@ -1252,7 +1223,6 @@ function $HttpProvider() {
        * Callback registered to $httpBackend():
        *  - caches the response if desired
        *  - resolves the raw $http promise
-       *  - calls $apply
        */
       function done(status, response, headersString, statusText) {
         if (cache) {
@@ -1264,16 +1234,7 @@ function $HttpProvider() {
           }
         }
 
-        function resolveHttpPromise() {
-          resolvePromise(response, status, headersString, statusText);
-        }
-
-        if (useApplyAsync) {
-          $rootScope.$applyAsync(resolveHttpPromise);
-        } else {
-          resolveHttpPromise();
-          if (!$rootScope.$$phase) $rootScope.$apply();
-        }
+        resolvePromise(response, status, headersString, statusText);
       }
 
 

src/ngMock/angular-mocks.js

@@ -1697,10 +1697,14 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
       while (count--) {
         if (!responses.length) throw new Error('No more pending request to flush !');
         responses.shift()();
+        // Calling $digest again to execute the callbacks of the promises returned from $http
+        // as these callbacks can add new requests to the queue to flush.
+        if (digest !== false) $rootScope.$digest();
       }
     } else {
       while (responses.length) {
         responses.shift()();
+        if (digest !== false) $rootScope.$digest();
       }
     }
     $httpBackend.verifyNoOutstandingExpectation(digest);

test/ng/directive/ngIncludeSpec.js

@@ -360,18 +360,23 @@ describe('ngInclude', function() {
 
   it('should construct SVG template elements with correct namespace', function() {
     if (!window.SVGRectElement) return;
-    module(function($compileProvider) {
+    module(function($compileProvider, $provide) {
       $compileProvider.directive('test', valueFn({
         templateNamespace: 'svg',
         templateUrl: 'my-rect.html',
         replace: true
       }));
+      $provide.decorator('$templateRequest', function($delegate) {
+        return jasmine.createSpy('$templateRequest').andCallFake($delegate);
+      });
     });
-    inject(function($compile, $rootScope, $httpBackend) {
+    inject(function($compile, $rootScope, $httpBackend, $templateRequest) {
       $httpBackend.expectGET('my-rect.html').respond('<g ng-include="\'include.svg\'"></g>');
       $httpBackend.expectGET('include.svg').respond('<rect></rect><rect></rect>');
       element = $compile('<svg><test></test></svg>')($rootScope);
-      $httpBackend.flush();
+      expect($templateRequest.calls.length).toBe(1);
+      $httpBackend.flush(2);
+      expect($templateRequest.calls.length).toBe(2);
       var child = element.find('rect');
       expect(child.length).toBe(2);
       expect(child[0] instanceof SVGRectElement).toBe(true);

test/ng/httpSpec.js

@@ -293,7 +293,7 @@ describe('$http', function() {
       $httpBackend = $hb;
       $http = $h;
       $rootScope = $rs;
-      spyOn($rootScope, '$apply').andCallThrough();
+      spyOn($rootScope, '$digest').andCallThrough();
     }]));
 
     it('should throw error if the request configuration is not an object', function() {
@@ -1012,37 +1012,6 @@ describe('$http', function() {
     });
 
 
-    describe('scope.$apply', function() {
-
-      it('should $apply after success callback', function() {
-        $httpBackend.when('GET').respond(200);
-        $http({method: 'GET', url: '/some'});
-        $httpBackend.flush();
-        expect($rootScope.$apply).toHaveBeenCalledOnce();
-      });
-
-
-      it('should $apply after error callback', function() {
-        $httpBackend.when('GET').respond(404);
-        $http({method: 'GET', url: '/some'});
-        $httpBackend.flush();
-        expect($rootScope.$apply).toHaveBeenCalledOnce();
-      });
-
-
-      it('should $apply even if exception thrown during callback', inject(function($exceptionHandler) {
-        $httpBackend.when('GET').respond(200);
-        callback.andThrow('error in callback');
-
-        $http({method: 'GET', url: '/some'}).then(callback);
-        $httpBackend.flush();
-        expect($rootScope.$apply).toHaveBeenCalledOnce();
-
-        $exceptionHandler.errors = [];
-      }));
-    });
-
-
     describe('transformData', function() {
 
       describe('request', function() {
@@ -1900,35 +1869,27 @@ describe('$http', function() {
 });
 
 
-describe('$http with $applyAsync', function() {
-  var $http, $httpBackend, $rootScope, $browser, log;
-  beforeEach(module(function($httpProvider) {
-    $httpProvider.useApplyAsync(true);
-  }, provideLog));
-
+describe('$http interacting with digest cycle', function() {
+  var $http, $httpBackend, $browser, log;
+  beforeEach(module(provideLog));
 
-  beforeEach(inject(['$http', '$httpBackend', '$rootScope', '$browser', 'log', function(http, backend, scope, browser, logger) {
+  beforeEach(inject(['$http', '$httpBackend', '$browser', 'log', function(http, backend, browser, logger) {
     $http = http;
     $httpBackend = backend;
-    $rootScope = scope;
     $browser = browser;
-    spyOn($rootScope, '$apply').andCallThrough();
-    spyOn($rootScope, '$applyAsync').andCallThrough();
-    spyOn($rootScope, '$digest').andCallThrough();
-    spyOn($browser.defer, 'cancel').andCallThrough();
     log = logger;
   }]));
 
 
-  it('should schedule coalesced apply on response', function() {
+  it('should execute callbacks asynchronously in $digest', function() {
     var handler = jasmine.createSpy('handler');
     $httpBackend.expect('GET', '/template1.html').respond(200, '<h1>Header!</h1>', {});
     $http.get('/template1.html').then(handler);
-    // Ensure requests are sent
-    $rootScope.$digest();
+    // Ensure requests are sent. ($http is internally promise-based and doesn't start working until
+    // $digest occurs.)
+    $browser.defer.flush();
 
     $httpBackend.flush(null, false);
-    expect($rootScope.$applyAsync).toHaveBeenCalledOnce();
     expect(handler).not.toHaveBeenCalled();
 
     $browser.defer.flush();
@@ -1943,11 +1904,14 @@ describe('$http with $applyAsync', function() {
     $http.get('/template1.html').then(log.fn('response 1'));
     $http.get('/template2.html').then(log.fn('response 2'));
     // Ensure requests are sent
-    $rootScope.$digest();
+    $browser.defer.flush();
 
+    // Resolve the promises. When a $q promise is resolved it uses $rootScope.$evalAsync to schedule
+    // the execution of its callbacks.
     $httpBackend.flush(null, false);
     expect(log).toEqual([]);
 
+    // Execute the promises' callbacks in the $digest scheduled with $evalAsync
     $browser.defer.flush();
     expect(log).toEqual(['response 1', 'response 2']);
   });
@@ -1961,16 +1925,16 @@ describe('$http with $applyAsync', function() {
     $http.get('/template1.html').then(log.fn('response 1'));
     $http.get('/template2.html').then(log.fn('response 2'));
     $http.get('/template3.html').then(log.fn('response 3'));
-    // Ensure requests are sent
-    $rootScope.$digest();
 
     // Intermediate $digest occurs before 3rd response is received, assert that pending responses
-    /// are handled
+    // are handled. Unless false is passed as the second parameter, $httpBackend.flush calls
+    // $rootScope.$digest at least twice (before and after doing the flush).
     $httpBackend.flush(2);
     expect(log).toEqual(['response 1', 'response 2']);
 
-    // Finally, third response is received, and a second coalesced $apply is started
+    // Finally, third response is received, and its callback is scheduled with $evalAsync
     $httpBackend.flush(null, false);
+    // Execute the promises' callbacks in the $digest scheduled with $evalAsync
     $browser.defer.flush();
     expect(log).toEqual(['response 1', 'response 2', 'response 3']);
   });