Skip to content

Commit 9619636

Browse files
jteplitzjteplitz
committed
fix(WebWorker): WebWorkerRenderer removes views after they're destroyed
closes #3240 Closes #3894
1 parent fa2c679 commit 9619636

5 files changed

Lines changed: 62 additions & 19 deletions

File tree

‎modules/angular2/src/web_workers/shared/render_view_with_fragments_store.ts‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,21 @@ import {
55
RenderViewWithFragments
66
} from "angular2/src/core/render/api";
77
import {ON_WEB_WORKER} from "angular2/src/web_workers/shared/api";
8-
import {List, ListWrapper} from "angular2/src/core/facade/collection";
8+
import {List, MapWrapper, ListWrapper} from "angular2/src/core/facade/collection";
99

1010
@Injectable()
1111
export class RenderViewWithFragmentsStore {
1212
private _nextIndex: number = 0;
1313
private _onWebWorker: boolean;
1414
private _lookupByIndex: Map<number, RenderViewRef | RenderFragmentRef>;
1515
private _lookupByView: Map<RenderViewRef | RenderFragmentRef, number>;
16+
private _viewFragments: Map<RenderViewRef, List<RenderFragmentRef>>;
1617

1718
constructor(@Inject(ON_WEB_WORKER) onWebWorker) {
1819
this._onWebWorker = onWebWorker;
1920
this._lookupByIndex = new Map<number, RenderViewRef | RenderFragmentRef>();
2021
this._lookupByView = new Map<RenderViewRef | RenderFragmentRef, number>();
22+
this._viewFragments = new Map<RenderViewRef, List<RenderFragmentRef>>();
2123
}
2224

2325
allocate(fragmentCount: number): RenderViewWithFragments {
@@ -34,7 +36,7 @@ export class RenderViewWithFragmentsStore {
3436
return renderViewWithFragments;
3537
}
3638

37-
store(view: RenderViewWithFragments, startIndex: number) {
39+
store(view: RenderViewWithFragments, startIndex: number): void {
3840
this._lookupByIndex.set(startIndex, view.viewRef);
3941
this._lookupByView.set(view.viewRef, startIndex);
4042
startIndex++;
@@ -44,13 +46,21 @@ export class RenderViewWithFragmentsStore {
4446
this._lookupByView.set(ref, startIndex);
4547
startIndex++;
4648
});
49+
50+
this._viewFragments.set(view.viewRef, view.fragmentRefs);
4751
}
4852

49-
retreive(ref: number): RenderViewRef | RenderFragmentRef {
50-
if (ref == null) {
51-
return null;
52-
}
53-
return this._lookupByIndex.get(ref);
53+
remove(view: RenderViewRef): void {
54+
this._removeRef(view);
55+
var fragments = this._viewFragments.get(view);
56+
fragments.forEach((fragment) => { this._removeRef(fragment); });
57+
MapWrapper.delete(this._viewFragments, view);
58+
}
59+
60+
private _removeRef(ref: RenderViewRef | RenderFragmentRef) {
61+
var index = this._lookupByView.get(ref);
62+
MapWrapper.delete(this._lookupByView, ref);
63+
MapWrapper.delete(this._lookupByIndex, index);
5464
}
5565

5666
serializeRenderViewRef(viewRef: RenderViewRef): number {
@@ -66,17 +76,30 @@ export class RenderViewWithFragmentsStore {
6676
return null;
6777
}
6878

69-
return this.retreive(ref);
79+
return this._retrieve(ref);
7080
}
7181

7282
deserializeRenderFragmentRef(ref: number): RenderFragmentRef {
7383
if (ref == null) {
7484
return null;
7585
}
7686

77-
return this.retreive(ref);
87+
return this._retrieve(ref);
88+
}
89+
90+
private _retrieve(ref: number): RenderViewRef | RenderFragmentRef {
91+
if (ref == null) {
92+
return null;
93+
}
94+
95+
if (!this._lookupByIndex.has(ref)) {
96+
return null;
97+
}
98+
99+
return this._lookupByIndex.get(ref);
78100
}
79101

102+
80103
private _serializeRenderFragmentOrViewRef(ref: RenderViewRef | RenderFragmentRef): number {
81104
if (ref == null) {
82105
return null;

‎modules/angular2/src/web_workers/ui/renderer.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ export class MessageBasedRenderer {
2929
bind(this._createRootHostView, this));
3030
broker.registerMethod("createView", [RenderProtoViewRef, PRIMITIVE, PRIMITIVE],
3131
bind(this._createView, this));
32-
broker.registerMethod("destroyView", [RenderViewRef],
33-
bind(this._renderer.destroyView, this._renderer));
32+
broker.registerMethod("destroyView", [RenderViewRef], bind(this._destroyView, this));
3433
broker.registerMethod("attachFragmentAfterFragment", [RenderFragmentRef, RenderFragmentRef],
3534
bind(this._renderer.attachFragmentAfterFragment, this._renderer));
3635
broker.registerMethod("attachFragmentAfterElement", [WebWorkerElementRef, RenderFragmentRef],
@@ -57,6 +56,11 @@ export class MessageBasedRenderer {
5756
bind(this._setEventDispatcher, this));
5857
}
5958

59+
private _destroyView(viewRef: RenderViewRef): void {
60+
this._renderer.destroyView(viewRef);
61+
this._renderViewWithFragmentsStore.remove(viewRef);
62+
}
63+
6064
private _createRootHostView(ref: RenderProtoViewRef, fragmentCount: number, selector: string,
6165
startIndex: number) {
6266
var renderViewWithFragments = this._renderer.createRootHostView(ref, fragmentCount, selector);

‎modules/angular2/src/web_workers/worker/renderer.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export class WebWorkerRenderer implements Renderer {
138138
var fnArgs = [new FnArg(viewRef, RenderViewRef)];
139139
var args = new UiArguments("destroyView", fnArgs);
140140
this._messageBroker.runOnUiThread(args, null);
141+
this._renderViewStore.remove(viewRef);
141142
}
142143

143144
/**

‎modules/angular2/test/web_workers/shared/render_view_with_fragments_store_spec.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {List, ListWrapper} from "angular2/src/core/facade/collection";
1414
export function main() {
1515
describe("RenderViewWithFragmentsStore", () => {
1616
describe("on WebWorker", () => {
17-
var store;
17+
var store: RenderViewWithFragmentsStore;
1818
beforeEach(() => { store = new RenderViewWithFragmentsStore(true); });
1919

2020
it("should allocate fragmentCount + 1 refs", () => {
@@ -44,10 +44,22 @@ export function main() {
4444
expect(store.deserializeViewWithFragments(store.serializeViewWithFragments(view)))
4545
.toEqual(view);
4646
});
47+
48+
it("should remove a view and all attached fragments", () => {
49+
const NUM_FRAGMENTS = 5;
50+
var view = store.allocate(NUM_FRAGMENTS);
51+
var viewRef = (<WebWorkerRenderViewRef>view.viewRef).refNumber;
52+
store.remove(view.viewRef);
53+
54+
expect(store.deserializeRenderViewRef(viewRef++)).toBeNull();
55+
for (var i = 0; i < NUM_FRAGMENTS; i++) {
56+
expect(store.deserializeRenderFragmentRef(viewRef++)).toBeNull();
57+
}
58+
});
4759
});
4860

4961
describe("on UI", () => {
50-
var store;
62+
var store: RenderViewWithFragmentsStore;
5163
beforeEach(() => { store = new RenderViewWithFragmentsStore(false); });
5264
function createMockRenderViewWithFragments(): RenderViewWithFragments {
5365
var view = new MockRenderViewRef();
@@ -62,10 +74,11 @@ export function main() {
6274
var renderViewWithFragments = createMockRenderViewWithFragments();
6375

6476
store.store(renderViewWithFragments, 100);
65-
expect(store.retreive(100)).toBe(renderViewWithFragments.viewRef);
77+
expect(store.deserializeRenderViewRef(100)).toBe(renderViewWithFragments.viewRef);
6678

6779
for (var i = 0; i < renderViewWithFragments.fragmentRefs.length; i++) {
68-
expect(store.retreive(101 + i)).toBe(renderViewWithFragments.fragmentRefs[i]);
80+
expect(store.deserializeRenderFragmentRef(101 + i))
81+
.toBe(renderViewWithFragments.fragmentRefs[i]);
6982
}
7083
});
7184

‎modules/angular2/test/web_workers/worker/renderer_integration_spec.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,12 +328,14 @@ export function main() {
328328
}
329329

330330
class WorkerTestRootView extends TestRootView {
331-
constructor(workerViewWithFragments: RenderViewWithFragments, uiRenderViewStore) {
331+
constructor(workerViewWithFragments: RenderViewWithFragments,
332+
uiRenderViewStore: RenderViewWithFragmentsStore) {
332333
super(new RenderViewWithFragments(
333-
uiRenderViewStore.retreive(
334+
uiRenderViewStore.deserializeRenderViewRef(
334335
(<WebWorkerRenderViewRef>workerViewWithFragments.viewRef).refNumber),
335-
ListWrapper.map(workerViewWithFragments.fragmentRefs,
336-
(val) => { return uiRenderViewStore.retreive(val.refNumber); })));
336+
ListWrapper.map(workerViewWithFragments.fragmentRefs, (val) => {
337+
return uiRenderViewStore.deserializeRenderFragmentRef(val.refNumber);
338+
})));
337339
}
338340
}
339341

0 commit comments

Comments
 (0)