fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)

Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.

These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.

I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).

Fixes #35231.

PR Close #35840

aio/scripts/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2987,
-        "main-es2015": 451469,
+        "main-es2015": 452060,
         "polyfills-es2015": 52195
       }
     }

integration/_payload-limits.json

@@ -3,7 +3,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 141569,
+        "main-es2015": 142073,
         "polyfills-es2015": 36657
       }
     }
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 147647,
+        "main-es2015": 148196,
         "polyfills-es2015": 36657
       }
     }

packages/core/src/render3/di_setup.ts

@@ -10,6 +10,7 @@
 import {resolveForwardRef} from '../di/forward_ref';
 import {ClassProvider, Provider} from '../di/interface/provider';
 import {isClassProvider, isTypeProvider, providerToFactory} from '../di/r3_injector';
+import {assertDefined} from '../util/assert';
 
 import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from './di';
 import {ɵɵdirectiveInject} from './instructions/all';
@@ -17,7 +18,7 @@ import {DirectiveDef} from './interfaces/definition';
 import {NodeInjectorFactory} from './interfaces/injector';
 import {TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TNodeProviderIndexes} from './interfaces/node';
 import {isComponentDef} from './interfaces/type_checks';
-import {LView, TData, TVIEW, TView} from './interfaces/view';
+import {DestroyHookData, LView, TData, TVIEW, TView} from './interfaces/view';
 import {getLView, getPreviousOrParentTNode, getTView} from './state';
 
 
