refactor(core): remove ActiveIndexFlag from LContainer (#37073)

The ActiveIndexFlag is no longer needed because we no longer have "inline embedded views".
There is only one type of embedded view so we do not need complex tracking for
inline embedded views.

HAS_TRANSPLANTED_VIEWS now takes the place of the ACTIVE_INDEX slot as a
simple boolean rather than being a shifted flag inside the ACTIVE_INDEX bits.

PR Close #37073

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

@@ -13,7 +13,7 @@ import {KeyValueArray} from '../../util/array_utils';
 import {assertDefined} from '../../util/assert';
 import {createNamedArrayType} from '../../util/named_array_type';
 import {initNgDevMode} from '../../util/ng_dev_mode';
-import {ACTIVE_INDEX, ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS, NATIVE} from '../interfaces/container';
+import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE} from '../interfaces/container';
 import {DirectiveDefList, PipeDefList, ViewQueriesFunction} from '../interfaces/definition';
 import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, TIcu} from '../interfaces/i18n';
 import {PropertyAliases, TConstants, TContainerNode, TElementNode, TNode as ITNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TViewNode} from '../interfaces/node';
@@ -23,7 +23,7 @@ import {RComment, RElement, Renderer3, RendererFactory3, RNode} from '../interfa
 import {getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate, TStylingKey, TStylingRange} from '../interfaces/styling';
 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';
+import {getTNode, unwrapRNode} from '../util/view_utils';
 
 const NG_DEV_MODE = ((typeof ngDevMode === 'undefined' || !!ngDevMode) && initNgDevMode());
 
