Skip to content

fix(WebWorker): WebWorkerRenderer removes views after they're destroyed #3894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jteplitz wants to merge 1 commit into from
Closed

fix(WebWorker): WebWorkerRenderer removes views after they're destroyed #3894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions modules/angular2/src/web_workers/shared/render_view_with_fragments_store.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
21 changes: 17 additions & 4 deletions modules/angular2/test/web_workers/shared/render_view_with_fragments_store_spec.ts
View file
Open in desktop
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
10 changes: 6 additions & 4 deletions modules/angular2/test/web_workers/worker/renderer_integration_spec.ts
View file
Open in desktop
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