From b7f175bfa9b2786a58718d544fc75c867cd2fa40 Mon Sep 17 00:00:00 2001 From: Chirayu Krishnappa Date: Fri, 30 May 2014 12:04:01 -0700 Subject: [PATCH] fix(element binder): fix memory leak with expando value holding onto node Expando keys are weak references but the values are not. The ElementProbe value holds strong references to the node and the injector keep it from being garbage collected. --- lib/core_dom/element_binder.dart | 1 + .../shadow_dom_component_factory.dart | 5 +-- test/core_dom/compiler_spec.dart | 33 ++++++++++++++++++- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/lib/core_dom/element_binder.dart b/lib/core_dom/element_binder.dart index 85103f309..d923a4736 100644 --- a/lib/core_dom/element_binder.dart +++ b/lib/core_dom/element_binder.dart @@ -319,6 +319,7 @@ class ElementBinder { nodeInjector = parentInjector.createChild([nodeModule]); probe = _expando[node] = new ElementProbe( parentInjector.getByKey(_ELEMENT_PROBE_KEY), node, nodeInjector, scope); + scope.on(ScopeEvent.DESTROY).listen((_) {_expando[node] = null;}); _link(nodeInjector, probe, scope, nodeAttrs, formatters); diff --git a/lib/core_dom/shadow_dom_component_factory.dart b/lib/core_dom/shadow_dom_component_factory.dart index 15807eb79..54bb4aa38 100644 --- a/lib/core_dom/shadow_dom_component_factory.dart +++ b/lib/core_dom/shadow_dom_component_factory.dart @@ -179,7 +179,7 @@ class _ComponentFactory implements Function { Injector createShadowInjector(injector, TemplateLoader templateLoader) { var probe; - var shadowModule = new Module() + var shadowModule = new Module() ..bindByKey(typeKey) ..bindByKey(_NG_ELEMENT_KEY) ..bindByKey(_EVENT_HANDLER_KEY, toImplementation: ShadowRootEventHandler) @@ -187,9 +187,10 @@ class _ComponentFactory implements Function { ..bindByKey(_TEMPLATE_LOADER_KEY, toValue: templateLoader) ..bindByKey(_SHADOW_ROOT_KEY, toValue: shadowDom) ..bindByKey(_ELEMENT_PROBE, toFactory: (_) => probe); - shadowInjector = injector.createChild([shadowModule], name: SHADOW_DOM_INJECTOR_NAME); + shadowInjector = injector.createChild([shadowModule], name: SHADOW_DOM_INJECTOR_NAME); probe = _expando[shadowDom] = new ElementProbe( injector.getByKey(_ELEMENT_PROBE), shadowDom, shadowInjector, shadowScope); + shadowScope.on(ScopeEvent.DESTROY).listen((ScopeEvent) {_expando[shadowDom] = null;}); return shadowInjector; } } diff --git a/test/core_dom/compiler_spec.dart b/test/core_dom/compiler_spec.dart index fab93e00b..02bd7018d 100644 --- a/test/core_dom/compiler_spec.dart +++ b/test/core_dom/compiler_spec.dart @@ -727,6 +727,28 @@ void main() { expect(_.rootElement.shadowRoot).toBeNotNull(); } })); + + describe('expando memory', () { + Expando expando; + + beforeEach(inject((Expando _expando) => expando = _expando)); + + ['shadowy', 'shadowless'].forEach((selector) { + it('should release expando when a node is freed ($selector)', async(() { + _.rootScope.context['flag'] = true; + _.compile('
<$selector>x
'); + microLeap(); _.rootScope.apply(); + var element = _.rootElement.querySelector('$selector'); + if (element.shadowRoot != null) { + element = element.shadowRoot; + } + expect(expando[element]).not.toEqual(null); + _.rootScope.context['flag'] = false; + microLeap(); _.rootScope.apply(); + expect(expando[element]).toEqual(null); + })); + }); + }); }); describe('bindings', () { @@ -849,7 +871,6 @@ void main() { expect(_.rootElement.text).toEqual('MyController'); }); }); - })); } @@ -975,6 +996,16 @@ class SimpleComponent { } } +@Component( + selector: 'simple2', + template: r'{{name}}(SHADOW-CONTENT)') +class Simple2Component { + Scope scope; + Simple2Component(Scope this.scope) { + scope.context['name'] = 'INNER'; + } +} + @Component( selector: 'shadowy', template: r'With shadow DOM',