From c27ce965f9a03c32121998ba7c6fe37ccce15931 Mon Sep 17 00:00:00 2001 From: Marko Vuksanovic Date: Wed, 30 Apr 2014 23:10:27 +1000 Subject: [PATCH 1/2] feat(Scope): Use VmTurnZone.onScheduleMicrotask in Scope BREAKING CHANGE: Previously a micro task registered in flush phase would cause a new digest cycle after the current digest cycle. The new behavior will cause an error. Closes #984 --- lib/core/scope.dart | 4 ++ lib/core/zone.dart | 1 + test/core/scope_spec.dart | 92 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) diff --git a/lib/core/scope.dart b/lib/core/scope.dart index 4a05a1da0..b47e8ff69 100644 --- a/lib/core/scope.dart +++ b/lib/core/scope.dart @@ -644,6 +644,7 @@ class RootScope extends Scope { { _zone.onTurnDone = apply; _zone.onError = (e, s, ls) => _exceptionHandler(e, s); + _zone.onScheduleMicrotask = runAsync; } RootScope get rootScope => this; @@ -787,6 +788,9 @@ class RootScope extends Scope { // QUEUES void runAsync(fn()) { + if (_state == STATE_FLUSH || _state == STATE_FLUSH_ASSERT) { + throw "Scheduling microtasks not allowed in $state state."; + } var chain = new _FunctionChain(fn); if (_runAsyncHead == null) { _runAsyncHead = _runAsyncTail = chain; diff --git a/lib/core/zone.dart b/lib/core/zone.dart index 68953859b..fae355b00 100644 --- a/lib/core/zone.dart +++ b/lib/core/zone.dart @@ -60,6 +60,7 @@ class VmTurnZone { /// an "inner" [Zone], which is a child of the outer [Zone]. async.Zone _innerZone; + /** * Associates with this * diff --git a/test/core/scope_spec.dart b/test/core/scope_spec.dart index aa1f62d21..2530be333 100644 --- a/test/core/scope_spec.dart +++ b/test/core/scope_spec.dart @@ -1440,6 +1440,98 @@ void main() { }); }); + describe('microtask processing', () { + beforeEach((VmTurnZone zone, RootScope scope, Logger log) { + var onTurnDone = zone.onTurnDone; + zone.onTurnDone = () { + log('['); + onTurnDone(); + log(']'); + }; + var onScheduleMicrotask = zone.onScheduleMicrotask; + zone.onScheduleMicrotask = (fn) { + log('('); + try { + onScheduleMicrotask(fn); + } catch (e) { + log('CATCH: $e'); + } + log(')'); + }; + }); + + it('should schedule apply after future resolution', + async((Logger log, VmTurnZone zone, RootScope scope) { + Completer completer; + zone.run(() { + completer = new Completer(); + completer.future.then((value) { + log('then($value)'); + }); + }); + + scope.runAsync(() => log('before')); + log.clear(); + completer.complete('OK'); // this one causes APPLY which processe 'before' + // This one schedules work but apply already run so it does not execute. + scope.runAsync(() => log('NOT_EXECUTED')); + + expect(log).toEqual(['(', ')', '[', 'before', 'then(OK)', ']']); + }) + ); + + it('should schedule microtask to runAsync queue during digest', + async((Logger log, VmTurnZone zone, RootScope scope) { + Completer completer; + zone.run(() { + completer = new Completer(); + completer.future. + then((value) { + scope.runAsync(() => log('in(${scope.state})')); + return new Future.value(value); + }). + then((value) { + log('then($value)'); + }); + }); + log.clear(); + completer.complete('OK'); + expect(log).toEqual(['(', ')', '[', '(', ')', 'in(digest)', 'then(OK)', ']']); + }) + ); + + it('should not allow microtasks in flush phase', + async((Logger log, VmTurnZone zone, RootScope scope) { + zone.run(() { + scope.domWrite(() { + return new Future.value('FAIL'); + }); + }); + expect(log).toEqual( + ['[', '(', 'CATCH: Scheduling microtasks not allowed in flush state.', ')', ']']); + }) + ); + + it('should allow creation of Completers in flush phase', + async((Logger log, VmTurnZone zone, RootScope scope) { + Completer completer; + zone.run(() { + scope.domWrite(() { + log('new Completer'); + completer = new Completer(); + completer.future.then((value) { + log('then($value)'); + }); + }); + }); + log('='); + completer.complete('OK'); + log(';'); + expect(log).toEqual( + ['[', 'new Completer', ']', '=', '(', ')', '[', 'then(OK)', ']', ';']); + }) + ); + }); describe('domRead/domWrite', () { beforeEachModule((Module module) { From 53dd4ad4014c61b354a50e722e866305d6bda49d Mon Sep 17 00:00:00 2001 From: James deBoer Date: Fri, 2 May 2014 12:05:14 -0700 Subject: [PATCH 2/2] fix(scope): Use runAsync for microtasks in digest. BREAKING CHANGE: Microtasks scheduled in flush will process in current cycle, but they are not allowed to do model changes. Microtasks scheduled in digest will be executed in digest, counting towards the ScopeDigestTTL. --- lib/core/scope.dart | 35 ++++++++++++++++++++++------------- test/core/scope_spec.dart | 27 ++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/lib/core/scope.dart b/lib/core/scope.dart index b47e8ff69..f891d1d24 100644 --- a/lib/core/scope.dart +++ b/lib/core/scope.dart @@ -680,15 +680,8 @@ class RootScope extends Scope { ChangeLog changeLog; _scopeStats.digestStart(); do { - while (_runAsyncHead != null) { - try { - _runAsyncHead.fn(); - } catch (e, s) { - _exceptionHandler(e, s); - } - _runAsyncHead = _runAsyncHead._next; - } - _runAsyncTail = null; + + int asyncCount = _runAsyncFns(); digestTTL--; count = rootWatchGroup.detectChanges( @@ -704,7 +697,7 @@ class RootScope extends Scope { digestLog = []; changeLog = (e, c, p) => digestLog.add('$e: $c <= $p'); } else { - log.add(digestLog.join(', ')); + log.add("${asyncCount > 0 ? 'async:$asyncCount' : ''}${digestLog.join(', ')}"); digestLog.clear(); } } @@ -713,7 +706,7 @@ class RootScope extends Scope { 'Last $LOG_COUNT iterations:\n${log.join('\n')}'; } _scopeStats.digestLoop(count); - } while (count > 0); + } while (count > 0 || _runAsyncHead != null); } finally { _scopeStats.digestEnd(); _transitionState(STATE_DIGEST, null); @@ -756,7 +749,8 @@ class RootScope extends Scope { if (_domReadHead == null) _stats.domReadEnd(); } _domReadTail = null; - } while (_domWriteHead != null || _domReadHead != null); + _runAsyncFns(); + } while (_domWriteHead != null || _domReadHead != null || _runAsyncHead != null); _stats.flushEnd(); assert((() { _stats.flushAssertStart(); @@ -788,7 +782,7 @@ class RootScope extends Scope { // QUEUES void runAsync(fn()) { - if (_state == STATE_FLUSH || _state == STATE_FLUSH_ASSERT) { + if (_state == STATE_FLUSH_ASSERT) { throw "Scheduling microtasks not allowed in $state state."; } var chain = new _FunctionChain(fn); @@ -799,6 +793,21 @@ class RootScope extends Scope { } } + _runAsyncFns() { + var count = 0; + while (_runAsyncHead != null) { + try { + count++; + _runAsyncHead.fn(); + } catch (e, s) { + _exceptionHandler(e, s); + } + _runAsyncHead = _runAsyncHead._next; + } + _runAsyncTail = null; + return count; + } + void domWrite(fn()) { var chain = new _FunctionChain(fn); if (_domWriteHead == null) { diff --git a/test/core/scope_spec.dart b/test/core/scope_spec.dart index 2530be333..bd2770fd6 100644 --- a/test/core/scope_spec.dart +++ b/test/core/scope_spec.dart @@ -1227,6 +1227,20 @@ void main() { }); + it(r'should detect infinite digest through runAsync', (RootScope rootScope) { + rootScope.context['value'] = () { rootScope.runAsync(() {}); return 'a'; }; + rootScope.watch('value()', (_, __) {}); + + expect(() { + rootScope.digest(); + }).toThrow('Model did not stabilize in 5 digests. ' + 'Last 3 iterations:\n' + 'async:1\n' + 'async:1\n' + 'async:1'); + }); + + it(r'should always call the watchr with newVal and oldVal equal on the first run', inject((RootScope rootScope) { var log = []; @@ -1500,15 +1514,22 @@ void main() { }) ); - it('should not allow microtasks in flush phase', + it('should allow microtasks in flush phase and process them immediatly', async((Logger log, VmTurnZone zone, RootScope scope) { + scope.watch('g()', (_, __) {}); + scope.context['g'] = () { + log('!'); + return 0; + }; + zone.run(() { scope.domWrite(() { - return new Future.value('FAIL'); + log('domWriteA'); + return new Future.value(null).then((_) => scope.domWrite(() => log('domWriteB'))); }); }); expect(log).toEqual( - ['[', '(', 'CATCH: Scheduling microtasks not allowed in flush state.', ')', ']']); + ['[', '!', '!', 'domWriteA', '(', ')', 'domWriteB', /* assert */'!', ']']); }) );