Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(Scope): don't let watch deregistration mess up the dirty-checking…
Browse files Browse the repository at this point in the history 
… digest loop

Closes #5525
  • Loading branch information
@IgorMinar
IgorMinar committed Jan 2, 2014
1 parent 010413f commit 884ef0d
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/ng/rootScope.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ function $RootScopeProvider(){

return function() {
arrayRemove(array, watcher);
lastDirtyWatch = null;
};
},

Expand Down
47 changes: 47 additions & 0 deletions test/ng/rootScopeSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,53 @@ describe('Scope', function() {
$rootScope.$apply('remove = true');
}).not.toThrow();
}));


it('should not mess up the digest loop if deregistration happens during digest', inject(
function($rootScope, log) {

// we are testing this due to regression #5525 which is related to how the digest loops lastDirtyWatch
// short-circuiting optimization works

// scenario: watch1 deregistering watch1
var scope = $rootScope.$new();
var deregWatch1 = scope.$watch(log.fn('watch1'), function() { deregWatch1(); log('watchAction1'); });
scope.$watch(log.fn('watch2'), log.fn('watchAction2'));
scope.$watch(log.fn('watch3'), log.fn('watchAction3'));

$rootScope.$digest();

expect(log).toEqual(['watch1', 'watchAction1', 'watch2', 'watchAction2', 'watch3', 'watchAction3',
'watch2', 'watch3']);
scope.$destroy();
log.reset();


// scenario: watch1 deregistering watch2
scope = $rootScope.$new();
scope.$watch(log.fn('watch1'), function() { deregWatch2(); log('watchAction1'); });
var deregWatch2 = scope.$watch(log.fn('watch2'), log.fn('watchAction2'));
scope.$watch(log.fn('watch3'), log.fn('watchAction3'));

$rootScope.$digest();

expect(log).toEqual(['watch1', 'watchAction1', 'watch1', 'watch3', 'watchAction3',
'watch1', 'watch3']);
scope.$destroy();
log.reset();


// scenario: watch2 deregistering watch1
scope = $rootScope.$new();
deregWatch1 = scope.$watch(log.fn('watch1'), log.fn('watchAction1'));
scope.$watch(log.fn('watch2'), function() { deregWatch1(); log('watchAction2'); });
scope.$watch(log.fn('watch3'), log.fn('watchAction3'));

$rootScope.$digest();

expect(log).toEqual(['watch1', 'watchAction1', 'watch2', 'watchAction2', 'watch3', 'watchAction3',
'watch2', 'watch3']);
}));
});


Expand Down

1 comment on commit 884ef0d

@facka
Copy link

@facka facka commented on 884ef0d Feb 22, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the deregisterWatch function the watcher object should be deleted;

return function() {
          arrayRemove(array, watcher);
          lastDirtyWatch = null;
          watcher = null;
};

I have tested and with out this fix the watcher object stays alive after deregisterWatch function is called.

Also I would suggest to avoid use {} to create objects since you can't measure with the Browser profiler the memory usage and also you cant count the instances.

Please sign in to comment.