fix(platform-webworker): integrate review feedback

closes #14581

modules/@angular/platform-webworker/src/web_workers/shared/client_message_broker.ts

@@ -67,7 +67,7 @@ export class ClientMessageBroker_ extends ClientMessageBroker {
     this._serializer = _serializer;
     const source = messageBus.from(channel);
 
-    source.subscribe({next: (message: MessageData) => this._handleMessage(message)});
+    source.subscribe({next: (message: ResponseMessageData) => this._handleMessage(message)});
   }
 
   private _generateMessageId(name: string): string {
@@ -102,31 +102,33 @@ export class ClientMessageBroker_ extends ClientMessageBroker {
       this._pending.set(id, completer);
 
       promise.catch((err) => {
-        if (console && console.log) {
+        if (console && console.error) {
           // tslint:disable-next-line:no-console
-          console.log(err);
+          console.error(err);
         }
 
         completer.reject(err);
       });
 
       promise = promise.then(
-          (value: any) =>
+          (v: any) => this._serializer ? this._serializer.deserialize(v, returnType) : v);
-              this._serializer ? value : this._serializer.deserialize(value, returnType));
     } else {
       promise = null;
     }
 
-    const message = {'method': args.method, 'args': fnArgs};
+    const message: RequestMessageData = {
+      'method': args.method,
+      'args': fnArgs,
+    };
     if (id != null) {
-      (message as any)['id'] = id;
+      message['id'] = id;
     }
     this._sink.emit(message);
 
     return promise;
   }
 