@@ -149,7 +150,7 @@ function resolveProvider(
         if (!isViewProvider && doesViewProvidersFactoryExist) {
           lInjectablesBlueprint[existingViewProvidersFactoryIndex].providerFactory = factory;
         }
-        registerDestroyHooksIfSupported(tView, provider, tInjectables.length);
+        registerDestroyHooksIfSupported(tView, provider, tInjectables.length, 0);
         tInjectables.push(token);
         tNode.directiveStart++;
         tNode.directiveEnd++;
@@ -160,13 +161,16 @@ function resolveProvider(
         lView.push(factory);
       } else {
         // Cases 1.b and 2.b
-        registerDestroyHooksIfSupported(
+        const indexInFactory = multiFactoryAdd(
-            tView, provider, existingProvidersFactoryIndex > -1 ?
+            lInjectablesBlueprint!
-                existingProvidersFactoryIndex :
+                [isViewProvider ? existingViewProvidersFactoryIndex :
-                existingViewProvidersFactoryIndex);
+                                  existingProvidersFactoryIndex],
-        multiFactoryAdd(
-            lInjectablesBlueprint ![isViewProvider ? existingViewProvidersFactoryIndex : existingProvidersFactoryIndex],
             providerFactory, !isViewProvider && isComponent);
+        registerDestroyHooksIfSupported(
+            tView, provider,
+            existingProvidersFactoryIndex > -1 ? existingProvidersFactoryIndex :
+                                                 existingViewProvidersFactoryIndex,
+            indexInFactory);
       }
       if (!isViewProvider && isComponent && doesViewProvidersFactoryExist) {
         lInjectablesBlueprint[existingViewProvidersFactoryIndex].componentProviders!++;
@@ -180,28 +184,47 @@ function resolveProvider(
  * @param tView `TView` in which to register the hook.
  * @param provider Provider whose hook should be registered.
  * @param contextIndex Index under which to find the context for the hook when it's being invoked.
+ * @param indexInFactory Only required for `multi` providers. Index of the provider in the multi
+ * provider factory.
  */
 function registerDestroyHooksIfSupported(
-    tView: TView, provider: Exclude<Provider, any[]>, contextIndex: number) {
+    tView: TView, provider: Exclude<Provider, any[]>, contextIndex: number,
+    indexInFactory?: number) {
   const providerIsTypeProvider = isTypeProvider(provider);
   if (providerIsTypeProvider || isClassProvider(provider)) {
     const prototype = ((provider as ClassProvider).useClass || provider).prototype;
     const ngOnDestroy = prototype.ngOnDestroy;
     if (ngOnDestroy) {
-      (tView.destroyHooks || (tView.destroyHooks = [])).push(contextIndex, ngOnDestroy);
+      const hooks = tView.destroyHooks || (tView.destroyHooks = []);
+
+      if (!providerIsTypeProvider && ((provider as ClassProvider)).multi) {
+        ngDevMode &&
+            assertDefined(
+                indexInFactory, 'indexInFactory when registering multi factory destroy hook');
+        const existingCallbacksIndex = hooks.indexOf(contextIndex);
+
+        if (existingCallbacksIndex === -1) {
+          hooks.push(contextIndex, [indexInFactory, ngOnDestroy]);
+        } else {
+          (hooks[existingCallbacksIndex + 1] as DestroyHookData).push(indexInFactory!, ngOnDestroy);
+        }
+      } else {
+        hooks.push(contextIndex, ngOnDestroy);
+      }
     }
   }
 }
 
 /**
  * Add a factory in a multi factory.
+ * @returns Index at which the factory was inserted.
  */
 function multiFactoryAdd(
-    multiFactory: NodeInjectorFactory, factory: () => any, isComponentProvider: boolean): void {
+    multiFactory: NodeInjectorFactory, factory: () => any, isComponentProvider: boolean): number {
-  multiFactory.multi !.push(factory);
   if (isComponentProvider) {
     multiFactory.componentProviders!++;
   }
+  return multiFactory.multi!.push(factory) - 1;
 }
 
 /**

packages/core/src/render3/instructions/lview_debug.ts

@@ -19,9 +19,9 @@ import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18
 import {PropertyAliases, TConstants, TContainerNode, TElementNode, TNode as ITNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TViewNode} from '../interfaces/node';
 import {SelectorFlags} from '../interfaces/projection';
 import {LQueries, TQueries} from '../interfaces/query';
-import {RComment, RElement, RNode, Renderer3, RendererFactory3} from '../interfaces/renderer';
+import {RComment, RElement, Renderer3, RendererFactory3, RNode} from '../interfaces/renderer';
-import {TStylingKey, TStylingRange, getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate} from '../interfaces/styling';
+import {getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate, TStylingKey, TStylingRange} from '../interfaces/styling';
-import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, HookData, INJECTOR, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, SANITIZER, TData, TVIEW, TView as ITView, TView, TViewType, T_HOST} from '../interfaces/view';
+import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, DestroyHookData, ExpandoInstructions, FLAGS, HEADER_OFFSET, HookData, HOST, INJECTOR, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, SANITIZER, T_HOST, TData, TVIEW, TView as ITView, TView, TViewType} from '../interfaces/view';
 import {attachDebugObject} from '../util/debug_utils';
 import {getLContainerActiveIndex, getTNode, unwrapRNode} from '../util/view_utils';
 
@@ -134,7 +134,7 @@ export const TViewConstructor = class TView implements ITView {
       public contentCheckHooks: HookData|null,               //
       public viewHooks: HookData|null,                       //
       public viewCheckHooks: HookData|null,                  //
-      public destroyHooks: HookData|null,                    //
+      public destroyHooks: DestroyHookData|null,             //
       public cleanup: any[]|null,                            //
       public contentQueries: number[]|null,                  //
       public components: number[]|null,                      //
@@ -236,8 +236,12 @@ class TNode implements ITNode {
     return buf.join('');
   }
 
-  get styleBindings_(): DebugStyleBindings { return toDebugStyleBinding(this, false); }
+  get styleBindings_(): DebugStyleBindings {
-  get classBindings_(): DebugStyleBindings { return toDebugStyleBinding(this, true); }
+    return toDebugStyleBinding(this, false);
+  }
+  get classBindings_(): DebugStyleBindings {
+    return toDebugStyleBinding(this, true);
+  }
 }
 export const TNodeDebug = TNode;
 export type TNodeDebug = TNode;
@@ -289,9 +293,8 @@ function processTNodeChildren(tNode: ITNode | null, buf: string[]) {
 }
 
 const TViewData = NG_DEV_MODE && createNamedArrayType('TViewData') || null! as ArrayConstructor;
-let TVIEWDATA_EMPTY:
+let TVIEWDATA_EMPTY: unknown[];  // can't initialize here or it will not be tree shaken, because
-    unknown[];  // can't initialize here or it will not be tree shaken, because `LView`
+                                 // `LView` constructor could have side-effects.
-                // constructor could have side-effects.
 /**
  * This function clones a blueprint and creates TData.
  *
@@ -390,10 +393,18 @@ export class LViewDebug {
       indexWithinInitPhase: flags >> LViewFlags.IndexWithinInitPhaseShift,
     };
   }
-  get parent(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lView[PARENT]); }
+  get parent(): LViewDebug|LContainerDebug|null {
-  get host(): string|null { return toHtml(this._raw_lView[HOST], true); }
+    return toDebug(this._raw_lView[PARENT]);
-  get html(): string { return (this.nodes || []).map(node => toHtml(node.native, true)).join(''); }
+  }
-  get context(): {}|null { return this._raw_lView[CONTEXT]; }
+  get host(): string|null {
+    return toHtml(this._raw_lView[HOST], true);
+  }
+  get html(): string {
+    return (this.nodes || []).map(node => toHtml(node.native, true)).join('');
+  }
+  get context(): {}|null {
+    return this._raw_lView[CONTEXT];
+  }
   /**
    * The tree of nodes associated with the current `LView`. The nodes have been normalized into
    * a
@@ -405,18 +416,42 @@ export class LViewDebug {
     return toDebugNodes(tNode, lView);
   }
 
-  get tView(): ITView { return this._raw_lView[TVIEW]; }
+  get tView(): ITView {
-  get cleanup(): any[]|null { return this._raw_lView[CLEANUP]; }
+    return this._raw_lView[TVIEW];
-  get injector(): Injector|null { return this._raw_lView[INJECTOR]; }
+  }
-  get rendererFactory(): RendererFactory3 { return this._raw_lView[RENDERER_FACTORY]; }
+  get cleanup(): any[]|null {
-  get renderer(): Renderer3 { return this._raw_lView[RENDERER]; }
+    return this._raw_lView[CLEANUP];
-  get sanitizer(): Sanitizer|null { return this._raw_lView[SANITIZER]; }
+  }
-  get childHead(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lView[CHILD_HEAD]); }
+  get injector(): Injector|null {
-  get next(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lView[NEXT]); }
+    return this._raw_lView[INJECTOR];
-  get childTail(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lView[CHILD_TAIL]); }
+  }
-  get declarationView(): LViewDebug|null { return toDebug(this._raw_lView[DECLARATION_VIEW]); }
+  get rendererFactory(): RendererFactory3 {
-  get queries(): LQueries|null { return this._raw_lView[QUERIES]; }
+    return this._raw_lView[RENDERER_FACTORY];
-  get tHost(): TViewNode|TElementNode|null { return this._raw_lView[T_HOST]; }
+  }
+  get renderer(): Renderer3 {
+    return this._raw_lView[RENDERER];
+  }
+  get sanitizer(): Sanitizer|null {
+    return this._raw_lView[SANITIZER];
+  }
+  get childHead(): LViewDebug|LContainerDebug|null {
+    return toDebug(this._raw_lView[CHILD_HEAD]);
+  }
+  get next(): LViewDebug|LContainerDebug|null {
+    return toDebug(this._raw_lView[NEXT]);
+  }
+  get childTail(): LViewDebug|LContainerDebug|null {
+    return toDebug(this._raw_lView[CHILD_TAIL]);
+  }
+  get declarationView(): LViewDebug|null {
+    return toDebug(this._raw_lView[DECLARATION_VIEW]);
+  }
+  get queries(): LQueries|null {
+    return this._raw_lView[QUERIES];
+  }
+  get tHost(): TViewNode|TElementNode|null {
+    return this._raw_lView[T_HOST];
+  }
 
   /**
    * Normalized view of child views (and containers) attached at this location.
@@ -474,7 +509,9 @@ export function buildDebugNode(tNode: ITNode, lView: LView, nodeIndex: number):
 export class LContainerDebug {
   constructor(private readonly _raw_lContainer: LContainer) {}
 
-  get activeIndex(): number { return getLContainerActiveIndex(this._raw_lContainer); }
+  get activeIndex(): number {
+    return getLContainerActiveIndex(this._raw_lContainer);
+  }
   get hasTransplantedViews(): boolean {
     return (this._raw_lContainer[ACTIVE_INDEX] & ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS) ===
         ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS;
@@ -483,11 +520,21 @@ export class LContainerDebug {
     return this._raw_lContainer.slice(CONTAINER_HEADER_OFFSET)
         .map(toDebug as (l: LView) => LViewDebug);
   }
-  get parent(): LViewDebug|LContainerDebug|null { return toDebug(this._raw_lContainer[PARENT]); }
+  get parent(): LViewDebug|LContainerDebug|null {
-  get movedViews(): LView[]|null { return this._raw_lContainer[MOVED_VIEWS]; }
+    return toDebug(this._raw_lContainer[PARENT]);
-  get host(): RElement|RComment|LView { return this._raw_lContainer[HOST]; }
+  }
-  get native(): RComment { return this._raw_lContainer[NATIVE]; }
+  get movedViews(): LView[]|null {
-  get next() { return toDebug(this._raw_lContainer[NEXT]); }
+    return this._raw_lContainer[MOVED_VIEWS];
+  }
+  get host(): RElement|RComment|LView {
+    return this._raw_lContainer[HOST];
+  }
+  get native(): RComment {
+    return this._raw_lContainer[NATIVE];
+  }
+  get next() {
+    return toDebug(this._raw_lContainer[NEXT]);
+  }
 }
 
 /**
@@ -508,7 +555,9 @@ export function readLViewValue(value: any): LView|null {
 export class I18NDebugItem {
   [key: string]: any;
 
-  get tNode() { return getTNode(this._lView[TVIEW], this.nodeIndex); }
+  get tNode() {
+    return getTNode(this._lView[TVIEW], this.nodeIndex);
+  }
 
   constructor(
       public __raw_opCode: any, private _lView: LView, public nodeIndex: number,
@@ -531,8 +580,9 @@ export function attachI18nOpCodesDebug(
 
   if (icus) {
     icus.forEach(icu => {
-      icu.create.forEach(
+      icu.create.forEach(icuCase => {
-          icuCase => { attachDebugObject(icuCase, new I18nMutateOpCodesDebug(icuCase, lView)); });
+        attachDebugObject(icuCase, new I18nMutateOpCodesDebug(icuCase, lView));
+      });
       icu.update.forEach(icuCase => {
         attachDebugObject(icuCase, new I18nUpdateOpCodesDebug(icuCase, icus, lView));
       });
@@ -658,14 +708,17 @@ export class I18nUpdateOpCodesDebug implements I18nOpCodesDebug {
                   __raw_opCode: opCode,
                   checkBit,
                   type: 'Attr',
-                  attrValue: value, attrName, sanitizeFn,
+                  attrValue: value,
+                  attrName,
+                  sanitizeFn,
                 });
                 break;
               case I18nUpdateOpCode.Text:
                 results.push({
                   __raw_opCode: opCode,
                   checkBit,
-                  type: 'Text', nodeIndex,
+                  type: 'Text',
+                  nodeIndex,
                   text: value,
                 });
                 break;
@@ -698,4 +751,6 @@ export class I18nUpdateOpCodesDebug implements I18nOpCodesDebug {
   }
 }
 
-export interface I18nOpCodesDebug { operations: any[]; }
+export interface I18nOpCodesDebug {
+  operations: any[];
+}

packages/core/src/render3/instructions/shared.ts

@@ -24,11 +24,11 @@ import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} fr
 import {ACTIVE_INDEX, ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS} from '../interfaces/container';
 import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
 import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from '../interfaces/injector';
-import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TConstants, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
+import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliases, PropertyAliasValue, TAttributes, TConstants, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
-import {RComment, RElement, RNode, RText, Renderer3, RendererFactory3, isProceduralRenderer} from '../interfaces/renderer';
+import {isProceduralRenderer, RComment, RElement, Renderer3, RendererFactory3, RNode, RText} from '../interfaces/renderer';
 import {SanitizerFn} from '../interfaces/sanitization';
 import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks';
-import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TData, TVIEW, TView, TViewType, T_HOST} from '../interfaces/view';
+import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TVIEW, TView, TViewType} from '../interfaces/view';
 import {assertNodeOfPossibleTypes} from '../node_assert';
 import {isNodeMatchingSelectorList} from '../node_selector_matcher';
 import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, getTView, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
@@ -39,7 +39,7 @@ import {getLViewParent} from '../util/view_traversal_utils';
 import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapLView, viewAttachedToChangeDetector} from '../util/view_utils';
 
 import {selectIndexInternal} from './advance';
-import {LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeDebug, TNodeInitialInputs, TNodeLocalNames, TViewComponents, TViewConstructor, attachLContainerDebug, attachLViewDebug, cloneToLViewFromTViewBlueprint, cloneToTViewData} from './lview_debug';
+import {attachLContainerDebug, attachLViewDebug, cloneToLViewFromTViewBlueprint, cloneToTViewData, LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeDebug, TNodeInitialInputs, TNodeLocalNames, TViewComponents, TViewConstructor} from './lview_debug';
 
 
 
@@ -155,8 +155,7 @@ function renderChildComponents(hostLView: LView, components: number[]): void {
  * @param renderer A renderer to use
  * @returns the element created
  */
-export function elementCreate(
+export function elementCreate(name: string, renderer: Renderer3, namespace: string|null): RElement {
-    name: string, renderer: Renderer3, namespace: string | null): RElement {
   if (isProceduralRenderer(renderer)) {
     return renderer.createElement(name, namespace);
   } else {
@@ -166,10 +165,9 @@ export function elementCreate(
 }
 
 export function createLView<T>(
-    parentLView: LView | null, tView: TView, context: T | null, flags: LViewFlags,
+    parentLView: LView|null, tView: TView, context: T|null, flags: LViewFlags, host: RElement|null,
-    host: RElement | null, tHostNode: TViewNode | TElementNode | null,
+    tHostNode: TViewNode|TElementNode|null, rendererFactory?: RendererFactory3|null,
-    rendererFactory?: RendererFactory3 | null, renderer?: Renderer3 | null,
+    renderer?: Renderer3|null, sanitizer?: Sanitizer|null, injector?: Injector|null): LView {
-    sanitizer?: Sanitizer | null, injector?: Injector | null): LView {
   const lView =
       ngDevMode ? cloneToLViewFromTViewBlueprint(tView) : tView.blueprint.slice() as LView;
   lView[HOST] = host;
@@ -184,7 +182,8 @@ export function createLView<T>(
   lView[SANITIZER] = sanitizer || parentLView && parentLView[SANITIZER] || null!;
   lView[INJECTOR as any] = injector || parentLView && parentLView[INJECTOR] || null;
   lView[T_HOST] = tHostNode;
-  ngDevMode && assertEqual(
+  ngDevMode &&
+      assertEqual(
           tView.type == TViewType.Embedded ? parentLView !== null : true, true,
           'Embedded views must have parentLView');
   lView[DECLARATION_COMPONENT_VIEW] =
@@ -208,8 +207,8 @@ export function createLView<T>(
  * @param attrs Any attrs for the native element, if applicable
  */
 export function getOrCreateTNode(
-    tView: TView, tHostNode: TNode | null, index: number, type: TNodeType.Element,
+    tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Element, name: string|null,
-    name: string | null, attrs: TAttributes | null): TElementNode;
+    attrs: TAttributes|null): TElementNode;
 export function getOrCreateTNode(
     tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Container,
     name: string|null, attrs: TAttributes|null): TContainerNode;
@@ -236,8 +235,8 @@ export function getOrCreateTNode(
 }
 
 function createTNodeAtIndex(
-    tView: TView, tHostNode: TNode | null, adjustedIndex: number, type: TNodeType,
+    tView: TView, tHostNode: TNode|null, adjustedIndex: number, type: TNodeType, name: string|null,
-    name: string | null, attrs: TAttributes | null) {
+    attrs: TAttributes|null) {
   const previousOrParentTNode = getPreviousOrParentTNode();
   const isParent = getIsParent();
   const parent =
@@ -294,7 +293,8 @@ export function assignTViewNodeToLView(
  * @param numSlotsToAlloc The number of slots to alloc in the LView, should be >0
  */
 export function allocExpando(tView: TView, lView: LView, numSlotsToAlloc: number) {
-  ngDevMode && assertGreaterThan(
+  ngDevMode &&
+      assertGreaterThan(
           numSlotsToAlloc, 0, 'The number of slots to alloc should be greater than 0');
   if (numSlotsToAlloc > 0) {
     if (tView.firstCreatePass) {
@@ -652,7 +652,7 @@ export function createTView(
              null,               // contentCheckHooks: HookData|null,
              null,               // viewHooks: HookData|null,
              null,               // viewCheckHooks: HookData|null,
-             null,               // destroyHooks: HookData|null,
+             null,               // destroyHooks: DestroyHookData|null,
              null,               // cleanup: any[]|null,
              null,               // contentQueries: number[]|null,
              null,               // components: number[]|null,
@@ -796,8 +796,8 @@ export function storeCleanupFn(tView: TView, lView: LView, cleanupFn: Function):
  * @returns the TNode object
  */
 export function createTNode(
-    tView: TView, tParent: TElementNode | TContainerNode | null, type: TNodeType,
+    tView: TView, tParent: TElementNode|TContainerNode|null, type: TNodeType, adjustedIndex: number,
-    adjustedIndex: number, tagName: string | null, attrs: TAttributes | null): TNode {
+    tagName: string|null, attrs: TAttributes|null): TNode {
   ngDevMode && ngDevMode.tNode++;
   let injectorIndex = tParent ? tParent.injectorIndex : -1;
   return ngDevMode ? new TNodeDebug(
@@ -1152,8 +1152,8 @@ export function resolveDirectives(
         }
 
         if (!preOrderCheckHooksFound && (def.onChanges || def.doCheck)) {
-          (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = [
+          (tView.preOrderCheckHooks || (tView.preOrderCheckHooks = []))
-           ])).push(tNode.index - HEADER_OFFSET);
+              .push(tNode.index - HEADER_OFFSET);
           preOrderCheckHooksFound = true;
         }
 
@@ -1296,7 +1296,8 @@ export function invokeHostBindingsInCreationMode(def: DirectiveDef<any>, directi
  */
 export function generateExpandoInstructionBlock(
     tView: TView, tNode: TNode, directiveCount: number): void {
-  ngDevMode && assertEqual(
+  ngDevMode &&
+      assertEqual(
           tView.firstCreatePass, true,
           'Expando block should only be generated on first create pass.');
 
@@ -1306,8 +1307,8 @@ export function generateExpandoInstructionBlock(
   const elementIndex = HEADER_OFFSET - tNode.index;
   const providerStartIndex = tNode.providerIndexes & TNodeProviderIndexes.ProvidersStartIndexMask;
   const providerCount = tView.data.length - providerStartIndex;
-  (tView.expandoInstructions || (tView.expandoInstructions = [
+  (tView.expandoInstructions || (tView.expandoInstructions = []))
-   ])).push(elementIndex, providerCount, directiveCount);
+      .push(elementIndex, providerCount, directiveCount);
 }
 
 /**
@@ -1318,7 +1319,8 @@ function findDirectiveDefMatches(
     tView: TView, viewData: LView,
     tNode: TElementNode|TContainerNode|TElementContainerNode): DirectiveDef<any>[]|null {
   ngDevMode && assertFirstCreatePass(tView);
-  ngDevMode && assertNodeOfPossibleTypes(
+  ngDevMode &&
+      assertNodeOfPossibleTypes(
           tNode, TNodeType.Element, TNodeType.ElementContainer, TNodeType.Container);
   const registry = tView.directiveRegistry;
   let matches: any[]|null = null;
@@ -1351,8 +1353,8 @@ function findDirectiveDefMatches(
 export function markAsComponentHost(tView: TView, hostTNode: TNode): void {
   ngDevMode && assertFirstCreatePass(tView);
   hostTNode.flags |= TNodeFlags.isComponentHost;
-  (tView.components || (tView.components = ngDevMode ? new TViewComponents() : [
+  (tView.components || (tView.components = ngDevMode ? new TViewComponents() : []))
-   ])).push(hostTNode.index);
+      .push(hostTNode.index);
 }
 
 
@@ -1360,8 +1362,7 @@ export function markAsComponentHost(tView: TView, hostTNode: TNode): void {
 function cacheMatchingLocalNames(
     tNode: TNode, localRefs: string[]|null, exportsMap: {[key: string]: number}): void {
   if (localRefs) {
-    const localNames: (string | number)[] = tNode.localNames =
+    const localNames: (string|number)[] = tNode.localNames = ngDevMode ? new TNodeLocalNames() : [];
-        ngDevMode ? new TNodeLocalNames() : [];
 
     // Local names must be stored in tNode in the same order that localRefs are defined
     // in the template to ensure the data is loaded in the same slots as their refs
@@ -1397,7 +1398,8 @@ function saveNameToExportMap(
  * @param index the initial index
  */
 export function initTNodeFlags(tNode: TNode, index: number, numberOfDirectives: number) {
-  ngDevMode && assertNotEqual(
+  ngDevMode &&
+      assertNotEqual(
           numberOfDirectives, tNode.directiveEnd - tNode.directiveStart,
           'Reached the max number of directives');
   tNode.flags |= TNodeFlags.isDirectiveHost;

packages/core/src/render3/interfaces/view.ts

@@ -365,8 +365,10 @@ export const enum InitPhaseState {
 
 /** More flags associated with an LView (saved in LView[PREORDER_HOOK_FLAGS]) */
 export const enum PreOrderHookFlags {
-  /** The index of the next pre-order hook to be called in the hooks array, on the first 16
+  /**
-     bits */
+     The index of the next pre-order hook to be called in the hooks array, on the first 16
+     bits
+   */
   IndexOfTheNextPreOrderHookMaskMask = 0b01111111111111111,
 
   /**
@@ -617,7 +619,7 @@ export interface TView {
    * Even indices: Directive index
    * Odd indices: Hook function
    */
-  destroyHooks: HookData|null;
+  destroyHooks: DestroyHookData|null;
 
   /**
    * When a view is destroyed, listeners need to be released and outputs need to be
@@ -684,7 +686,11 @@ export interface TView {
   consts: TConstants|null;
 }
 
-export const enum RootContextFlags {Empty = 0b00, DetectChanges = 0b01, FlushPlayers = 0b10}
+export const enum RootContextFlags {
+  Empty = 0b00,
+  DetectChanges = 0b01,
+  FlushPlayers = 0b10
+}
 
 
 /**
@@ -722,6 +728,15 @@ export interface RootContext {
   flags: RootContextFlags;
 }
 
+/** Single hook callback function. */
+export type HookFn = () => void;
+
+/**
+ * Information necessary to call a hook. E.g. the callback that
+ * needs to invoked and the index at which to find its context.
+ */
+export type HookEntry = number|HookFn;
+
 /**
  * Array of hooks that should be executed for a view and their directive indices.
  *
@@ -734,7 +749,27 @@ export interface RootContext {
  * Special cases:
  *  - a negative directive index flags an init hook (ngOnInit, ngAfterContentInit, ngAfterViewInit)
  */
-export type HookData = (number | (() => void))[];
+export type HookData = HookEntry[];
+
+/**
+ * Array of destroy hooks that should be executed for a view and their directive indices.
+ *
+ * The array is set up as a series of number/function or number/(number|function)[]:
+ * - Even indices represent the context with which hooks should be called.
+ * - Odd indices are the hook functions themselves. If a value at an odd index is an array,
+ *   it represents the destroy hooks of a `multi` provider where:
+ *     - Even indices represent the index of the provider for which we've registered a destroy hook,
+ *       inside of the `multi` provider array.
+ *     - Odd indices are the destroy hook functions.
+ * For example:
+ * LView: `[0, 1, 2, AService, 4, [BService, CService, DService]]`
+ * destroyHooks: `[3, AService.ngOnDestroy, 5, [0, BService.ngOnDestroy, 2, DService.ngOnDestroy]]`
+ *
+ * In the example above `AService` is a type provider with an `ngOnDestroy`, whereas `BService`,
+ * `CService` and `DService` are part of a `multi` provider where only `BService` and `DService`
+ * have an `ngOnDestroy` hook.
+ */
+export type DestroyHookData = (HookEntry|HookData)[];
 
 /**
  * Static data that corresponds to the instance-specific data array on an LView.
@@ -764,8 +799,8 @@ export type HookData = (number | (() => void))[];
  * Injector bloom filters are also stored here.
  */
 export type TData =
-    (TNode | PipeDef<any>| DirectiveDef<any>| ComponentDef<any>| number | TStylingRange |
+    (TNode|PipeDef<any>|DirectiveDef<any>|ComponentDef<any>|number|TStylingRange|TStylingKey|
-     TStylingKey | Type<any>| InjectionToken<any>| TI18n | I18nUpdateOpCodes | null | string)[];
+     Type<any>|InjectionToken<any>|TI18n|I18nUpdateOpCodes|null|string)[];
 
 // Note: This hack is necessary so we don't erroneously get a circular dependency
 // failure based on types.

packages/core/src/render3/node_manipulation.ts

@@ -10,6 +10,7 @@ import {Renderer2} from '../core';
 import {ViewEncapsulation} from '../metadata/view';
 import {addToArray, removeFromArray} from '../util/array_utils';
 import {assertDefined, assertDomNode, assertEqual, assertString} from '../util/assert';
+
 import {assertLContainer, assertLView, assertTNodeForLView} from './assert';
 import {attachPatchData} from './context_discovery';
 import {ACTIVE_INDEX, ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
@@ -17,9 +18,9 @@ import {ComponentDef} from './interfaces/definition';
 import {NodeInjectorFactory} from './interfaces/injector';
 import {TElementNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
 import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection';
-import {ProceduralRenderer3, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer';
+import {isProceduralRenderer, ProceduralRenderer3, RElement, Renderer3, RNode, RText, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer';
 import {isLContainer, isLView} from './interfaces/type_checks';
-import {CHILD_HEAD, CLEANUP, DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, FLAGS, HOST, HookData, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, TView, T_HOST, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
+import {CHILD_HEAD, CLEANUP, DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, DestroyHookData, FLAGS, HookData, HookFn, HOST, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, T_HOST, TVIEW, TView, unusedValueExportToPlacateAjd as unused5} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
 import {getLViewParent} from './util/view_traversal_utils';
 import {getNativeByTNode, unwrapRNode} from './util/view_utils';
@@ -291,7 +292,8 @@ function trackMovedView(declarationContainer: LContainer, lView: LView) {
 
 function detachMovedView(declarationContainer: LContainer, lView: LView) {
   ngDevMode && assertLContainer(declarationContainer);
-  ngDevMode && assertDefined(
+  ngDevMode &&
+      assertDefined(
           declarationContainer[MOVED_VIEWS],
           'A projected view should belong to a non-empty projected views collection');
   const movedViews = declarationContainer[MOVED_VIEWS]!;
@@ -484,7 +486,7 @@ function removeListeners(tView: TView, lView: LView): void {
 
 /** Calls onDestroy hooks for this view */
 function executeOnDestroys(tView: TView, lView: LView): void {
-  let destroyHooks: HookData|null;
+  let destroyHooks: DestroyHookData|null;
 
   if (tView != null && (destroyHooks = tView.destroyHooks) != null) {
     for (let i = 0; i < destroyHooks.length; i += 2) {
@@ -492,7 +494,15 @@ function executeOnDestroys(tView: TView, lView: LView): void {
 
       // Only call the destroy hook if the context has been requested.
       if (!(context instanceof NodeInjectorFactory)) {
-        (destroyHooks[i + 1] as() => void).call(context);
+        const toCall = destroyHooks[i + 1] as HookFn | HookData;
+
+        if (Array.isArray(toCall)) {
+          for (let j = 0; j < toCall.length; j += 2) {
+            (toCall[j + 1] as HookFn).call(context[toCall[j] as number]);
+          }
+        } else {
+          toCall.call(context);
+        }
       }
     }
   }
@@ -514,7 +524,8 @@ function getRenderParent(tView: TView, tNode: TNode, currentView: LView): REleme
   // Skip over element and ICU containers as those are represented by a comment node and
   // can't be used as a render parent.
   let parentTNode = tNode.parent;
-  while (parentTNode != null && (parentTNode.type === TNodeType.ElementContainer ||
+  while (parentTNode != null &&
+         (parentTNode.type === TNodeType.ElementContainer ||
           parentTNode.type === TNodeType.IcuContainer)) {
     tNode = parentTNode;
     parentTNode = tNode.parent;
@@ -682,7 +693,8 @@ export function appendChild(
  */
 function getFirstNativeNode(lView: LView, tNode: TNode|null): RNode|null {
   if (tNode !== null) {
-    ngDevMode && assertNodeOfPossibleTypes(
+    ngDevMode &&
+        assertNodeOfPossibleTypes(
             tNode, TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer,
             TNodeType.IcuContainer, TNodeType.Projection);
 
@@ -761,7 +773,8 @@ function applyNodes(
     renderParent: RElement|null, beforeNode: RNode|null, isProjection: boolean) {
   while (tNode != null) {
     ngDevMode && assertTNodeForLView(tNode, lView);
-    ngDevMode && assertNodeOfPossibleTypes(
+    ngDevMode &&
+        assertNodeOfPossibleTypes(
             tNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer,
             TNodeType.Projection, TNodeType.Projection, TNodeType.IcuContainer);
     const rawSlotValue = lView[tNode.index];

packages/core/test/acceptance/providers_spec.ts

@@ -6,16 +6,14 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, Inject, Injectable, InjectionToken, Injector, NgModule, Optional, forwardRef} from '@angular/core';
+import {Component, Directive, forwardRef, Inject, Injectable, InjectionToken, Injector, NgModule, Optional} from '@angular/core';
-import {TestBed, async, inject} from '@angular/core/testing';
+import {async, inject, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
-import {onlyInIvy} from '@angular/private/testing';
+import {modifiedInIvy, onlyInIvy} from '@angular/private/testing';
 
 describe('providers', () => {
-
   describe('inheritance', () => {
-
     it('should NOT inherit providers', () => {
       const SOME_DIRS = new InjectionToken('someDirs');
 
@@ -52,7 +50,6 @@ describe('providers', () => {
       expect(otherDir.dirs.length).toEqual(1);
       expect(otherDir.dirs[0] instanceof SubDirective).toBe(true);
     });
-
   });
 
   describe('lifecycles', () => {
@@ -61,7 +58,9 @@ describe('providers', () => {
 
       @Injectable()
       class SuperInjectableWithDestroyHook {
-        ngOnDestroy() { logs.push('OnDestroy'); }
+        ngOnDestroy() {
+          logs.push('OnDestroy');
+        }
       }
 
       @Injectable()
@@ -86,7 +85,9 @@ describe('providers', () => {
 
       @Injectable()
       class InjectableWithDestroyHook {
-        ngOnDestroy() { logs.push('OnDestroy'); }
+        ngOnDestroy() {
+          logs.push('OnDestroy');
+        }
       }
 
       @Component({template: '', providers: [InjectableWithDestroyHook]})
@@ -106,7 +107,9 @@ describe('providers', () => {
 
       @Injectable()
       class InjectableWithDestroyHook {
-        ngOnDestroy() { logs.push('OnDestroy'); }
+        ngOnDestroy() {
+          logs.push('OnDestroy');
+        }
       }
 
       @Component({selector: 'my-cmp', template: ''})
@@ -137,7 +140,9 @@ describe('providers', () => {
 
       @Injectable()
       class InjectableWithDestroyHook {
-        ngOnDestroy() { logs.push('OnDestroy'); }
+        ngOnDestroy() {
+          logs.push('OnDestroy');
+        }
       }
 
       @Component({
@@ -162,12 +167,16 @@ describe('providers', () => {
 
           @Injectable()
           class InjectableWithDestroyHookToken {
-            ngOnDestroy() { logs.push('OnDestroy Token'); }
+            ngOnDestroy() {
+              logs.push('OnDestroy Token');
+            }
           }
 
           @Injectable()
           class InjectableWithDestroyHookValue {
-            ngOnDestroy() { logs.push('OnDestroy Value'); }
+            ngOnDestroy() {
+              logs.push('OnDestroy Value');
+            }
           }
 
           @Component({
@@ -193,21 +202,23 @@ describe('providers', () => {
 
       @Injectable()
       class InjectableWithDestroyHookToken {
-        ngOnDestroy() { logs.push('OnDestroy Token'); }
+        ngOnDestroy() {
+          logs.push('OnDestroy Token');
+        }
       }
 
       @Injectable()
       class InjectableWithDestroyHookExisting {
-        ngOnDestroy() { logs.push('OnDestroy Existing'); }
+        ngOnDestroy() {
+          logs.push('OnDestroy Existing');
+        }
       }
 
       @Component({
         template: '',
         providers: [
-          InjectableWithDestroyHookExisting, {
+          InjectableWithDestroyHookExisting,
-            provide: InjectableWithDestroyHookToken,
+          {provide: InjectableWithDestroyHookToken, useExisting: InjectableWithDestroyHookExisting}
-            useExisting: InjectableWithDestroyHookExisting
-          }
         ]
       })
       class App {
@@ -232,23 +243,33 @@ describe('providers', () => {
 
          @Injectable()
          class DestroyService {
-           constructor() { resolvedServices.push(this); }
+           constructor() {
-           ngOnDestroy() { destroyContexts.push(this); }
+             resolvedServices.push(this);
+           }
+           ngOnDestroy() {
+             destroyContexts.push(this);
+           }
          }
 
          @Directive({selector: '[dir-one]', providers: [DestroyService]})
          class DirOne {
-           constructor(service: DestroyService) { childService = service; }
+           constructor(service: DestroyService) {
+             childService = service;
+           }
          }
 
          @Directive({selector: '[dir-two]', providers: [DestroyService]})
          class DirTwo {
-           constructor(service: DestroyService) { childService = service; }
+           constructor(service: DestroyService) {
+             childService = service;
+           }
          }
 
          @Component({template: '<div dir-one dir-two></div>', providers: [DestroyService]})
          class App {
-           constructor(service: DestroyService) { parentService = service; }
+           constructor(service: DestroyService) {
+             parentService = service;
+           }
          }
 
          TestBed.configureTestingModule({declarations: [App, DirOne, DirTwo]});
@@ -274,20 +295,28 @@ describe('providers', () => {
 
               @Injectable()
               class DestroyService {
-                constructor() { resolvedServices.push(this); }
+                constructor() {
-                ngOnDestroy() { destroyContexts.push(this); }
+                  resolvedServices.push(this);
+                }
+                ngOnDestroy() {
+                  destroyContexts.push(this);
+                }
               }
 
               @Directive(
                   {selector: '[dir-one]', providers: [{provide: token, useClass: DestroyService}]})
               class DirOne {
-                constructor(@Inject(token) service: DestroyService) { childService = service; }
+                constructor(@Inject(token) service: DestroyService) {
+                  childService = service;
+                }
               }
 
               @Directive(
                   {selector: '[dir-two]', providers: [{provide: token, useClass: DestroyService}]})
               class DirTwo {
-                constructor(@Inject(token) service: DestroyService) { childService = service; }
+                constructor(@Inject(token) service: DestroyService) {
+                  childService = service;
+                }
               }
 
               @Component({
@@ -295,7 +324,9 @@ describe('providers', () => {
                 providers: [{provide: token, useClass: DestroyService}]
               })
               class App {
-                constructor(@Inject(token) service: DestroyService) { parentService = service; }
+                constructor(@Inject(token) service: DestroyService) {
+                  parentService = service;
+                }
               }
 
               TestBed.configureTestingModule({declarations: [App, DirOne, DirTwo]});
@@ -310,21 +341,25 @@ describe('providers', () => {
               expect(destroyContexts).toEqual([parentService, childService]);
             });
 
+
     onlyInIvy('ngOnDestroy hooks for multi providers were not supported in ViewEngine')
-        .it('should not invoke ngOnDestroy on multi providers', () => {
+        .describe('ngOnDestroy on multi providers', () => {
-          // TODO(FW-1866): currently we only assert that the hook was called,
+          it('should invoke ngOnDestroy on multi providers with the correct context', () => {
-          // but we should also be checking that the correct context was passed in.
+            const destroyCalls: any[] = [];
-          let destroyCalls = 0;
             const SERVICES = new InjectionToken<any>('SERVICES');
 
             @Injectable()
             class DestroyService {
-            ngOnDestroy() { destroyCalls++; }
+              ngOnDestroy() {
+                destroyCalls.push(this);
+              }
             }
 
             @Injectable()
             class OtherDestroyService {
-            ngOnDestroy() { destroyCalls++; }
+              ngOnDestroy() {
+                destroyCalls.push(this);
+              }
             }
 
             @Component({
@@ -343,13 +378,137 @@ describe('providers', () => {
             fixture.detectChanges();
             fixture.destroy();
 
-          expect(destroyCalls).toBe(2);
+            expect(destroyCalls).toEqual([
+              jasmine.any(DestroyService), jasmine.any(OtherDestroyService)
+            ]);
           });
 
+          it('should invoke destroy hooks on multi providers with the correct context, if only some have a destroy hook',
+             () => {
+               const destroyCalls: any[] = [];
+               const SERVICES = new InjectionToken<any>('SERVICES');
+
+               @Injectable()
+               class Service1 {
+               }
+
+               @Injectable()
+               class Service2 {
+                 ngOnDestroy() {
+                   destroyCalls.push(this);
+                 }
+               }
+
+               @Injectable()
+               class Service3 {
+               }
+
+               @Injectable()
+               class Service4 {
+                 ngOnDestroy() {
+                   destroyCalls.push(this);
+                 }
+               }
+
+               @Component({
+                 template: '<div></div>',
+                 providers: [
+                   {provide: SERVICES, useClass: Service1, multi: true},
+                   {provide: SERVICES, useClass: Service2, multi: true},
+                   {provide: SERVICES, useClass: Service3, multi: true},
+                   {provide: SERVICES, useClass: Service4, multi: true},
+                 ]
+               })
+               class App {
+                 constructor(@Inject(SERVICES) s: any) {}
+               }
+
+               TestBed.configureTestingModule({declarations: [App]});
+               const fixture = TestBed.createComponent(App);
+               fixture.detectChanges();
+               fixture.destroy();
+
+               expect(destroyCalls).toEqual([jasmine.any(Service2), jasmine.any(Service4)]);
+             });
+
+          it('should not invoke ngOnDestroy on multi providers created via useFactory', () => {
+            let destroyCalls = 0;
+            const SERVICES = new InjectionToken<any>('SERVICES');
+
+            @Injectable()
+            class DestroyService {
+              ngOnDestroy() {
+                destroyCalls++;
+              }
+            }
+
+            @Injectable()
+            class OtherDestroyService {
+              ngOnDestroy() {
+                destroyCalls++;
+              }
+            }
+
+            @Component({
+              template: '<div></div>',
+              providers: [
+                {provide: SERVICES, useFactory: () => new DestroyService(), multi: true},
+                {provide: SERVICES, useFactory: () => new OtherDestroyService(), multi: true},
+              ]
+            })
+            class App {
+              constructor(@Inject(SERVICES) s: any) {}
+            }
+
+            TestBed.configureTestingModule({declarations: [App]});
+            const fixture = TestBed.createComponent(App);
+            fixture.detectChanges();
+            fixture.destroy();
+
+            expect(destroyCalls).toBe(0);
+          });
+        });
+
+    modifiedInIvy('ViewEngine did not support destroy hooks on multi providers')
+        .it('should not invoke ngOnDestroy on multi providers', () => {
+          let destroyCalls = 0;
+          const SERVICES = new InjectionToken<any>('SERVICES');
+
+          @Injectable()
+          class DestroyService {
+            ngOnDestroy() {
+              destroyCalls++;
+            }
+          }
+
+          @Injectable()
+          class OtherDestroyService {
+            ngOnDestroy() {
+              destroyCalls++;
+            }
+          }
+
+          @Component({
+            template: '<div></div>',
+            providers: [
+              {provide: SERVICES, useClass: DestroyService, multi: true},
+              {provide: SERVICES, useClass: OtherDestroyService, multi: true},
+            ]
+          })
+          class App {
+            constructor(@Inject(SERVICES) s: any) {}
+          }
+
+          TestBed.configureTestingModule({declarations: [App]});
+          const fixture = TestBed.createComponent(App);
+          fixture.detectChanges();
+          fixture.destroy();
+
+          expect(destroyCalls).toBe(0);
+        });
   });
 
   describe('components and directives', () => {
-
     class MyService {
       value = 'some value';
     }
@@ -409,7 +568,6 @@ describe('providers', () => {
   });
 
   describe('forward refs', () => {
-
     it('should support forward refs in provider deps', () => {
       class MyService {
         constructor(public dep: {value: string}) {}
@@ -444,7 +602,6 @@ describe('providers', () => {
     });
 
     it('should support forward refs in useClass when impl version is also provided', () => {
-
       @Injectable({providedIn: 'root', useClass: forwardRef(() => SomeProviderImpl)})
       abstract class SomeProvider {
       }
@@ -490,11 +647,9 @@ describe('providers', () => {
 
       expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl);
     });
-
   });
 
   describe('flags', () => {
-
     class MyService {
       constructor(public value: OtherService|null) {}
     }