Memory leak fixes for several views (#7057)

* Change the mount utility to use Vue's createApp and defineComponent methods * Fix display layout memory leaks caused by `getSelectionContext` * fix some display layout leaks due to use of slots * Fix imagery memory leak (removed span tag). NOTE: CompassRose svg leaks memory - must test on firefox to see if this is a Chrome leak. * Fix ActionsAPI action collection and applicable actions leak. * Fix flexible layout memory leaks - remove listeners on unmount. NOTE: One type of overlay plot (Rover Yaw) is still leaking. * pass in the el on mount * e2e test config and spec changes * Remove mounting of limit lines. Use components directly * test: remove `.only()` * Fix display layout memory leaks * Enable passing tests * e2e README and appActions should be what master has. * lint: add word to cspell list * lint: fixes * lint:fix * fix: revert `el` change * fix: remove empty span * fix: creating shapes in displayLayout * fix: avoid `splice` as it loses reactivity * test: reduce timeout time * quick fixes * add prod mode and convert the test config to select the correct mode * Fix webpack prod config * Add launch flag for exposing window.gc * never worked * explicit naming * rename * We don't need to destroy view providers * test: increase timeout time * test: unskip all mem tests * fix(vue-loader): disable static hoisting * chore: run `test:perf:memory` * Don't destroy view providers * Move context menu once listener to beforeUnmount instead. * Disconnect all resize observers on unmount * Delete Test vue component * Use beforeUnmount and remove splice(0) in favor of [] for emptying arrays * re-structure * fix: unregister listener in pane.vue * test: tweak timeouts * chore: lint:fix * test: unskip perf tests * fix: unregister events properly * fix: unregister listener * fix: unregister listener * fix: unregister listener * fix: use `unmounted()` * fix: unregister listeners * fix: unregister listener properly * chore: lint:fix * test: fix imagery layer toggle test * test: increase timeout * Don't use anonymous functions for listeners * Destroy objects and event listeners properly * Delete config stores that are created by components * Use the right unmount hook. Destroy mounted view on unmount. * Use unmounted, not beforeUnmounted * Lint fixes * Fix time strip memory leak * Undo unneeded change for memory leaks. * chore: combine common webpack configs --------- Co-authored-by: Jesse Mazzella <jesse.d.mazzella@nasa.gov> Co-authored-by: John Hill <john.c.hill@nasa.gov>
2025-06-15 05:38:12 +00:00 · 2023-09-20 10:34:05 -07:00
parent 61e7050391
commit b8949db767
65 changed files with 887 additions and 393 deletions
--- a/src/plugins/displayLayout/DisplayLayoutToolbar.js
+++ b/src/plugins/displayLayout/DisplayLayoutToolbar.js
@ -170,7 +170,7 @@ define(['lodash'], function (_) {
                if (form) {
                  showForm(form, name, selectionPath);
                } else {
-                  selectionPath[0].context.addElement(name);
+                  openmct.objectViews.emit('contextAction', 'addElement', name);
                }
              },
              key: 'add',
@ -236,7 +236,6 @@ define(['lodash'], function (_) {
            icon: 'icon-trash',
            title: 'Delete the selected object',
            method: function () {
-              let removeItem = selectionPath[1].context.removeItem;
              let prompt = openmct.overlays.dialog({
                iconClass: 'alert',
                message: `Warning! This action will remove this item from the Display Layout. Do you want to continue?`,
@ -245,7 +244,11 @@ define(['lodash'], function (_) {
                    label: 'OK',
                    emphasis: 'true',
                    callback: function () {
-                      removeItem(getAllTypes(selection));
+                      openmct.objectViews.emit(
+                        'contextAction',
+                        'removeItem',
+                        getAllTypes(selection)
+                      );
                      prompt.dismiss();
                    }
                  },
@ -290,7 +293,12 @@ define(['lodash'], function (_) {
              }
            ],
            method: function (option) {
-              selectionPath[1].context.orderItem(option.value, getAllTypes(selectedObjects));
+              openmct.objectViews.emit(
+                'contextAction',
+                'orderItem',
+                option.value,
+                getAllTypes(selectedObjects)
+              );
            }
          };
        }
@ -474,9 +482,7 @@ define(['lodash'], function (_) {
            icon: 'icon-duplicate',
            title: 'Duplicate the selected object',
            method: function () {
-              let duplicateItem = selectionPath[1].context.duplicateItem;
-
-              duplicateItem(selection);
+              openmct.objectViews.emit('contextAction', 'duplicateItem', selection);
            }
          };
        }
@ -555,6 +561,7 @@ define(['lodash'], function (_) {

        function getViewSwitcherMenu(selectedParent, selectionPath, selection) {
          if (selection.length === 1) {
+            // eslint-disable-next-line no-unused-vars
            let displayLayoutContext = selectionPath[1].context;
            let selectedItemContext = selectionPath[0].context;
            let selectedItemType = selectedItemContext.item.type;
@ -574,14 +581,18 @@ define(['lodash'], function (_) {
                label: 'View type',
                options: viewOptions,
                method: function (option) {
-                  displayLayoutContext.switchViewType(selectedItemContext, option.value, selection);
+                  openmct.objectViews.emit(
+                    'contextAction',
+                    'switchViewType',
+                    selectedItemContext,
+                    option.value,
+                    selection
+                  );
                }
              };
            }
          } else if (selection.length > 1) {
            if (areAllViews('telemetry-view', 'layoutItem.type', selection)) {
-              let displayLayoutContext = selectionPath[1].context;
-
              return {
                control: 'menu',
                domainObject: selectedParent,
@ -590,12 +601,15 @@ define(['lodash'], function (_) {
                label: 'View type',
                options: APPLICABLE_VIEWS['telemetry-view-multi'],
                method: function (option) {
-                  displayLayoutContext.mergeMultipleTelemetryViews(selection, option.value);
+                  openmct.objectViews.emit(
+                    'contextAction',
+                    'mergeMultipleTelemetryViews',
+                    selection,
+                    option.value
+                  );
                }
              };
            } else if (areAllViews('telemetry.plot.overlay', 'item.type', selection)) {
-              let displayLayoutContext = selectionPath[1].context;
-
              return {
                control: 'menu',
                domainObject: selectedParent,
@ -603,7 +617,12 @@ define(['lodash'], function (_) {
                title: 'Merge into a stacked plot',
                options: APPLICABLE_VIEWS['telemetry.plot.overlay-multi'],
                method: function (option) {
-                  displayLayoutContext.mergeMultipleOverlayPlots(selection, option.value);
+                  openmct.objectViews.emit(
+                    'contextAction',
+                    'mergeMultipleOverlayPlots',
+                    selection,
+                    option.value
+                  );
                }
              };
            }
@ -627,7 +646,7 @@ define(['lodash'], function (_) {
            domainObject: displayLayoutContext.item,
            icon: ICON_GRID_SHOW,
            method: function () {
-              displayLayoutContext.toggleGrid();
+              openmct.objectViews.emit('contextAction', 'toggleGrid');

              this.icon = this.icon === ICON_GRID_SHOW ? ICON_GRID_HIDE : ICON_GRID_SHOW;
            },
@ -653,7 +672,7 @@ define(['lodash'], function (_) {

        function showForm(formStructure, name, selectionPath) {
          openmct.forms.showForm(formStructure).then((changes) => {
-            selectionPath[0].context.addElement(name, changes);
+            openmct.objectViews.emit('contextAction', 'addElement', name, changes);
          });
        }

--- a/src/plugins/displayLayout/components/BoxView.vue
+++ b/src/plugins/displayLayout/components/BoxView.vue
@ -25,14 +25,16 @@
    :item="item"
    :grid-size="gridSize"
    :is-editing="isEditing"
-    @move="(gridDelta) => $emit('move', gridDelta)"
-    @endMove="() => $emit('endMove')"
+    @move="move"
+    @endMove="endMove"
  >
-    <div
-      class="c-box-view u-style-receiver js-style-receiver"
-      :class="[styleClass]"
-      :style="style"
-    ></div>
+    <template #content>
+      <div
+        class="c-box-view u-style-receiver js-style-receiver"
+        :class="[styleClass]"
+        :style="style"
+      ></div>
+    </template>
  </layout-frame>
 </template>

@ -115,10 +117,18 @@ export default {
      this.initSelect
    );
  },
-  unmounted() {
+  beforeUnmount() {
    if (this.removeSelectable) {
      this.removeSelectable();
    }
+  },
+  methods: {
+    move(gridDelta) {
+      this.$emit('move', gridDelta);
+    },
+    endMove() {
+      this.$emit('endMove');
+    }
  }
 };
 </script>
--- a/src/plugins/displayLayout/components/DisplayLayout.vue
+++ b/src/plugins/displayLayout/components/DisplayLayout.vue
@ -145,7 +145,7 @@ function getItemDefinition(itemType, ...options) {

 export default {
  components: components,
-  inject: ['openmct', 'objectPath', 'options', 'objectUtils', 'currentView'],
+  inject: ['openmct', 'objectPath', 'options', 'currentView'],
  props: {
    domainObject: {
      type: Object,
@ -222,13 +222,21 @@ export default {
    this.composition.load();
    this.gridDimensions = [this.$el.offsetWidth, this.$el.scrollHeight];

-    this.openmct.objects.observe(this.domainObject, 'configuration.items', (items) => {
-      this.layoutItems = items;
-    });
+    this.unObserveItems = this.openmct.objects.observe(
+      this.domainObject,
+      'configuration.items',
+      (items) => {
+        this.layoutItems = [...items];
+      }
+    );

    this.watchDisplayResize();
  },
-  unmounted() {
+  beforeUnmount() {
+    if (this.unObserveItems) {
+      this.unObserveItems();
+    }
+    this.unwatchDisplayResize();
    this.openmct.selection.off('change', this.setSelection);
    this.composition.off('add', this.addChild);
    this.composition.off('remove', this.removeChild);
@ -252,9 +260,15 @@ export default {
      this.$el.click();
    },
    watchDisplayResize() {
-      const resizeObserver = new ResizeObserver(() => this.updateGrid());
+      this.unwatchDisplayResize();
+      this.resizeObserver = new ResizeObserver(this.updateGrid);

-      resizeObserver.observe(this.$el);
+      this.resizeObserver.observe(this.$el);
+    },
+    unwatchDisplayResize() {
+      if (this.resizeObserver) {
+        this.resizeObserver.disconnect();
+      }
    },
    addElement(itemType, element) {
      this.addItem(itemType + '-view', element);
@ -624,7 +638,7 @@ export default {
          return this.openmct.objects.makeKeyString(item.identifier) !== keyString;
        }
      });
-      this.layoutItems = layoutItems;
+      this.layoutItems = [...layoutItems];
      this.mutate('configuration.items', layoutItems);
      this.clearSelection();
    },
--- a/src/plugins/displayLayout/components/EllipseView.vue
+++ b/src/plugins/displayLayout/components/EllipseView.vue
@ -25,14 +25,16 @@
    :item="item"
    :grid-size="gridSize"
    :is-editing="isEditing"
-    @move="(gridDelta) => $emit('move', gridDelta)"
-    @endMove="() => $emit('endMove')"
+    @move="move"
+    @endMove="endMove"
  >
-    <div
-      class="c-ellipse-view u-style-receiver js-style-receiver"
-      :class="[styleClass]"
-      :style="style"
-    ></div>
+    <template #content>
+      <div
+        class="c-ellipse-view u-style-receiver js-style-receiver"
+        :class="[styleClass]"
+        :style="style"
+      ></div>
+    </template>
  </layout-frame>
 </template>

@ -115,10 +117,18 @@ export default {
      this.initSelect
    );
  },
-  unmounted() {
+  beforeUnmount() {
    if (this.removeSelectable) {
      this.removeSelectable();
    }
+  },
+  methods: {
+    move(gridDelta) {
+      this.$emit('move', gridDelta);
+    },
+    endMove() {
+      this.$emit('endMove');
+    }
  }
 };
 </script>
--- a/src/plugins/displayLayout/components/ImageView.vue
+++ b/src/plugins/displayLayout/components/ImageView.vue
@ -25,10 +25,12 @@
    :item="item"
    :grid-size="gridSize"
    :is-editing="isEditing"
-    @move="(gridDelta) => $emit('move', gridDelta)"
-    @endMove="() => $emit('endMove')"
+    @move="move"
+    @endMove="endMove"
  >
-    <div class="c-image-view" :class="[styleClass]" :style="style"></div>
+    <template #content>
+      <div class="c-image-view" :style="style"></div>
+    </template>
  </layout-frame>
 </template>

@ -118,10 +120,18 @@ export default {
      this.initSelect
    );
  },
-  unmounted() {
+  beforeUnmount() {
    if (this.removeSelectable) {
      this.removeSelectable();
    }
+  },
+  methods: {
+    move(gridDelta) {
+      this.$emit('move', gridDelta);
+    },
+    endMove() {
+      this.$emit('endMove');
+    }
  }
 };
 </script>
--- a/src/plugins/displayLayout/components/LayoutFrame.vue
+++ b/src/plugins/displayLayout/components/LayoutFrame.vue
@ -29,7 +29,7 @@
    }"
    :style="style"
  >
-    <slot></slot>
+    <slot name="content"></slot>
    <div class="c-frame__move-bar" @mousedown.left="startMove($event)"></div>
  </div>
 </template>
--- a/src/plugins/displayLayout/components/SubobjectView.vue
+++ b/src/plugins/displayLayout/components/SubobjectView.vue
@ -20,24 +20,26 @@
 at runtime from the About dialog for additional information.
 -->
 <template>
-  <LayoutFrame
+  <layout-frame
    :item="item"
    :grid-size="gridSize"
    :is-editing="isEditing"
-    @move="(gridDelta) => $emit('move', gridDelta)"
-    @endMove="() => $emit('endMove')"
+    @move="move"
+    @endMove="endMove"
  >
-    <ObjectFrame
-      v-if="domainObject"
-      ref="objectFrame"
-      :domain-object="domainObject"
-      :object-path="currentObjectPath"
-      :has-frame="item.hasFrame"
-      :show-edit-view="false"
-      :layout-font-size="item.fontSize"
-      :layout-font="item.font"
-    />
-  </LayoutFrame>
+    <template #content>
+      <ObjectFrame
+        v-if="domainObject"
+        ref="objectFrame"
+        :domain-object="domainObject"
+        :object-path="currentObjectPath"
+        :has-frame="item.hasFrame"
+        :show-edit-view="false"
+        :layout-font-size="item.fontSize"
+        :layout-font="item.font"
+      />
+    </template>
+  </layout-frame>
 </template>

 <script>
@ -104,8 +106,7 @@ export default {
  data() {
    return {
      domainObject: undefined,
-      currentObjectPath: [],
-      mutablePromise: undefined
+      currentObjectPath: []
    };
  },
  watch: {
@ -168,6 +169,12 @@ export default {
          delete this.immediatelySelect;
        }
      });
+    },
+    move(gridDelta) {
+      this.$emit('move', gridDelta);
+    },
+    endMove() {
+      this.$emit('endMove');
    }
  }
 };
--- a/src/plugins/displayLayout/components/TelemetryView.vue
+++ b/src/plugins/displayLayout/components/TelemetryView.vue
@ -25,42 +25,44 @@
    :item="item"
    :grid-size="gridSize"
    :is-editing="isEditing"
-    @move="(gridDelta) => $emit('move', gridDelta)"
-    @endMove="() => $emit('endMove')"
+    @move="move"
+    @endMove="endMove"
  >
-    <div
-      v-if="domainObject"
-      ref="telemetryViewWrapper"
-      class="c-telemetry-view u-style-receiver"
-      :class="[itemClasses]"
-      :style="styleObject"
-      :data-font-size="item.fontSize"
-      :data-font="item.font"
-      @contextmenu.prevent="showContextMenu"
-      @mouseover.ctrl="showToolTip"
-      @mouseleave="hideToolTip"
-    >
-      <div class="is-status__indicator" :title="`This item is ${status}`"></div>
-      <div v-if="showLabel" class="c-telemetry-view__label">
-        <div class="c-telemetry-view__label-text">
-          {{ domainObject.name }}
-        </div>
-      </div>
-
+    <template #content>
      <div
-        v-if="showValue"
-        :title="fieldName"
-        class="c-telemetry-view__value"
-        :class="[telemetryClass]"
+        v-if="domainObject"
+        ref="telemetryViewWrapper"
+        class="c-telemetry-view u-style-receiver"
+        :class="[itemClasses]"
+        :style="styleObject"
+        :data-font-size="item.fontSize"
+        :data-font="item.font"
+        @contextmenu.prevent="showContextMenu"
+        @mouseover.ctrl="showToolTip"
+        @mouseleave="hideToolTip"
      >
-        <div class="c-telemetry-view__value-text">
-          {{ telemetryValue }}
-          <span v-if="unit && item.showUnits" class="c-telemetry-view__value-text__unit">
-            {{ unit }}
-          </span>
+        <div class="is-status__indicator" :title="`This item is ${status}`"></div>
+        <div v-if="showLabel" class="c-telemetry-view__label">
+          <div class="c-telemetry-view__label-text">
+            {{ domainObject.name }}
+          </div>
+        </div>
+
+        <div
+          v-if="showValue"
+          :title="fieldName"
+          class="c-telemetry-view__value"
+          :class="[telemetryClass]"
+        >
+          <div class="c-telemetry-view__value-text">
+            {{ telemetryValue }}
+            <span v-if="unit && item.showUnits" class="c-telemetry-view__value-text__unit">
+              {{ unit }}
+            </span>
+          </div>
        </div>
      </div>
-    </div>
+    </template>
  </layout-frame>
 </template>

@ -388,6 +390,12 @@ export default {
    async showToolTip() {
      const { BELOW } = this.openmct.tooltips.TOOLTIP_LOCATIONS;
      this.buildToolTip(await this.getObjectPath(), BELOW, 'telemetryViewWrapper');
+    },
+    move(gridDelta) {
+      this.$emit('move', gridDelta);
+    },
+    endMove() {
+      this.$emit('endMove');
    }
  }
 };
--- a/src/plugins/displayLayout/components/TextView.vue
+++ b/src/plugins/displayLayout/components/TextView.vue
@ -25,18 +25,20 @@
    :item="item"
    :grid-size="gridSize"
    :is-editing="isEditing"
-    @move="(gridDelta) => $emit('move', gridDelta)"
-    @endMove="() => $emit('endMove')"
+    @move="move"
+    @endMove="endMove"
  >
-    <div
-      class="c-text-view u-style-receiver js-style-receiver"
-      :data-font-size="item.fontSize"
-      :data-font="item.font"
-      :class="[styleClass]"
-      :style="style"
-    >
-      <div class="c-text-view__text">{{ item.text }}</div>
-    </div>
+    <template #content>
+      <div
+        class="c-text-view u-style-receiver js-style-receiver"
+        :data-font-size="item.fontSize"
+        :data-font="item.font"
+        :class="[styleClass]"
+        :style="style"
+      >
+        <div class="c-text-view__text">{{ item.text }}</div>
+      </div>
+    </template>
  </layout-frame>
 </template>

@ -127,10 +129,18 @@ export default {
      this.initSelect
    );
  },
-  unmounted() {
+  beforeUnmount() {
    if (this.removeSelectable) {
      this.removeSelectable();
    }
+  },
+  methods: {
+    move(gridDelta) {
+      this.$emit('move', gridDelta);
+    },
+    endMove() {
+      this.$emit('endMove');
+    }
  }
 };
 </script>
--- a/src/plugins/displayLayout/plugin.js
+++ b/src/plugins/displayLayout/plugin.js
@ -20,7 +20,6 @@
 * at runtime from the About dialog for additional information.
 *****************************************************************************/

-import objectUtils from 'objectUtils';
 import mount from 'utils/mount';

 import CopyToClipboardAction from './actions/CopyToClipboardAction';
@ -38,7 +37,6 @@ class DisplayLayoutView {
    this.options = options;

    this.component = null;
-    this.app = null;
  }

  show(container, isEditing) {
@ -52,7 +50,6 @@ class DisplayLayoutView {
          openmct: this.openmct,
          objectPath: this.objectPath,
          options: this.options,
-          objectUtils,
          currentView: this
        },
        data: () => {
@ -84,20 +81,17 @@ class DisplayLayoutView {
  getSelectionContext() {
    return {
      item: this.domainObject,
-      supportsMultiSelect: true,
-      addElement: this.component && this.component.$refs.displayLayout.addElement,
-      removeItem: this.component && this.component.$refs.displayLayout.removeItem,
-      orderItem: this.component && this.component.$refs.displayLayout.orderItem,
-      duplicateItem: this.component && this.component.$refs.displayLayout.duplicateItem,
-      switchViewType: this.component && this.component.$refs.displayLayout.switchViewType,
-      mergeMultipleTelemetryViews:
-        this.component && this.component.$refs.displayLayout.mergeMultipleTelemetryViews,
-      mergeMultipleOverlayPlots:
-        this.component && this.component.$refs.displayLayout.mergeMultipleOverlayPlots,
-      toggleGrid: this.component && this.component.$refs.displayLayout.toggleGrid
+      supportsMultiSelect: true
    };
  }

+  contextAction() {
+    const action = arguments[0];
+    if (this.component && this.component.$refs.displayLayout[action]) {
+      this.component.$refs.displayLayout[action](...Array.from(arguments).splice(1));
+    }
+  }
+
  onEditModeChange(isEditing) {
    this.component.isEditing = isEditing;
  }
@ -105,6 +99,7 @@ class DisplayLayoutView {
  destroy() {
    if (this._destroy) {
      this._destroy();
+      this.component = undefined;
    }
  }
 }