From 2f708417d8fce46ac85dbf0cb89c346a3b56a034 Mon Sep 17 00:00:00 2001
From: Billy <billyzh@amazon.com>
Date: Tue, 26 Sep 2023 23:17:06 -0700
Subject: [PATCH] feat: avoid memory leak when caching LCP candidates

---
 .../__tests__/Orchestration.test.ts           |  9 +++++
 src/plugins/event-plugins/WebVitalsPlugin.ts  |  9 +++--
 .../__tests__/WebVitalsPlugin.test.ts         | 35 +++++++++++++++++++
 src/utils/common-utils.ts                     | 15 ++++++++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/src/orchestration/__tests__/Orchestration.test.ts b/src/orchestration/__tests__/Orchestration.test.ts
index ecfb993a..e673c8be 100644
--- a/src/orchestration/__tests__/Orchestration.test.ts
+++ b/src/orchestration/__tests__/Orchestration.test.ts
@@ -21,6 +21,15 @@ jest.mock('../../dispatch/Dispatch', () => ({
     }))
 }));
 
+jest.mock('../../utils/common-utils', () => {
+    const originalModule = jest.requireActual('../../utils/common-utils');
+    return {
+        __esModule: true,
+        ...originalModule,
+        isLCPSupported: jest.fn().mockReturnValue(true)
+    };
+});
+
 const enableEventCache = jest.fn();
 const disableEventCache = jest.fn();
 const recordPageView = jest.fn();
diff --git a/src/plugins/event-plugins/WebVitalsPlugin.ts b/src/plugins/event-plugins/WebVitalsPlugin.ts
index 419aa888..9dc35c5e 100644
--- a/src/plugins/event-plugins/WebVitalsPlugin.ts
+++ b/src/plugins/event-plugins/WebVitalsPlugin.ts
@@ -25,7 +25,8 @@ import {
     HasLatency,
     ResourceType,
     performanceKey,
-    RumLCPAttribution
+    RumLCPAttribution,
+    isLCPSupported
 } from '../../utils/common-utils';
 
 export const WEB_VITAL_EVENT_PLUGIN_ID = 'web-vitals';
@@ -36,6 +37,7 @@ export class WebVitalsPlugin extends InternalPlugin {
     }
     private resourceEventIds = new Map<string, string>();
     private navigationEventId?: string;
+    private cacheLCPCandidates = isLCPSupported();
 
     // eslint-disable-next-line @typescript-eslint/no-empty-function
     enable(): void {}
@@ -58,7 +60,10 @@ export class WebVitalsPlugin extends InternalPlugin {
             // lcp resource is either image or text
             case PERFORMANCE_RESOURCE_EVENT_TYPE:
                 const details = event.details as ResourceEvent;
-                if (details.fileType === ResourceType.IMAGE) {
+                if (
+                    this.cacheLCPCandidates &&
+                    details.fileType === ResourceType.IMAGE
+                ) {
                     this.resourceEventIds.set(
                         performanceKey(event.details as HasLatency),
                         event.id
diff --git a/src/plugins/event-plugins/__tests__/WebVitalsPlugin.test.ts b/src/plugins/event-plugins/__tests__/WebVitalsPlugin.test.ts
index a590b885..d93bf3d8 100644
--- a/src/plugins/event-plugins/__tests__/WebVitalsPlugin.test.ts
+++ b/src/plugins/event-plugins/__tests__/WebVitalsPlugin.test.ts
@@ -1,3 +1,11 @@
+jest.mock('../../../utils/common-utils', () => {
+    const originalModule = jest.requireActual('../../../utils/common-utils');
+    return {
+        __esModule: true,
+        ...originalModule,
+        isLCPSupported: jest.fn().mockReturnValue(true)
+    };
+});
 import { ResourceType } from '../../../utils/common-utils';
 import {
     CLS_EVENT_TYPE,
@@ -237,6 +245,33 @@ describe('WebVitalsPlugin tests', () => {
         // restore
         imageResourceRumEvent.details.fileType = fileType;
     });
+
+    test('when lcp is recorded then cache is empty', async () => {
+        const plugin = new WebVitalsPlugin();
+
+        plugin.load(context);
+        expect(record).toHaveBeenCalledWith(LCP_EVENT_TYPE, expect.anything());
+        expect((plugin as any).resourceEventIds.size).toEqual(0);
+        expect((plugin as any).navigationEventId).toBeUndefined();
+    });
+
+    test('when lcp is not supported then cache is empty', async () => {
+        const plugin = new WebVitalsPlugin();
+        (plugin as any).cacheLCPCandidates = false;
+
+        plugin.load(context);
+        expect(record).toHaveBeenCalledWith(
+            LCP_EVENT_TYPE,
+            expect.objectContaining({
+                attribution: expect.not.objectContaining({
+                    lcpResourceEntry: expect.anything()
+                })
+            })
+        );
+        expect((plugin as any).resourceEventIds.size).toEqual(0);
+        expect((plugin as any).navigationEventId).toBeUndefined();
+    });
+
     test('when lcp is recorded then unsubscribe is called', async () => {
         // init
         const unsubscribeSpy = jest.spyOn(context.eventBus, 'unsubscribe');
diff --git a/src/utils/common-utils.ts b/src/utils/common-utils.ts
index ff2ca370..acb79657 100644
--- a/src/utils/common-utils.ts
+++ b/src/utils/common-utils.ts
@@ -199,3 +199,18 @@ export interface RumLCPAttribution {
     lcpResourceEntry?: string;
     navigationEntry?: string;
 }
+
+/** Checks at runtime if the web vitals package will record LCP
+ * If PerformanceAPI ever changes this API, or if WebVitals package implements a polyfill,
+ * then this needs to be updated
+ *
+ * Reference code from web vitals package:
+ * https://github.com/GoogleChrome/web-vitals/blob/main/src/lib/observe.ts#L46
+ * Discussion for context:
+ * https://github.com/aws-observability/aws-rum-web/pull/448#issuecomment-1734314463
+ */
+export const isLCPSupported = () => {
+    return PerformanceObserver.supportedEntryTypes.includes(
+        'largest-contentful-paint'
+    );
+};