fix(scope): fix edge case for $digest & $broadcast scope traversal

- fixed traversal originating on a scope with with a right sibling
- unified code for both $broadcast and $digest
This commit is contained in:
Igor Minar 2011-08-30 17:36:39 -07:00
parent c763b009ac
commit 93f96a16f6
2 changed files with 81 additions and 59 deletions

View file

@ -321,31 +321,31 @@ Scope.prototype = {
*
*/
$digest: function() {
var watch, value, last, next,
var watch, value, last,
watchers,
asyncQueue,
length,
dirty, ttl = 100,
scope;
next, current, target = this;
if (this.$$phase) {
throw Error(this.$$phase + ' already in progress');
if (target.$$phase) {
throw Error(target.$$phase + ' already in progress');
}
do {
dirty = false;
scope = this;
current = target;
do {
scope.$$phase = '$digest';
asyncQueue = scope.$$asyncQueue;
current.$$phase = '$digest';
asyncQueue = current.$$asyncQueue;
while(asyncQueue.length) {
try {
scope.$eval(asyncQueue.shift());
current.$eval(asyncQueue.shift());
} catch (e) {
scope.$service('$exceptionHandler')(e);
current.$service('$exceptionHandler')(e);
}
}
if ((watchers = scope.$$watchers)) {
if ((watchers = current.$$watchers)) {
// process our watches
length = watchers.length;
while (length--) {
@ -353,28 +353,27 @@ Scope.prototype = {
watch = watchers[length];
// Most common watches are on primitives, in which case we can short
// circuit it with === operator, only when === fails do we use .equals
if ((value = watch.get(scope)) !== (last = watch.last) && !equals(value, last)) {
if ((value = watch.get(current)) !== (last = watch.last) && !equals(value, last)) {
dirty = true;
watch.fn(scope, watch.last = copy(value), last);
watch.fn(current, watch.last = copy(value), last);
}
} catch (e) {
scope.$service('$exceptionHandler')(e);
current.$service('$exceptionHandler')(e);
}
}
}
current.$$phase = null;
scope.$$phase = null;
// find the next scope in traversal.
if (!(next = scope.$$childHead || scope.$$nextSibling) && scope !== this) {
do {
scope = scope.$parent;
if (scope == this || (next = scope.$$nextSibling)) {
break;
}
} while (scope !== this);
// 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
if (!(next = (current.$$childHead || (current !== target && current.$$nextSibling)))) {
while(current !== target && !(next = current.$$nextSibling)) {
current = current.$parent;
}
}
} while ((scope = next));
} while ((current = next));
if(!(ttl--)) {
throw Error('100 $digest() iterations reached. Aborting!');
@ -651,44 +650,34 @@ Scope.prototype = {
* @param {...*} args Optional set of arguments which will be passed onto the event listeners.
*/
$broadcast: function(name, args) {
var targetScope = this,
currentScope = targetScope,
nextScope = targetScope,
var target = this,
current = target,
next = target,
event = { name: name,
targetScope: targetScope },
targetScope: target },
listenerArgs = concat([event], arguments, 1);
//down while you can, then up and next sibling or up and next sibling until back at root
do {
currentScope = nextScope;
event.currentScope = currentScope;
forEach(currentScope.$$listeners[name], function(listener) {
current = next;
event.currentScope = current;
forEach(current.$$listeners[name], function(listener) {
try {
listener.apply(null, listenerArgs);
} catch(e) {
currentScope.$service('$exceptionHandler')(e);
current.$service('$exceptionHandler')(e);
}
});
// down or to the right!
nextScope = currentScope.$$childHead || currentScope.$$nextSibling;
if (nextScope) {
// found child or sibling
continue;
// 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 $digest
if (!(next = (current.$$childHead || (current !== target && current.$$nextSibling)))) {
while(current !== target && !(next = current.$$nextSibling)) {
current = current.$parent;
}
}
// we have to restore nextScope and go up!
nextScope = currentScope;
while (!nextScope.$$nextSibling && (nextScope != targetScope)) {
nextScope = nextScope.$parent;
}
if (nextScope != targetScope) {
nextScope = nextScope.$$nextSibling;
}
} while (nextScope != targetScope);
} while ((current = next));
}
};

View file

@ -153,7 +153,7 @@ describe('Scope', function() {
});
it('should delegate $digest to children in addition order', function() {
it('should call child $watchers in addition order', function() {
// this is not an external guarantee, just our own sanity
var log = '';
var childA = root.$new();
@ -168,6 +168,30 @@ describe('Scope', function() {
});
it('should allow $digest on a child scope with and without a right sibling', function() {
// tests a traversal edge case which we originally missed
var log = '',
childA = root.$new(),
childB = root.$new();
root.$watch(function() { log += 'r'; });
childA.$watch(function() { log += 'a'; });
childB.$watch(function() { log += 'b'; });
// init
root.$digest();
expect(log).toBe('rabrab');
log = '';
childA.$digest();
expect(log).toBe('a');
log = '';
childB.$digest();
expect(log).toBe('b');
});
it('should repeat watch cycle while model changes are identified', function() {
var log = '';
root.$watch('c', function(self, v){self.d = v; log+='c'; });
@ -498,7 +522,7 @@ describe('Scope', function() {
describe('$broadcast', function() {
describe('event propagation', function() {
var log, child1, child2, child3, grandChild11, grandChild21, grandChild22,
var log, child1, child2, child3, grandChild11, grandChild21, grandChild22, grandChild23,
greatGrandChild211;
function logger(event) {
@ -513,6 +537,7 @@ describe('Scope', function() {
grandChild11 = child1.$new();
grandChild21 = child2.$new();
grandChild22 = child2.$new();
grandChild23 = child2.$new();
greatGrandChild211 = grandChild21.$new();
root.id = 0;
@ -522,6 +547,7 @@ describe('Scope', function() {
grandChild11.id = 11;
grandChild21.id = 21;
grandChild22.id = 22;
grandChild23.id = 23;
greatGrandChild211.id = 211;
root.$on('myEvent', logger);
@ -531,13 +557,14 @@ describe('Scope', function() {
grandChild11.$on('myEvent', logger);
grandChild21.$on('myEvent', logger);
grandChild22.$on('myEvent', logger);
grandChild23.$on('myEvent', logger);
greatGrandChild211.$on('myEvent', logger);
// R
// / | \
// 1 2 3
// / / \
// 11 21 22
// R
// / | \
// 1 2 3
// / / | \
// 11 21 22 23
// |
// 211
});
@ -545,22 +572,28 @@ describe('Scope', function() {
it('should broadcast an event from the root scope', function() {
root.$broadcast('myEvent');
expect(log).toBe('0>1>11>2>21>211>22>3>');
expect(log).toBe('0>1>11>2>21>211>22>23>3>');
});
it('should broadcast an event from a child scope', function() {
child2.$broadcast('myEvent');
expect(log).toBe('2>21>211>22>');
expect(log).toBe('2>21>211>22>23>');
});
it('should broadcast an event from a leaf scope', function() {
it('should broadcast an event from a leaf scope with a sibling', function() {
grandChild22.$broadcast('myEvent');
expect(log).toBe('22>');
});
it('should broadcast an event from a leaf scope without a sibling', function() {
grandChild23.$broadcast('myEvent');
expect(log).toBe('23>');
});
it('should not not fire any listeners for other events', function() {
root.$broadcast('fooEvent');
expect(log).toBe('');