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

This `$apply` call was needed only for tests, so it's been moved to `$httpBackend.flush`.

BREAKING CHANGE: `$httpProvider.useApplyAsync` has been removed.

Closes #13108
Closes #13111

Author: thorn0

src/ng/http.js

@@ -343,34 +343,6 @@ function $HttpProvider() {
     jsonpCallbackParam: 'callback'
   };
 
-  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;
-  };
-
   /**
    * @ngdoc property
    * @name $httpProvider#interceptors
@@ -1342,17 +1314,9 @@ function $HttpProvider() {
           var applyHandlers = {};
           forEach(eventHandlers, function(eventHandler, key) {
             applyHandlers[key] = function(event) {
-              if (useApplyAsync) {
-                $rootScope.$applyAsync(callEventHandler);
-              } else if ($rootScope.$$phase) {
-                callEventHandler();
-              } else {
-                $rootScope.$apply(callEventHandler);
-              }
-
-              function callEventHandler() {
+              $rootScope.$evalAsync(function callEventHandler() {
                 eventHandler(event);
-              }
+              });
             };
           });
           return applyHandlers;
@@ -1364,7 +1328,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, xhrStatus) {
         if (cache) {
@@ -1376,16 +1339,7 @@ function $HttpProvider() {
           }
         }
 
-        function resolveHttpPromise() {
-          resolvePromise(response, status, headersString, statusText, xhrStatus);
-        }
-
-        if (useApplyAsync) {
-          $rootScope.$applyAsync(resolveHttpPromise);
-        } else {
-          resolveHttpPromise();
-          if (!$rootScope.$$phase) $rootScope.$apply();
-        }
+        resolvePromise(response, status, headersString, statusText, xhrStatus);
       }
 
 

src/ngMock/angular-mocks.js

@@ -1849,10 +1849,14 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
         var part = responses.splice(skip, 1);
         if (!part.length) throw new Error('No more pending request to flush !');
         part[0]();
+        // 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 > skip) {
         responses.splice(skip, 1)[0]();
+        if (digest !== false) $rootScope.$digest();
       }
     }
     $httpBackend.verifyNoOutstandingExpectation(digest);

test/ng/directive/ngIncludeSpec.js

@@ -362,18 +362,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').and.callFake($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).toHaveBeenCalledTimes(1);
+        $httpBackend.flush(2);
+        expect($templateRequest).toHaveBeenCalledTimes(2);
         var child = element.find('rect');
         expect(child.length).toBe(2);
         // eslint-disable-next-line no-undef

test/ng/httpSpec.js

@@ -301,7 +301,6 @@ describe('$http', function() {
       $http = $h;
       $rootScope = $rs;
       $sce = $sc;
-      spyOn($rootScope, '$apply').and.callThrough();
     }]));
 
     it('should throw error if the request configuration is not an object', function() {
@@ -1073,35 +1072,7 @@ describe('$http', function() {
 
     describe('callbacks', 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'}).catch(noop);
-        $httpBackend.flush();
-        expect($rootScope.$apply).toHaveBeenCalledOnce();
-      });
-
-
-      it('should $apply even if exception thrown during callback', inject(function($exceptionHandler) {
-        $httpBackend.when('GET').respond(200);
-        callback.and.throwError('error in callback');
-
-        $http({method: 'GET', url: '/some'}).then(callback);
-        $httpBackend.flush();
-        expect($rootScope.$apply).toHaveBeenCalledOnce();
-
-        $exceptionHandler.errors = [];
-      }));
-
-
-      it('should pass the event handlers through to the backend', function() {
+      it('should pass the event handlers through to the backend', inject(function($browser) {
         var progressFn = jasmine.createSpy('progressFn');
         var uploadProgressFn = jasmine.createSpy('uploadProgressFn');
         $httpBackend.when('GET').respond(200);
@@ -1117,16 +1088,17 @@ describe('$http', function() {
         expect(mockXHR.upload.$$events.progress).toEqual(jasmine.any(Function));
 
         var eventObj = {};
-        spyOn($rootScope, '$digest');
 
         mockXHR.$$events.progress(eventObj);
+        // The invocation of the callback is scheduled via `$evalAsync`,
+        // that's why `flush` is needed here.
+        $browser.defer.flush();
         expect(progressFn).toHaveBeenCalledOnceWith(eventObj);
-        expect($rootScope.$digest).toHaveBeenCalledTimes(1);
 
         mockXHR.upload.$$events.progress(eventObj);
+        $browser.defer.flush();
         expect(uploadProgressFn).toHaveBeenCalledOnceWith(eventObj);
-        expect($rootScope.$digest).toHaveBeenCalledTimes(2);
-      });
+      }));
     });
 
 
@@ -2293,35 +2265,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').and.callThrough();
-    spyOn($rootScope, '$applyAsync').and.callThrough();
-    spyOn($rootScope, '$digest').and.callThrough();
-    spyOn($browser.defer, 'cancel').and.callThrough();
     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, null, false);
-    expect($rootScope.$applyAsync).toHaveBeenCalledOnce();
     expect(handler).not.toHaveBeenCalled();
 
     $browser.defer.flush();
@@ -2336,11 +2300,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, 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']);
   });
@@ -2354,16 +2321,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, null, false);
+    // Execute the promises' callbacks in the $digest scheduled with $evalAsync
     $browser.defer.flush();
     expect(log).toEqual(['response 1', 'response 2', 'response 3']);
   });