fix(core): invoke profiler around ngOnDestroy lifecycle hooks (#41969)

mgechev · alxhub · commit 40cc29aa6e26 · 2021-05-07T10:04:22.000-04:00
Invoke the profiler for `ngOnDestroy` lifecycle hooks for services, components, directives, and pipes. PR Close #41969
diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts
@@ -26,6 +26,7 @@ import {RComment, RElement, RNode, RText} from './interfaces/renderer_dom';
 import {isLContainer, isLView} from './interfaces/type_checks';
 import {CHILD_HEAD, CLEANUP, DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, DestroyHookData, FLAGS, HookData, HookFn, HOST, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, T_HOST, TVIEW, TView, TViewType, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
 import {assertTNodeType} from './node_assert';
+import {profiler, ProfilerEvent} from './profiler';
 import {getLViewParent} from './util/view_traversal_utils';
 import {getNativeByTNode, unwrapRNode, updateTransplantedViewCount} from './util/view_utils';
 
@@ -506,10 +507,22 @@ function executeOnDestroys(tView: TView, lView: LView): void {
 
         if (Array.isArray(toCall)) {
           for (let j = 0; j < toCall.length; j += 2) {
-            (toCall[j + 1] as HookFn).call(context[toCall[j] as number]);
+            const callContext = context[toCall[j] as number];
+            const hook = toCall[j + 1] as HookFn;
+            profiler(ProfilerEvent.LifecycleHookStart, callContext, hook);
+            try {
+              hook.call(callContext);
+            } finally {
+              profiler(ProfilerEvent.LifecycleHookEnd, callContext, hook);
+            }
           }
         } else {
-          toCall.call(context);
+          profiler(ProfilerEvent.LifecycleHookStart, context, toCall);
+          try {
+            toCall.call(context);
+          } finally {
+            profiler(ProfilerEvent.LifecycleHookEnd, context, toCall);
+          }
         }
       }
     }
diff --git a/packages/core/test/acceptance/profiler_spec.ts b/packages/core/test/acceptance/profiler_spec.ts
@@ -11,7 +11,7 @@ import {TestBed} from '@angular/core/testing';
 import {expect} from '@angular/core/testing/src/testing_internal';
 import {onlyInIvy} from '@angular/private/testing';
 
-import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, Component, DoCheck, ErrorHandler, EventEmitter, Input, OnChanges, OnInit, Output, ViewChild} from '../../src/core';
+import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, Component, DoCheck, ErrorHandler, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, ViewChild} from '../../src/core';
 
 
 onlyInIvy('Ivy-specific functionality').describe('profiler', () => {
@@ -187,13 +187,19 @@ onlyInIvy('Ivy-specific functionality').describe('profiler', () => {
 
   describe('lifecycle hooks', () => {
     it('should call the profiler on lifecycle execution', () => {
-      @Component({selector: 'my-comp', template: '{{prop}}'})
+      class Service implements OnDestroy {
+        ngOnDestroy() {}
+      }
+      @Component({selector: 'my-comp', template: '{{prop}}', providers: [Service]})
       class MyComponent implements OnInit, AfterViewInit, AfterViewChecked, AfterContentInit,
-                                   AfterContentChecked, OnChanges, DoCheck {
+                                   AfterContentChecked, OnChanges, DoCheck, OnDestroy {
         @Input() prop = 1;
 
+        constructor(private service: Service) {}
+
         ngOnInit() {}
         ngDoCheck() {}
+        ngOnDestroy() {}
         ngOnChanges() {}
         ngAfterViewInit() {}
         ngAfterViewChecked() {}
@@ -293,6 +299,27 @@ onlyInIvy('Ivy-specific functionality').describe('profiler', () => {
       expect(onChangesSpy).toHaveBeenCalled();
       expect(ngOnChangesStart).toBeTruthy();
       expect(ngOnChangesEnd).toBeTruthy();
+
+      fixture.destroy();
+      const ngOnDestroyStart = findProfilerCall(
+          (args: any[]) =>
+              args[0] === ProfilerEvent.LifecycleHookStart && args[2] === myComp.ngOnDestroy);
+      const ngOnDestroyEnd = findProfilerCall(
+          (args: any[]) =>
+              args[0] === ProfilerEvent.LifecycleHookEnd && args[2] === myComp.ngOnDestroy);
+
+      expect(ngOnDestroyStart).toBeTruthy();
+      expect(ngOnDestroyEnd).toBeTruthy();
+
+      const serviceNgOnDestroyStart = findProfilerCall(
+          (args: any[]) => args[0] === ProfilerEvent.LifecycleHookStart &&
+              args[2] === Service.prototype.ngOnDestroy);
+      const serviceNgOnDestroyEnd = findProfilerCall(
+          (args: any[]) => args[0] === ProfilerEvent.LifecycleHookEnd &&
+              args[2] === Service.prototype.ngOnDestroy);
+
+      expect(serviceNgOnDestroyStart).toBeTruthy();
+      expect(serviceNgOnDestroyEnd).toBeTruthy();
     });
   });
 