@@ -510,12 +510,8 @@ 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 hasTransplantedViews(): boolean {
-    return (this._raw_lContainer[ACTIVE_INDEX] & ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS) ===
+    return this._raw_lContainer[HAS_TRANSPLANTED_VIEWS];
-        ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS;
   }
   get views(): LViewDebug[] {
     return this._raw_lContainer.slice(CONTAINER_HEADER_OFFSET)

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

@@ -21,7 +21,7 @@ import {getFactoryDef} from '../definition';
 import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di';
 import {throwMultipleComponentError} from '../errors';
 import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks';
-import {ACTIVE_INDEX, ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS} from '../interfaces/container';
+import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, 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, PropertyAliases, PropertyAliasValue, TAttributes, TConstants, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
@@ -35,7 +35,7 @@ import {enterView, getBindingsEnabled, getCheckNoChangesMode, getCurrentDirectiv
 import {NO_CHANGE} from '../tokens';
 import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils';
 import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils';
-import {getLViewParent} from '../util/view_traversal_utils';
+import {getFirstLContainer, getLViewParent, getNextLContainer} from '../util/view_traversal_utils';
 import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapLView, updateTransplantedViewCount, viewAttachedToChangeDetector} from '../util/view_utils';
 
 import {selectIndexInternal} from './advance';
@@ -1603,16 +1603,16 @@ export function createLContainer(
   ngDevMode && !isProceduralRenderer(currentView[RENDERER]) && assertDomNode(native);
   // https://jsperf.com/array-literal-vs-new-array-really
   const lContainer: LContainer = new (ngDevMode ? LContainerArray : Array)(
-      hostNative,  // host native
+      hostNative,   // host native
-      true,        // Boolean `true` in this position signifies that this is an `LContainer`
+      true,         // Boolean `true` in this position signifies that this is an `LContainer`
-      ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY << ActiveIndexFlag.SHIFT,  // active index
+      false,        // has transplanted views
-      currentView,                                                           // parent
+      currentView,  // parent
-      null,                                                                  // next
+      null,         // next
-      0,       // transplanted views to refresh count
+      0,            // transplanted views to refresh count
-      tNode,   // t_host
+      tNode,        // t_host
-      native,  // native,
+      native,       // native,
-      null,    // view refs
+      null,         // view refs
-      null,    // moved views
+      null,         // moved views
   );
   ngDevMode &&
       assertEqual(
@@ -1640,63 +1640,32 @@ function refreshDynamicEmbeddedViews(lView: LView) {
   }
 }
 
-/**
- * Gets the first `LContainer` in the LView or `null` if none exists.
- */
-function getFirstLContainer(lView: LView): LContainer|null {
-  let viewOrContainer = lView[CHILD_HEAD];
-  while (viewOrContainer !== null &&
-         !(isLContainer(viewOrContainer) &&
-           viewOrContainer[ACTIVE_INDEX] >> ActiveIndexFlag.SHIFT ===
-               ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY)) {
-    viewOrContainer = viewOrContainer[NEXT];
-  }
-  return viewOrContainer;
-}
-
-/**
- * Gets the next `LContainer` that is a sibling of the given container.
- */
-function getNextLContainer(container: LContainer): LContainer|null {
-  let viewOrContainer = container[NEXT];
-  while (viewOrContainer !== null &&
-         !(isLContainer(viewOrContainer) &&
-           viewOrContainer[ACTIVE_INDEX] >> ActiveIndexFlag.SHIFT ===
-               ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY)) {
-    viewOrContainer = viewOrContainer[NEXT];
-  }
-  return viewOrContainer;
-}
-
 /**
  * Mark transplanted views as needing to be refreshed at their insertion points.
  *
- * See: `ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS` and `LView[DECLARATION_COMPONENT_VIEW]` for
- * explanation of transplanted views.
- *
  * @param lView The `LView` that may have transplanted views.
  */
 function markTransplantedViewsForRefresh(lView: LView) {
   for (let lContainer = getFirstLContainer(lView); lContainer !== null;
        lContainer = getNextLContainer(lContainer)) {
-    if ((lContainer[ACTIVE_INDEX] & ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS) !== 0) {
+    if (!lContainer[HAS_TRANSPLANTED_VIEWS]) continue;
-      const movedViews = lContainer[MOVED_VIEWS]!;
+
-      ngDevMode && assertDefined(movedViews, 'Transplanted View flags set but missing MOVED_VIEWS');
+    const movedViews = lContainer[MOVED_VIEWS]!;
-      for (let i = 0; i < movedViews.length; i++) {
+    ngDevMode && assertDefined(movedViews, 'Transplanted View flags set but missing MOVED_VIEWS');
-        const movedLView = movedViews[i]!;
+    for (let i = 0; i < movedViews.length; i++) {
-        const insertionLContainer = movedLView[PARENT] as LContainer;
+      const movedLView = movedViews[i]!;
-        ngDevMode && assertLContainer(insertionLContainer);
+      const insertionLContainer = movedLView[PARENT] as LContainer;
-        // We don't want to increment the counter if the moved LView was already marked for
+      ngDevMode && assertLContainer(insertionLContainer);
-        // refresh.
+      // We don't want to increment the counter if the moved LView was already marked for
-        if ((movedLView[FLAGS] & LViewFlags.RefreshTransplantedView) === 0) {
+      // refresh.
-          updateTransplantedViewCount(insertionLContainer, 1);
+      if ((movedLView[FLAGS] & LViewFlags.RefreshTransplantedView) === 0) {
-        }
+        updateTransplantedViewCount(insertionLContainer, 1);
-        // Note, it is possible that the `movedViews` is tracking views that are transplanted *and*
-        // those that aren't (declaration component === insertion component). In the latter case,
-        // it's fine to add the flag, as we will clear it immediately in
-        // `refreshDynamicEmbeddedViews` for the view currently being refreshed.
-        movedLView[FLAGS] |= LViewFlags.RefreshTransplantedView;
       }
+      // Note, it is possible that the `movedViews` is tracking views that are transplanted *and*
+      // those that aren't (declaration component === insertion component). In the latter case,
+      // it's fine to add the flag, as we will clear it immediately in
+      // `refreshDynamicEmbeddedViews` for the view currently being refreshed.
+      movedLView[FLAGS] |= LViewFlags.RefreshTransplantedView;
     }
   }
 }

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

@@ -26,7 +26,16 @@ export const TYPE = 1;
  * without having to remember the specific indices.
  * Uglify will inline these when minifying so there shouldn't be a cost.
  */
-export const ACTIVE_INDEX = 2;
+
+/**
+ * Flag to signify that this `LContainer` may have transplanted views which need to be change
+ * detected. (see: `LView[DECLARATION_COMPONENT_VIEW])`.
+ *
+ * This flag, once set, is never unset for the `LContainer`. This means that when unset we can skip
+ * a lot of work in `refreshDynamicEmbeddedViews`. But when set we still need to verify
+ * that the `MOVED_VIEWS` are transplanted and on-push.
+ */
+export const HAS_TRANSPLANTED_VIEWS = 2;
 
 // PARENT, NEXT, TRANSPLANTED_VIEWS_TO_REFRESH are indices 3, 4, and 5
 // As we already have these constants in LView, we don't need to re-create them.
@@ -47,32 +56,6 @@ export const MOVED_VIEWS = 9;
  */
 export const CONTAINER_HEADER_OFFSET = 10;
 
-
-/**
- * Used to track Transplanted `LView`s (see: `LView[DECLARATION_COMPONENT_VIEW])`
- */
-export const enum ActiveIndexFlag {
-  /**
-   * Flag which signifies that the `LContainer` does not have any inline embedded views.
-   */
-  DYNAMIC_EMBEDDED_VIEWS_ONLY = -1,
-
-  /**
-   * Flag to signify that this `LContainer` may have transplanted views which need to be change
-   * detected. (see: `LView[DECLARATION_COMPONENT_VIEW])`.
-   *
-   * This flag once set is never unset for the `LContainer`. This means that when unset we can skip
-   * a lot of work in `refreshDynamicEmbeddedViews`. But when set we still need to verify
-   * that the `MOVED_VIEWS` are transplanted and on-push.
-   */
-  HAS_TRANSPLANTED_VIEWS = 1,
-
-  /**
-   * Number of bits to shift inline embedded views counter to make space for other flags.
-   */
-  SHIFT = 1,
-}
-
 /**
  * The state associated with a container.
  *
@@ -97,16 +80,12 @@ export interface LContainer extends Array<any> {
   [TYPE]: true;
 
   /**
-   * The next active index in the views array to read or write to. This helps us
+   * Flag to signify that this `LContainer` may have transplanted views which need to be change
-   * keep track of where we are in the views array.
+   * detected. (see: `LView[DECLARATION_COMPONENT_VIEW])`.
-   * In the case the LContainer is created for a ViewContainerRef,
-   * it is set to null to identify this scenario, as indices are "absolute" in that case,
-   * i.e. provided directly by the user of the ViewContainerRef API.
    *
-   * The lowest bit signals that this `LContainer` has transplanted views which need to be change
+   * This flag, once set, is never unset for the `LContainer`.
-   * detected as part of the declaration CD. (See `LView[DECLARATION_COMPONENT_VIEW]`)
    */
-  [ACTIVE_INDEX]: ActiveIndexFlag;
+  [HAS_TRANSPLANTED_VIEWS]: boolean;
 
   /**
    * Access to the parent view is necessary so we can propagate back

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

@@ -262,7 +262,7 @@ export interface LView extends Array<any> {
    *
    * see also:
    *   - https://hackmd.io/@mhevery/rJUJsvv9H write up of the problem
-   *   - `LContainer[ACTIVE_INDEX]` for flag which marks which `LContainer` has transplanted views.
+   *   - `LContainer[HAS_TRANSPLANTED_VIEWS]` which marks which `LContainer` has transplanted views.
    *   - `LContainer[TRANSPLANT_HEAD]` and `LContainer[TRANSPLANT_TAIL]` storage for transplanted
    *   - `LView[DECLARATION_LCONTAINER]` similar problem for queries
    *   - `LContainer[MOVED_VIEWS]` similar problem for queries

packages/core/src/render3/node_manipulation.ts

@@ -13,7 +13,7 @@ import {assertDefined, assertDomNode, assertEqual, assertString} from '../util/a
 
 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';
+import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE, unusedValueExportToPlacateAjd as unused1} from './interfaces/container';
 import {ComponentDef} from './interfaces/definition';
 import {NodeInjectorFactory} from './interfaces/injector';
 import {TElementNode, TNode, TNodeFlags, TNodeType, TProjectionNode, TViewNode, unusedValueExportToPlacateAjd as unused2} from './interfaces/node';
@@ -276,7 +276,7 @@ function trackMovedView(declarationContainer: LContainer, lView: LView) {
     // At this point the declaration-component is not same as insertion-component; this means that
     // this is a transplanted view. Mark the declared lView as having transplanted views so that
     // those views can participate in CD.
-    declarationContainer[ACTIVE_INDEX] |= ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS;
+    declarationContainer[HAS_TRANSPLANTED_VIEWS] = true;
   }
   if (movedViews === null) {
     declarationContainer[MOVED_VIEWS] = [lView];

packages/core/src/render3/util/view_traversal_utils.ts

@@ -8,8 +8,10 @@
 
 import {assertDefined} from '../../util/assert';
 import {assertLView} from '../assert';
+import {LContainer} from '../interfaces/container';
 import {isLContainer, isLView} from '../interfaces/type_checks';
-import {CONTEXT, FLAGS, LView, LViewFlags, PARENT, RootContext} from '../interfaces/view';
+import {CHILD_HEAD, CONTEXT, FLAGS, LView, LViewFlags, NEXT, PARENT, RootContext} from '../interfaces/view';
+
 import {readPatchedLView} from './view_utils';
 
 
@@ -53,3 +55,25 @@ export function getRootContext(viewOrComponent: LView|{}): RootContext {
       assertDefined(rootView[CONTEXT], 'RootView has no context. Perhaps it is disconnected?');
   return rootView[CONTEXT] as RootContext;
 }
+
+
+/**
+ * Gets the first `LContainer` in the LView or `null` if none exists.
+ */
+export function getFirstLContainer(lView: LView): LContainer|null {
+  return getNearestLContainer(lView[CHILD_HEAD]);
+}
+
+/**
+ * Gets the next `LContainer` that is a sibling of the given container.
+ */
+export function getNextLContainer(container: LContainer): LContainer|null {
+  return getNearestLContainer(container[NEXT]);
+}
+
+function getNearestLContainer(viewOrContainer: LContainer|LView|null) {
+  while (viewOrContainer !== null && !isLContainer(viewOrContainer)) {
+    viewOrContainer = viewOrContainer[NEXT];
+  }
+  return viewOrContainer;
+}

packages/core/src/render3/util/view_utils.ts

@@ -8,7 +8,7 @@
 
 import {assertDataInRange, assertDefined, assertDomNode, assertGreaterThan, assertLessThan} from '../../util/assert';
 import {assertTNodeForLView} from '../assert';
-import {ACTIVE_INDEX, ActiveIndexFlag, LContainer, TYPE} from '../interfaces/container';
+import {LContainer, TYPE} from '../interfaces/container';
 import {LContext, MONKEY_PATCH_KEY_NAME} from '../interfaces/context';
 import {TConstants, TNode} from '../interfaces/node';
 import {isProceduralRenderer, RNode} from '../interfaces/renderer';
@@ -188,14 +188,6 @@ export function resetPreOrderHookFlags(lView: LView) {
   lView[PREORDER_HOOK_FLAGS] = 0;
 }
 
-export function getLContainerActiveIndex(lContainer: LContainer) {
-  return lContainer[ACTIVE_INDEX] >> ActiveIndexFlag.SHIFT;
-}
-
-export function setLContainerActiveIndex(lContainer: LContainer, index: number) {
-  lContainer[ACTIVE_INDEX] = index << ActiveIndexFlag.SHIFT;
-}
-
 /**
  * Updates the `TRANSPLANTED_VIEWS_TO_REFRESH` counter on the `LContainer` as well as the parents
  * whose

packages/core/src/render3/view_engine_compatibility.ts

@@ -21,7 +21,7 @@ import {assertDefined, assertEqual, assertGreaterThan, assertLessThan} from '../
 import {assertLContainer} from './assert';
 import {getParentInjectorLocation, NodeInjector} from './di';
 import {addToViewTree, createLContainer, createLView, renderView} from './instructions/shared';
-import {ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, VIEW_REFS} from './interfaces/container';
+import {CONTAINER_HEADER_OFFSET, LContainer, VIEW_REFS} from './interfaces/container';
 import {TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
 import {isProceduralRenderer, RComment, RElement} from './interfaces/renderer';
 import {isComponentHost, isLContainer, isLView, isRootView} from './interfaces/type_checks';
@@ -31,7 +31,7 @@ import {addRemoveViewFromContainer, appendChild, detachView, getBeforeNodeForVie
 import {getParentInjectorTNode} from './node_util';
 import {getLView, getPreviousOrParentTNode} from './state';
 import {getParentInjectorView, hasParentInjector} from './util/injector_utils';
-import {getComponentLViewByIndex, getNativeByTNode, setLContainerActiveIndex, unwrapRNode, viewAttachedToContainer} from './util/view_utils';
+import {getComponentLViewByIndex, getNativeByTNode, unwrapRNode, viewAttachedToContainer} from './util/view_utils';
 import {ViewRef} from './view_ref';
 
 
@@ -349,7 +349,6 @@ export function createContainerRef(
   if (isLContainer(slotValue)) {
     // If the host is a container, we don't need to create a new LContainer
     lContainer = slotValue;
-    setLContainerActiveIndex(lContainer, ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY);
   } else {
     let commentNode: RComment;
     // If the host is an element container, the native host element is guaranteed to be a

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

@@ -1,7 +1,4 @@
 [
-  {
-    "name": "ACTIVE_INDEX"
-  },
   {
     "name": "BLOOM_MASK"
   },
@@ -44,6 +41,9 @@
   {
     "name": "FLAGS"
   },
+  {
+    "name": "HAS_TRANSPLANTED_VIEWS"
+  },
   {
     "name": "HEADER_OFFSET"
   },
@@ -362,6 +362,9 @@
   {
     "name": "getNativeByTNode"
   },
+  {
+    "name": "getNearestLContainer"
+  },
   {
     "name": "getNextLContainer"
   },

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -1,7 +1,4 @@
 [
-  {
-    "name": "ACTIVE_INDEX"
-  },
   {
     "name": "BLOOM_MASK"
   },
@@ -41,6 +38,9 @@
   {
     "name": "FLAGS"
   },
+  {
+    "name": "HAS_TRANSPLANTED_VIEWS"
+  },
   {
     "name": "HEADER_OFFSET"
   },
@@ -293,6 +293,9 @@
   {
     "name": "getNativeByTNode"
   },
+  {
+    "name": "getNearestLContainer"
+  },
   {
     "name": "getNextLContainer"
   },

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -1,7 +1,4 @@
 [
-  {
-    "name": "ACTIVE_INDEX"
-  },
   {
     "name": "BLOOM_MASK"
   },
@@ -71,6 +68,9 @@
   {
     "name": "FLAGS"
   },
+  {
+    "name": "HAS_TRANSPLANTED_VIEWS"
+  },
   {
     "name": "HEADER_OFFSET"
   },
@@ -656,6 +656,9 @@
   {
     "name": "getNativeByTNode"
   },
+  {
+    "name": "getNearestLContainer"
+  },
   {
     "name": "getNextLContainer"
   },
@@ -1136,9 +1139,6 @@
   {
     "name": "setIsNotParent"
   },
-  {
-    "name": "setLContainerActiveIndex"
-  },
   {
     "name": "setPreviousOrParentTNode"
   },