feat(scope): throw exception when recursive $apply

2 files changed, 87 insertions, 22 deletions

diff --git a/src/service/scope.js b/src/service/scope.js
index 1c6ee63f..2836b42e 100644
--- a/src/service/scope.js
+++ b/src/service/scope.js
@@ -36,7 +36,8 @@
  */
 function $RootScopeProvider(){
   this.$get = ['$injector', '$exceptionHandler', '$parse',
-      function( $injector,   $exceptionHandler,   $parse){
+      function( $injector,   $exceptionHandler,   $parse) {
+
     /**
      * @ngdoc function
      * @name angular.module.ng.$rootScope.Scope
@@ -152,8 +153,7 @@ function $RootScopeProvider(){
         child.$parent = this;
         child.$id = nextUid();
         child.$$asyncQueue = [];
-        child.$$phase = child.$$watchers =
-          child.$$nextSibling = child.$$childHead = child.$$childTail = null;
+        child.$$watchers = child.$$nextSibling = child.$$childHead = child.$$childTail = null;
         child.$$prevSibling = this.$$childTail;
         if (this.$$childHead) {
           this.$$childTail.$$nextSibling = child;
@@ -326,15 +326,12 @@ function $RootScopeProvider(){
             watchLog = [],
             logIdx, logMsg;
 
-        if (target.$$phase) {
-          throw Error(target.$$phase + ' already in progress');
-        }
-        do {
+        flagPhase(target, '$digest');
 
+        do {
           dirty = false;
           current = target;
           do {
-            current.$$phase = '$digest';
             asyncQueue = current.$$asyncQueue;
             while(asyncQueue.length) {
               try {
@@ -356,7 +353,7 @@ function $RootScopeProvider(){
                     watch.last = copy(value);
                     watch.fn(current, value, ((last === initWatchVal) ? value : last));
                     if (ttl < 5) {
-                      logIdx = 4-ttl;
+                      logIdx = 4 - ttl;
                       if (!watchLog[logIdx]) watchLog[logIdx] = [];
                       logMsg = (isFunction(watch.exp))
                           ? 'fn: ' + (watch.exp.name || watch.exp.toString())
@@ -371,8 +368,6 @@ function $RootScopeProvider(){
               }
             }
 
-            current.$$phase = null;
-
             // Insanity Warning: scope depth-first traversal
             // yes, this code is a bit crazy, but it works and we have tests to prove it!
             // this piece should be kept in sync with the traversal in $broadcast
@@ -388,6 +383,8 @@ function $RootScopeProvider(){
                 'Watchers fired in the last 5 iterations: ' + toJson(watchLog));
           }
         } while (dirty || asyncQueue.length);
+
+        this.$root.$$phase = null;
       },
 
       /**
@@ -524,10 +521,12 @@ function $RootScopeProvider(){
        */
       $apply: function(expr) {
         try {
+          flagPhase(this, '$apply');
           return this.$eval(expr);
         } catch (e) {
           $exceptionHandler(e);
         } finally {
+          this.$root.$$phase = null;
           this.$root.$digest();
         }
       },
@@ -671,6 +670,17 @@ function $RootScopeProvider(){
       }
     };
 
+
+    function flagPhase(scope, phase) {
+      var root = scope.$root;
+
+      if (root.$$phase) {
+        throw Error(root.$$phase + ' already in progress');
+      }
+
+      root.$$phase = phase;
+    }
+
     return new Scope();
 
     function compileToFn(exp, name) {
diff --git a/test/service/scopeSpec.js b/test/service/scopeSpec.js
index a3501816..e1e8ab8b 100644
--- a/test/service/scopeSpec.js
+++ b/test/service/scopeSpec.js
@@ -2,11 +2,6 @@
 
 describe('Scope', function() {
 
-  beforeEach(inject(function($exceptionHandlerProvider) {
-    $exceptionHandlerProvider.mode('log');
-  }));
-
-
   describe('$root', function() {
     it('should point to itself', inject(function($rootScope) {
       expect($rootScope.$root).toEqual($rootScope);
@@ -122,7 +117,9 @@ describe('Scope', function() {
     }));
 
 
-    it('should delegate exceptions', inject(function($rootScope, $exceptionHandler, $log) {
+    it('should delegate exceptions', inject(function($exceptionHandlerProvider) {
+      $exceptionHandlerProvider.mode('log');
+    }, function($rootScope, $exceptionHandler, $log) {
       $rootScope.$watch('a', function() {throw new Error('abc');});
       $rootScope.a = 1;
       $rootScope.$digest();
@@ -227,7 +224,7 @@ describe('Scope', function() {
     }));
 
 
-    it('should prevent infinite recurcion and print print watcher function name or body',
+    it('should prevent infinite recursion and print print watcher function name or body',
         inject(function($rootScope) {
       $rootScope.$watch(function watcherA() {return $rootScope.a;}, function(self) {self.b++;});
       $rootScope.$watch(function() {return $rootScope.b;}, function(self) {self.a++;});
@@ -277,7 +274,7 @@ describe('Scope', function() {
     }));
 
 
-    it('should prevent recursion', inject(function($rootScope) {
+    it('should prevent $digest recursion', inject(function($rootScope) {
       var callCount = 0;
       $rootScope.$watch('name', function() {
         expect(function() {
@@ -462,7 +459,9 @@ describe('Scope', function() {
     }));
 
 
-    it('should catch exceptions', inject(function($rootScope, $exceptionHandler, $log) {
+    it('should catch exceptions', inject(function($exceptionHandlerProvider) {
+        $exceptionHandlerProvider.mode('log');
+      }, function($rootScope, $exceptionHandler, $log) {
       var log = '';
       var child = $rootScope.$new();
       $rootScope.$watch('a', function(scope, a) { log += '1'; });
@@ -476,7 +475,9 @@ describe('Scope', function() {
 
     describe('exceptions', function() {
       var log;
-      beforeEach(inject(function($rootScope) {
+      beforeEach(inject(function($exceptionHandlerProvider) {
+          $exceptionHandlerProvider.mode('log');
+        }, function($rootScope) {
         log = '';
         $rootScope.$watch(function() { log += '$digest;'; });
         $rootScope.$digest();
@@ -502,6 +503,57 @@ describe('Scope', function() {
         expect($exceptionHandler.errors).toEqual([error]);
       }));
     });
+
+
+    describe('recursive $apply protection', function() {
+      it('should throw an exception if $apply is called while an $apply is in progress', inject(
+          function($rootScope) {
+        expect(function() {
+          $rootScope.$apply(function() {
+            $rootScope.$apply();
+          });
+        }).toThrow('$apply already in progress');
+      }));
+
+
+      it('should throw an exception if $apply is called while flushing evalAsync queue', inject(
+          function($rootScope) {
+        expect(function() {
+          $rootScope.$apply(function() {
+            $rootScope.$evalAsync(function() {
+              $rootScope.$apply();
+            });
+          });
+        }).toThrow('$digest already in progress');
+      }));
+
+
+      it('should throw an exception if $apply is called while a watch is being initialized', inject(
+          function($rootScope) {
+        var childScope1 = $rootScope.$new();
+        childScope1.$watch('x', function() {
+          childScope1.$apply();
+        });
+        expect(function() { childScope1.$apply(); }).toThrow('$digest already in progress');
+      }));
+
+
+      it('should thrown an exception if $apply in called from a watch fn (after init)', inject(
+          function($rootScope) {
+        var childScope2 = $rootScope.$new();
+        childScope2.$apply(function() {
+          childScope2.$watch('x', function(scope, newVal, oldVal) {
+            if (newVal !== oldVal) {
+              childScope2.$apply();
+            }
+          });
+        });
+
+        expect(function() { childScope2.$apply(function() {
+          childScope2.x = 'something';
+        }); }).toThrow('$digest already in progress');
+      }));
+    });
   });
 
 
@@ -561,7 +613,10 @@ describe('Scope', function() {
         log += event.currentScope.id + '>';
       }
 
-      beforeEach(inject(function($rootScope) {
+      beforeEach(inject(
+          function($exceptionHandlerProvider) {
+        $exceptionHandlerProvider.mode('log');
+      }, function($rootScope) {
         log = '';
         child = $rootScope.$new();
         grandChild = child.$new();