diff --git a/lib/block.dart b/lib/block.dart index c2960e1e5..d6b993b01 100644 --- a/lib/block.dart +++ b/lib/block.dart @@ -296,9 +296,17 @@ class _ComponentFactory { cssFuture = new async.Future.value(null); } var blockFuture; + // Since we create a new Future, we need to manage the scope $apply + // explicitly instead of letting HTTP handle it. + // TODO(deboer): Move this $apply into a higher level of the framework + // (e.g. the compiler) + var onFutureDone = () {}; if (directive.$template != null) { blockFuture = new async.Future.value($blockCache.fromHtml(directive.$template)); + onFutureDone = () { + shadowInjector.get(Scope).$apply(); + }; } else if (directive.$templateUrl != null) { blockFuture = $blockCache.fromUrl(directive.$templateUrl); } @@ -308,8 +316,10 @@ class _ComponentFactory { shadowDom.innerHtml = ''; } if (blockFuture != null) { - return blockFuture.then((BlockFactory blockFactory) => - attachBlockToShadowDom(blockFactory)); + return blockFuture.then((BlockFactory blockFactory) { + attachBlockToShadowDom(blockFactory); + onFutureDone(); + }); } return shadowDom; })); @@ -324,7 +334,6 @@ class _ComponentFactory { attachBlockToShadowDom(BlockFactory blockFactory) { var block = blockFactory(shadowInjector); shadowDom.nodes.addAll(block.elements); - shadowInjector.get(Scope).$digest(); return shadowDom; } diff --git a/lib/directives/ng_include.dart b/lib/directives/ng_include.dart index a99c161fb..6893df6a5 100644 --- a/lib/directives/ng_include.dart +++ b/lib/directives/ng_include.dart @@ -45,9 +45,6 @@ class NgIncludeAttrDirective { // an url template blockCache.fromUrl(value).then((createBlock) { updateContent(createBlock); - - // Http should take care of this - scope.$digest(); }); } }); diff --git a/lib/http.dart b/lib/http.dart index 9944db996..b56d1059d 100644 --- a/lib/http.dart +++ b/lib/http.dart @@ -30,8 +30,9 @@ class Http { Map> _pendingRequests = >{}; UrlRewriter rewriter; HttpBackend backend; + Scope scope; - Http(UrlRewriter this.rewriter, HttpBackend this.backend); + Http(Scope this.scope, UrlRewriter this.rewriter, HttpBackend this.backend); async.Future getString(String url, {bool withCredentials, void onProgress(ProgressEvent e), Cache cache}) { @@ -55,9 +56,45 @@ class Http { if (cache != null && _pendingRequests.containsKey(url)) { return _pendingRequests[url]; } + + var requestFuture; + async.runZonedExperimental(() { + requestFuture = _requestUnguarded(url, + method: method, + withCredentials: withCredentials, + responseType: responseType, + mimeType: mimeType, + requestHeaders: requestHeaders, + sendData: sendData, + onProgress: onProgress, + cache: cache).then((x) { + // Disallow $digest inside of http handlers. + scope.$beginPhase('http'); + return x; + }); + }, onDone: () { + scope.$clearPhase(); + try { + scope.$apply(); + } catch (e, s) { + dump('Exception from HTTP, Dart may throw a cryptic error next: $e\n\n$s'); + rethrow; + } + }); + return requestFuture; + } + + async.Future _requestUnguarded(String url, + {String method, bool withCredentials, String responseType, + String mimeType, Map requestHeaders, sendData, + void onProgress(dom.ProgressEvent e), + Cache cache}) { + var cachedValue = cache != null ? cache.get(url) : null; if (cachedValue != null) { - return new async.Future.value(cachedValue); + // The then here forced runZoned's onDone handler to wait for the + // future to complete. + return new async.Future.value(cachedValue).then((x) => x); } var result = backend.request(url, method: method, diff --git a/lib/scope.dart b/lib/scope.dart index dbc1c7ac7..a623eef49 100644 --- a/lib/scope.dart +++ b/lib/scope.dart @@ -184,7 +184,7 @@ class Scope implements Map { Watch watch; Scope next, current, target = this; - _beginPhase('\$digest'); + $beginPhase('\$digest'); try { do { // "while dirty" loop dirty = false; @@ -255,7 +255,7 @@ class Scope implements Map { } } while (dirty || asyncQueue.length > 0); } finally { - _clearPhase(); + $clearPhase(); } } @@ -284,12 +284,12 @@ class Scope implements Map { $apply([expr]) { try { - _beginPhase('\$apply'); + $beginPhase('\$apply'); return $eval(expr); } catch (e, s) { _exceptionHandler(e, s); } finally { - _clearPhase(); + $clearPhase(); try { $root.$digest(); } catch (e, s) { @@ -393,15 +393,15 @@ class Scope implements Map { return event; } - _beginPhase(phase) { + $beginPhase(phase) { if ($root._phase != null) { - throw '${$root._phase} already in progress'; + throw '${$root._phase} already in progress trying to start $phase.'; } $root._phase = phase; } - _clearPhase() { + $clearPhase() { $root._phase = null; } diff --git a/test/_http.dart b/test/_http.dart index 848f48c80..a5e4493b0 100644 --- a/test/_http.dart +++ b/test/_http.dart @@ -8,14 +8,14 @@ class MockHttp extends Http { Map gets = {}; List futures = []; - MockHttp(UrlRewriter rewriter, HttpBackend backend) : super(rewriter, backend); + MockHttp(Scope scope, UrlRewriter rewriter, HttpBackend backend) : super(scope, rewriter, backend); expectGET(String url, String content, {int times: 1}) { gets[url] = new MockHttpData(content, times); } flush() => Future.wait(futures); - + assertAllGetsCalled() { if (gets.length != 0) { throw "Expected GETs not called $gets"; @@ -56,8 +56,9 @@ class MockHttpBackend extends HttpBackend { } flush() { - completersAndValues.forEach((cv) => cv[0].complete(cv[1])); + var toFlush = new List.from(completersAndValues); completersAndValues = []; + toFlush.forEach((cv) => runAsync(() => cv[0].complete(cv[1]))); } assertAllGetsCalled() { diff --git a/test/directives/ng_include_spec.dart b/test/directives/ng_include_spec.dart index ae46c9aca..2634ad453 100644 --- a/test/directives/ng_include_spec.dart +++ b/test/directives/ng_include_spec.dart @@ -20,7 +20,8 @@ main() { scope['template'] = 'tpl.html'; }); - nextTurn(); // load the template from cache. + nextTurn(true); // load the template from cache. + expect(element.text()).toEqual('my name is Vojta'); }))); diff --git a/test/http_spec.dart b/test/http_spec.dart index 9e0393d29..0857606c9 100644 --- a/test/http_spec.dart +++ b/test/http_spec.dart @@ -1,6 +1,8 @@ import "_specs.dart"; import "_http.dart"; +import "dart:async"; + var VALUE = 'val'; var CACHED_VALUE = 'cached_value'; @@ -31,7 +33,7 @@ main() { })); - it('should rewrite URLs before calling the backend', inject((Http http) { + it('should rewrite URLs before calling the backend', async(inject((Http http) { backend.expectGET('a', VALUE, times: 1); var called = 0; @@ -43,13 +45,14 @@ main() { expect(called).toEqual(0); backend.flush(); + nextTurn(true); expect(called).toEqual(1); backend.assertAllGetsCalled(); - })); + }))); - it('should support pending requests for different raw URLs', inject((Http http) { + it('should support pending requests for different raw URLs', async(inject((Http http) { backend.expectGET('a', VALUE, times: 1); var called = 0; @@ -64,9 +67,11 @@ main() { expect(called).toEqual(0); backend.flush(); + nextTurn(true); + expect(called).toEqual(11); backend.assertAllGetsCalled(); - })); + }))); it('should support caching', async(inject((Http http) { @@ -86,7 +91,7 @@ main() { }); describe('caching', () { - it('should not cache if no cache is present', inject((Http http) { + it('should not cache if no cache is present', async(inject((Http http) { backend.expectGET('a', VALUE, times: 2); var called = 0; @@ -102,13 +107,14 @@ main() { expect(called).toEqual(0); backend.flush(); + nextTurn(true); expect(called).toEqual(11); backend.assertAllGetsCalled(); - })); + }))); - it('should return a pending request', inject((Http http) { + it('should return a pending request', async(inject((Http http) { backend.expectGET('a', VALUE, times: 1); var called = 0; @@ -123,12 +129,14 @@ main() { expect(called).toEqual(0); backend.flush(); + nextTurn(true); + expect(called).toEqual(11); backend.assertAllGetsCalled(); - })); + }))); - it('should not return a pending request after the request is complete', inject((Http http) { + it('should not return a pending request after the request is complete', async(inject((Http http) { backend.expectGET('a', VALUE, times: 2); var called = 0; @@ -139,6 +147,7 @@ main() { expect(called).toEqual(0); backend.flush(); + nextTurn(true); http.getString('a', cache: cache).then((v) { expect(v).toBe(VALUE); @@ -147,9 +156,11 @@ main() { expect(called).toEqual(1); backend.flush(); + nextTurn(true); + expect(called).toEqual(11); backend.assertAllGetsCalled(); - })); + }))); it('should return a cached value if present', async(inject((Http http) { @@ -168,5 +179,95 @@ main() { backend.assertAllGetsCalled(); }))); }); + + describe('scope digesting', () { + it('should digest scope after a request', async(inject((Scope scope, Http http) { + backend.expectGET('a', 'value'); + + scope.$watch('fromHttp', (v) { + scope.digested = v; + }); + + http.getString('a').then((v) { + scope.fromHttp = v; + }); + + backend.flush(); + nextTurn(true); + + expect(scope.digested).toEqual('value'); + }))); + + + it('should digest scope after a cached request', async(inject((Scope scope, Http http) { + scope.$watch('fromHttp', (v) { + scope.digested = v; + }); + + http.getString('f', cache: cache).then((v) { + scope.fromHttp = v; + }); + + backend.flush(); + nextTurn(true); + + expect(scope.digested).toEqual('cached_value'); + }))); + + + it('should digest twice for chained requests', async(inject((Scope scope, Http http) { + backend.expectGET('a', 'value'); + backend.expectGET('b', 'bval'); + + scope.$watch('fromHttp', (v) { + scope.digested = v; + }); + + http.getString('a').then((v) { + scope.fromHttp = 'A:$v'; + http.getString('b').then((vb) { + scope.fromHttp += ' B:$vb'; + }); + }); + + backend.flush(); + nextTurn(true); + + expect(scope.digested).toEqual('A:value'); + + backend.flush(); + nextTurn(true); + + expect(scope.digested).toEqual('A:value B:bval'); + }))); + + + it('should NOT digest after an chained runAsync', async(inject((Scope scope, Http http) { + backend.expectGET('a', 'value'); + + scope.$watch('fromHttp', (v) { + scope.digested = v; + }); + + http.getString('a').then((v) { + scope.fromHttp = 'then:$v'; + runAsync(() { + // This is an example of a bug. If you use runAsync, + // you are responsible for digesting the scope. + // In general, don't use runAsync! + scope.fromHttp = 'async:$v'; + }); + }); + + backend.flush(); + nextTurn(true); + + expect(scope.digested).toEqual('then:value'); + + // Note that the runAsync has run, but it was not digested. + scope.$digest(); + expect(scope.digested).toEqual('async:value'); + }))); + }); }); } diff --git a/test/templateurl_spec.dart b/test/templateurl_spec.dart index 3f2080103..aa67b8b44 100644 --- a/test/templateurl_spec.dart +++ b/test/templateurl_spec.dart @@ -41,7 +41,7 @@ main() { ..type(UrlRewriter, PrefixedUrlRewriter); })); - it('should use the UrlRewriter for both HTML and CSS URLs', inject((Http $http, Compiler $compile, Scope $rootScope, Log log, Injector injector) { + it('should use the UrlRewriter for both HTML and CSS URLs', async(inject((Http $http, Compiler $compile, Scope $rootScope, Log log, Injector injector) { backend.expectGET('PREFIX:simple.html', '
Simple!
'); backend.expectGET('PREFIX:simple.css', '.hello{}'); @@ -50,13 +50,14 @@ main() { $compile(element)(injector, element); backend.flush(); + nextTurn(true); backend.assertAllGetsCalled(); expect(renderedText(element)).toEqual('.hello{}Simple!'); expect(element[0].nodes[0].shadowRoot.innerHtml).toEqual( '
Simple!
' ); - })); + }))); });