Skip to content

Commit

Permalink
fix(WebWorker): WebWorkerRenderer removes views after they're destroyed
Browse files Browse the repository at this point in the history
closes #3240

Closes #3894
  • Loading branch information
jteplitz committed Aug 31, 2015
1 parent fa2c679 commit 9619636
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@ 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 {
private _nextIndex: number = 0;
private _onWebWorker: boolean;
private _lookupByIndex: Map<number, RenderViewRef | RenderFragmentRef>;
private _lookupByView: Map<RenderViewRef | RenderFragmentRef, number>;
private _viewFragments: Map<RenderViewRef, List<RenderFragmentRef>>;

constructor(@Inject(ON_WEB_WORKER) onWebWorker) {
this._onWebWorker = onWebWorker;
this._lookupByIndex = new Map<number, RenderViewRef | RenderFragmentRef>();
this._lookupByView = new Map<RenderViewRef | RenderFragmentRef, number>();
this._viewFragments = new Map<RenderViewRef, List<RenderFragmentRef>>();
}

allocate(fragmentCount: number): RenderViewWithFragments {
Expand All @@ -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++;
Expand All @@ -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 {
Expand All @@ -66,17 +76,30 @@ export class RenderViewWithFragmentsStore {
return null;
}

return this.retreive(ref);
return this._retrieve(ref);
}

deserializeRenderFragmentRef(ref: number): RenderFragmentRef {
if (ref == null) {
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;
Expand Down
8 changes: 6 additions & 2 deletions modules/angular2/src/web_workers/ui/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions modules/angular2/src/web_workers/worker/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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 = (<WebWorkerRenderViewRef>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();
Expand All @@ -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]);
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
(<WebWorkerRenderViewRef>workerViewWithFragments.viewRef).refNumber),
ListWrapper.map(workerViewWithFragments.fragmentRefs,
(val) => { return uiRenderViewStore.retreive(val.refNumber); })));
ListWrapper.map(workerViewWithFragments.fragmentRefs, (val) => {
return uiRenderViewStore.deserializeRenderFragmentRef(val.refNumber);
})));
}
}

Expand Down

0 comments on commit 9619636

Please sign in to comment.