From 9619636ba7c89f112fab492df6a4d4d230a4fc35 Mon Sep 17 00:00:00 2001 From: Jason Teplitz Date: Thu, 27 Aug 2015 10:39:39 -0700 Subject: [PATCH] fix(WebWorker): WebWorkerRenderer removes views after they're destroyed closes #3240 Closes #3894 --- .../render_view_with_fragments_store.ts | 41 +++++++++++++++---- .../angular2/src/web_workers/ui/renderer.ts | 8 +++- .../src/web_workers/worker/renderer.ts | 1 + .../render_view_with_fragments_store_spec.ts | 21 ++++++++-- .../worker/renderer_integration_spec.ts | 10 +++-- 5 files changed, 62 insertions(+), 19 deletions(-) diff --git a/modules/angular2/src/web_workers/shared/render_view_with_fragments_store.ts b/modules/angular2/src/web_workers/shared/render_view_with_fragments_store.ts index 7435c4b51f..49ef5de9ae 100644 --- a/modules/angular2/src/web_workers/shared/render_view_with_fragments_store.ts +++ b/modules/angular2/src/web_workers/shared/render_view_with_fragments_store.ts @@ -5,7 +5,7 @@ import { RenderViewWithFragments } from "angular2/src/core/render/api"; import {ON_WEB_WORKER} from "angular2/src/web_workers/shared/api"; -import {List, ListWrapper} from "angular2/src/core/facade/collection"; +import {List, MapWrapper, ListWrapper} from "angular2/src/core/facade/collection"; @Injectable() export class RenderViewWithFragmentsStore { @@ -13,11 +13,13 @@ export class RenderViewWithFragmentsStore { private _onWebWorker: boolean; private _lookupByIndex: Map; private _lookupByView: Map; + private _viewFragments: Map>; constructor(@Inject(ON_WEB_WORKER) onWebWorker) { this._onWebWorker = onWebWorker; this._lookupByIndex = new Map(); this._lookupByView = new Map(); + this._viewFragments = new Map>(); } allocate(fragmentCount: number): RenderViewWithFragments { @@ -34,7 +36,7 @@ export class RenderViewWithFragmentsStore { return renderViewWithFragments; } - store(view: RenderViewWithFragments, startIndex: number) { + store(view: RenderViewWithFragments, startIndex: number): void { this._lookupByIndex.set(startIndex, view.viewRef); this._lookupByView.set(view.viewRef, startIndex); startIndex++; @@ -44,13 +46,21 @@ export class RenderViewWithFragmentsStore { this._lookupByView.set(ref, startIndex); startIndex++; }); + + this._viewFragments.set(view.viewRef, view.fragmentRefs); } - retreive(ref: number): RenderViewRef | RenderFragmentRef { - if (ref == null) { - return null; - } - return this._lookupByIndex.get(ref); + remove(view: RenderViewRef): void { + this._removeRef(view); + var fragments = this._viewFragments.get(view); + fragments.forEach((fragment) => { this._removeRef(fragment); }); + MapWrapper.delete(this._viewFragments, view); + } + + private _removeRef(ref: RenderViewRef | RenderFragmentRef) { + var index = this._lookupByView.get(ref); + MapWrapper.delete(this._lookupByView, ref); + MapWrapper.delete(this._lookupByIndex, index); } serializeRenderViewRef(viewRef: RenderViewRef): number { @@ -66,7 +76,7 @@ export class RenderViewWithFragmentsStore { return null; } - return this.retreive(ref); + return this._retrieve(ref); } deserializeRenderFragmentRef(ref: number): RenderFragmentRef { @@ -74,9 +84,22 @@ export class RenderViewWithFragmentsStore { return null; } - return this.retreive(ref); + return this._retrieve(ref); } + private _retrieve(ref: number): RenderViewRef | RenderFragmentRef { + if (ref == null) { + return null; + } + + if (!this._lookupByIndex.has(ref)) { + return null; + } + + return this._lookupByIndex.get(ref); + } + + private _serializeRenderFragmentOrViewRef(ref: RenderViewRef | RenderFragmentRef): number { if (ref == null) { return null; diff --git a/modules/angular2/src/web_workers/ui/renderer.ts b/modules/angular2/src/web_workers/ui/renderer.ts index 53053642df..6ad781f4f9 100644 --- a/modules/angular2/src/web_workers/ui/renderer.ts +++ b/modules/angular2/src/web_workers/ui/renderer.ts @@ -29,8 +29,7 @@ export class MessageBasedRenderer { bind(this._createRootHostView, this)); broker.registerMethod("createView", [RenderProtoViewRef, PRIMITIVE, PRIMITIVE], bind(this._createView, this)); - broker.registerMethod("destroyView", [RenderViewRef], - bind(this._renderer.destroyView, this._renderer)); + broker.registerMethod("destroyView", [RenderViewRef], bind(this._destroyView, this)); broker.registerMethod("attachFragmentAfterFragment", [RenderFragmentRef, RenderFragmentRef], bind(this._renderer.attachFragmentAfterFragment, this._renderer)); broker.registerMethod("attachFragmentAfterElement", [WebWorkerElementRef, RenderFragmentRef], @@ -57,6 +56,11 @@ export class MessageBasedRenderer { bind(this._setEventDispatcher, this)); } + private _destroyView(viewRef: RenderViewRef): void { + this._renderer.destroyView(viewRef); + this._renderViewWithFragmentsStore.remove(viewRef); + } + private _createRootHostView(ref: RenderProtoViewRef, fragmentCount: number, selector: string, startIndex: number) { var renderViewWithFragments = this._renderer.createRootHostView(ref, fragmentCount, selector); diff --git a/modules/angular2/src/web_workers/worker/renderer.ts b/modules/angular2/src/web_workers/worker/renderer.ts index 59416f7606..77fb65478f 100644 --- a/modules/angular2/src/web_workers/worker/renderer.ts +++ b/modules/angular2/src/web_workers/worker/renderer.ts @@ -138,6 +138,7 @@ export class WebWorkerRenderer implements Renderer { var fnArgs = [new FnArg(viewRef, RenderViewRef)]; var args = new UiArguments("destroyView", fnArgs); this._messageBroker.runOnUiThread(args, null); + this._renderViewStore.remove(viewRef); } /** diff --git a/modules/angular2/test/web_workers/shared/render_view_with_fragments_store_spec.ts b/modules/angular2/test/web_workers/shared/render_view_with_fragments_store_spec.ts index fad2db235c..b8436aa610 100644 --- a/modules/angular2/test/web_workers/shared/render_view_with_fragments_store_spec.ts +++ b/modules/angular2/test/web_workers/shared/render_view_with_fragments_store_spec.ts @@ -14,7 +14,7 @@ import {List, ListWrapper} from "angular2/src/core/facade/collection"; export function main() { describe("RenderViewWithFragmentsStore", () => { describe("on WebWorker", () => { - var store; + var store: RenderViewWithFragmentsStore; beforeEach(() => { store = new RenderViewWithFragmentsStore(true); }); it("should allocate fragmentCount + 1 refs", () => { @@ -44,10 +44,22 @@ export function main() { expect(store.deserializeViewWithFragments(store.serializeViewWithFragments(view))) .toEqual(view); }); + + it("should remove a view and all attached fragments", () => { + const NUM_FRAGMENTS = 5; + var view = store.allocate(NUM_FRAGMENTS); + var viewRef = (view.viewRef).refNumber; + store.remove(view.viewRef); + + expect(store.deserializeRenderViewRef(viewRef++)).toBeNull(); + for (var i = 0; i < NUM_FRAGMENTS; i++) { + expect(store.deserializeRenderFragmentRef(viewRef++)).toBeNull(); + } + }); }); describe("on UI", () => { - var store; + var store: RenderViewWithFragmentsStore; beforeEach(() => { store = new RenderViewWithFragmentsStore(false); }); function createMockRenderViewWithFragments(): RenderViewWithFragments { var view = new MockRenderViewRef(); @@ -62,10 +74,11 @@ export function main() { var renderViewWithFragments = createMockRenderViewWithFragments(); store.store(renderViewWithFragments, 100); - expect(store.retreive(100)).toBe(renderViewWithFragments.viewRef); + expect(store.deserializeRenderViewRef(100)).toBe(renderViewWithFragments.viewRef); for (var i = 0; i < renderViewWithFragments.fragmentRefs.length; i++) { - expect(store.retreive(101 + i)).toBe(renderViewWithFragments.fragmentRefs[i]); + expect(store.deserializeRenderFragmentRef(101 + i)) + .toBe(renderViewWithFragments.fragmentRefs[i]); } }); diff --git a/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts b/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts index 4a48389518..f14c35f5e8 100644 --- a/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts +++ b/modules/angular2/test/web_workers/worker/renderer_integration_spec.ts @@ -328,12 +328,14 @@ export function main() { } class WorkerTestRootView extends TestRootView { - constructor(workerViewWithFragments: RenderViewWithFragments, uiRenderViewStore) { + constructor(workerViewWithFragments: RenderViewWithFragments, + uiRenderViewStore: RenderViewWithFragmentsStore) { super(new RenderViewWithFragments( - uiRenderViewStore.retreive( + uiRenderViewStore.deserializeRenderViewRef( (workerViewWithFragments.viewRef).refNumber), - ListWrapper.map(workerViewWithFragments.fragmentRefs, - (val) => { return uiRenderViewStore.retreive(val.refNumber); }))); + ListWrapper.map(workerViewWithFragments.fragmentRefs, (val) => { + return uiRenderViewStore.deserializeRenderFragmentRef(val.refNumber); + }))); } }