-  private _handleMessage(message: MessageData): void {
+  private _handleMessage(message: ResponseMessageData): void {
     if (message.type === 'result' || message.type === 'error') {
       const id = message.id;
       if (this._pending.has(id)) {
@@ -141,7 +143,13 @@ export class ClientMessageBroker_ extends ClientMessageBroker {
   }
 }
 
-interface MessageData {
+interface RequestMessageData {
+  method: string;
+  args?: any[];
+  id?: string;
+}
+
+interface ResponseMessageData {
   type: 'result'|'error';
   value?: any;
   id?: string;

modules/@angular/platform-webworker/src/web_workers/shared/render_store.ts

@@ -17,18 +17,21 @@ export class RenderStore {
   allocateId(): number { return this._nextIndex++; }
 
   store(obj: any, id: number): void {
+    if (id == null) return;
     this._lookupById.set(id, obj);
     this._lookupByObject.set(obj, id);
   }
 
   remove(obj: any): void {
     const index = this._lookupByObject.get(obj);
-    this._lookupByObject.delete(obj);
+    if (index != null) {
-    this._lookupById.delete(index);
+      this._lookupByObject.delete(obj);
+      this._lookupById.delete(index);
+    }
   }
 
   deserialize(id: number): any {
-    return id == null || !this._lookupById.has(id) ? null : this._lookupById.get(id);
+    return this._lookupById.has(id) ? this._lookupById.get(id) : null;
   }
 
   serialize(obj: any): number { return obj == null ? null : this._lookupByObject.get(obj); }

modules/@angular/platform-webworker/src/web_workers/shared/serializer.ts

@@ -19,7 +19,7 @@ import {RenderStore} from './render_store';
  * @experimental WebWorker support in Angular is currently experimental.
  * @deprecated in v4. Use SerializerTypes.PRIMITIVE instead
  */
-export const PRIMITIVE: Type<any> = String;
+export const PRIMITIVE = SerializerTypes.PRIMITIVE;
 
 export class LocationType {
   constructor(
@@ -45,7 +45,7 @@ export class Serializer {
   constructor(private _renderStore: RenderStore) {}
 
   serialize(obj: any, type: Type<any>|SerializerTypes = SerializerTypes.PRIMITIVE): Object {
-    if (obj == null || type === PRIMITIVE || type === SerializerTypes.PRIMITIVE) {
+    if (obj == null || type === SerializerTypes.PRIMITIVE) {
       return obj;
     }
     if (Array.isArray(obj)) {
@@ -68,7 +68,7 @@ export class Serializer {
 
   deserialize(map: any, type: Type<any>|SerializerTypes = SerializerTypes.PRIMITIVE, data?: any):
       any {
-    if (map == null || type === PRIMITIVE || type === SerializerTypes.PRIMITIVE) {
+    if (map == null || type === SerializerTypes.PRIMITIVE) {
       return map;
     }
     if (Array.isArray(map)) {

modules/@angular/platform-webworker/src/web_workers/ui/renderer.ts

@@ -265,24 +265,21 @@ export class MessageBasedRendererV2 {
     const methods: any[][] = [
       ['createRenderer', this.createRenderer, RSO, CRT, P],
       ['createElement', this.createElement, RSO, P, P, P],
-      ['createComment', this.createComment, RSO, P, P],
+      ['createComment', this.createComment, RSO, P, P], ['createText', this.createText, RSO, P, P],
-      ['createText', this.createText, RSO, P, P],
       ['appendChild', this.appendChild, RSO, RSO, RSO],
       ['insertBefore', this.insertBefore, RSO, RSO, RSO, RSO],
       ['removeChild', this.removeChild, RSO, RSO, RSO],
       ['selectRootElement', this.selectRootElement, RSO, P, P],
-      ['parentNode', this.parentNode, RSO, RSO, P],
+      ['parentNode', this.parentNode, RSO, RSO, P], ['nextSibling', this.nextSibling, RSO, RSO, P],
-      ['nextSibling', this.nextSibling, RSO, RSO, P],
       ['setAttribute', this.setAttribute, RSO, RSO, P, P, P],
       ['removeAttribute', this.removeAttribute, RSO, RSO, P, P],
-      ['addClass', this.addClass, RSO, RSO, P],
+      ['addClass', this.addClass, RSO, RSO, P], ['removeClass', this.removeClass, RSO, RSO, P],
-      ['removeClass', this.removeClass, RSO, RSO, P],
       ['setStyle', this.setStyle, RSO, RSO, P, P, P, P],
       ['removeStyle', this.removeStyle, RSO, RSO, P, P],
-      ['setProperty', this.setProperty, RSO, RSO, P, P],
+      ['setProperty', this.setProperty, RSO, RSO, P, P], ['setValue', this.setValue, RSO, RSO, P],
-      ['setValue', this.setValue, RSO, RSO, P],
+      ['listen', this.listen, RSO, RSO, P, P, P], ['unlisten', this.unlisten, RSO, RSO],
-      ['listen', this.listen, RSO, RSO, P, P, P],
+      ['destroy', this.destroy, RSO], ['destroyNode', this.destroyNode, RSO, P]
-      ['unlisten', this.unlisten, RSO, RSO],
+
     ];
 
     methods.forEach(([name, method, ...argTypes]: any[]) => {
@@ -290,6 +287,15 @@ export class MessageBasedRendererV2 {
     });
   }
 
+  private destroy(r: RendererV2) { r.destroy(); }
+
+  private destroyNode(r: RendererV2, node: any) {
+    if (r.destroyNode) {
+      r.destroyNode(node);
+    }
+    this._renderStore.remove(node);
+  }
+
   private createRenderer(el: any, type: RendererTypeV2, id: number) {
     this._renderStore.store(this._rendererFactory.createRenderer(el, type), id);
   }

modules/@angular/platform-webworker/src/web_workers/worker/renderer.ts

@@ -48,10 +48,10 @@ export class NamedEventEmitter {
   }
 }
 
-const globalEvents = new NamedEventEmitter();
-
 @Injectable()
 export class WebWorkerRootRenderer implements RootRenderer {
+  globalEvents = new NamedEventEmitter();
+
   private _messageBroker: ClientMessageBroker;
   private _componentRenderers = new Map<string, WebWorkerRenderer>();
 
@@ -79,7 +79,7 @@ export class WebWorkerRootRenderer implements RootRenderer {
       const target = message['eventTarget'];
       const event = message['event'];
       if (target) {
-        globalEvents.dispatchEvent(eventNameWithTarget(target, eventName), event);
+        this.globalEvents.dispatchEvent(eventNameWithTarget(target, eventName), event);
       } else {
         element.events.dispatchEvent(eventName, event);
       }
@@ -277,7 +277,7 @@ export class WebWorkerRenderer implements Renderer {
   }
 
   listenGlobal(target: string, name: string, callback: Function): Function {
-    globalEvents.listen(eventNameWithTarget(target, name), callback);
+    this._rootRenderer.globalEvents.listen(eventNameWithTarget(target, name), callback);
     const unlistenCallbackId = this._rootRenderer.allocateId();
     this._runOnService('listenGlobal', [
       new FnArg(target),
@@ -285,7 +285,7 @@ export class WebWorkerRenderer implements Renderer {
       new FnArg(unlistenCallbackId),
     ]);
     return () => {
-      globalEvents.unlisten(eventNameWithTarget(target, name), callback);
+      this._rootRenderer.globalEvents.unlisten(eventNameWithTarget(target, name), callback);
       this._runOnService('listenDone', [new FnArg(unlistenCallbackId)]);
     };
   }
@@ -322,6 +322,8 @@ function eventNameWithTarget(target: string, eventName: string): string {
 
 @Injectable()
 export class WebWorkerRendererFactoryV2 implements RendererFactoryV2 {
+  globalEvents = new NamedEventEmitter();
+
   private _messageBroker: ClientMessageBroker;
 
   constructor(
@@ -359,6 +361,8 @@ export class WebWorkerRendererFactoryV2 implements RendererFactoryV2 {
     return result;
   }
 
+  freeNode(node: any) { this.renderStore.remove(node); }
+
   allocateId(): number { return this.renderStore.allocateId(); }
 
   private _dispatchEvent(message: {[key: string]: any}): void {
@@ -370,7 +374,7 @@ export class WebWorkerRendererFactoryV2 implements RendererFactoryV2 {
     const event = message['event'];
 
     if (target) {
-      globalEvents.dispatchEvent(eventNameWithTarget(target, eventName), event);
+      this.globalEvents.dispatchEvent(eventNameWithTarget(target, eventName), event);
     } else {
       element.events.dispatchEvent(eventName, event);
     }
@@ -380,13 +384,16 @@ export class WebWorkerRendererFactoryV2 implements RendererFactoryV2 {
 
 export class WebWorkerRendererV2 implements RendererV2 {
   constructor(private _rendererFactory: WebWorkerRendererFactoryV2) {}
-  destroyNode: (node: any) => void | null = null;
 
   private asFnArg = new FnArg(this, SerializerTypes.RENDER_STORE_OBJECT);
 
-  // TODO(vicb): destroy the allocated nodes
   destroy(): void { this.callUIWithRenderer('destroy'); }
 
+  destroyNode(node: any) {
+    this.callUIWithRenderer('destroyNode', [new FnArg(node, SerializerTypes.RENDER_STORE_OBJECT)]);
+    this._rendererFactory.freeNode(node);
+  }
+
   createElement(name: string, namespace?: string): any {
     const node = this._rendererFactory.allocateNode();
     this.callUIWithRenderer('createElement', [
@@ -543,7 +550,7 @@ export class WebWorkerRendererV2 implements RendererV2 {
         [target, null, null];
 
     if (fullName) {
-      globalEvents.listen(fullName, listener);
+      this._rendererFactory.globalEvents.listen(fullName, listener);
     } else {
       targetEl.events.listen(eventName, listener);
     }
@@ -557,7 +564,7 @@ export class WebWorkerRendererV2 implements RendererV2 {
 
     return () => {
       if (fullName) {
-        globalEvents.unlisten(fullName, listener);
+        this._rendererFactory.globalEvents.unlisten(fullName, listener);
       } else {
         targetEl.events.unlisten(eventName, listener);
       }

modules/@angular/platform-webworker/test/web_workers/shared/web_worker_test_util.ts

@@ -42,7 +42,7 @@ export function createPairedMessageBuses(): PairedMessageBuses {
 export function expectBrokerCall(
     broker: SpyMessageBroker, methodName: string, vals?: Array<any>,
     handler?: (..._: any[]) => Promise<any>| void): void {
-  broker.spy('callUI').and.callFake((args: UiArguments, returnType: Type<any>) => {
+  broker.spy('runOnService').and.callFake((args: UiArguments, returnType: Type<any>) => {
     expect(args.method).toEqual(methodName);
     if (isPresent(vals)) {
       expect(args.args.length).toEqual(vals.length);

tools/public_api_guard/platform-webworker/index.d.ts

@@ -47,7 +47,7 @@ export declare const platformWorkerApp: (extraProviders?: Provider[]) => Platfor
 export declare const platformWorkerUi: (extraProviders?: Provider[]) => PlatformRef;
 
 /** @experimental */
-export declare const PRIMITIVE: Type<any>;
+export declare const PRIMITIVE: SerializerTypes;
 
 /** @experimental */
 export interface ReceivedMessage {