🐛 [RUMF-1449] implement a workaround for Firefox memory leak

packages/core/src/browser/addEventListener.spec.ts

@@ -0,0 +1,37 @@
+import { stubZoneJs } from '../../test/specHelper'
+import { noop } from '../tools/utils'
+
+import { addEventListener, DOM_EVENT } from './addEventListener'
+
+describe('addEventListener', () => {
+  describe('Zone.js support', () => {
+    let zoneJsStub: ReturnType<typeof stubZoneJs>
+
+    beforeEach(() => {
+      zoneJsStub = stubZoneJs()
+    })
+
+    afterEach(() => {
+      zoneJsStub.restore()
+    })
+
+    it('uses the original addEventListener method instead of the method patched by Zone.js', () => {
+      const zoneJsPatchedAddEventListener = jasmine.createSpy()
+      const eventTarget = document.createElement('div')
+      zoneJsStub.replaceProperty(eventTarget, 'addEventListener', zoneJsPatchedAddEventListener)
+
+      addEventListener(eventTarget, DOM_EVENT.CLICK, noop)
+      expect(zoneJsPatchedAddEventListener).not.toHaveBeenCalled()
+    })
+
+    it('uses the original removeEventListener method instead of the method patched by Zone.js', () => {
+      const zoneJsPatchedRemoveEventListener = jasmine.createSpy()
+      const eventTarget = document.createElement('div')
+      zoneJsStub.replaceProperty(eventTarget, 'removeEventListener', zoneJsPatchedRemoveEventListener)
+
+      const { stop } = addEventListener(eventTarget, DOM_EVENT.CLICK, noop)
+      stop()
+      expect(zoneJsPatchedRemoveEventListener).not.toHaveBeenCalled()
+    })
+  })
+})

packages/core/src/browser/addEventListener.ts

@@ -1,4 +1,6 @@
 import { monitor } from '../tools/monitor'
+import { getZoneJsOriginalValue } from '../tools/getZoneJsOriginalValue'
+import { toStackTraceString } from '../tools/error'
 
 export const enum DOM_EVENT {
   BEFORE_UNLOAD = 'beforeunload',
@@ -86,8 +88,14 @@ export function addEventListeners<E extends Event>(
   )
 
   const options = passive ? { capture, passive } : capture
-  events.forEach((event) => eventTarget.addEventListener(event, wrappedListener, options))
-  const stop = () => events.forEach((event) => eventTarget.removeEventListener(event, wrappedListener, options))
+
+  const add = getZoneJsOriginalValue(eventTarget, 'addEventListener')
+  events.forEach((event) => add.call(eventTarget, event, wrappedListener, options))
+
+  function stop() {
+    const remove = getZoneJsOriginalValue(eventTarget, 'removeEventListener')
+    events.forEach((event) => remove.call(eventTarget, event, wrappedListener, options))
+  }
 
   return {
     stop,

packages/core/src/index.ts

@@ -58,6 +58,7 @@ export * from './tools/utils'
 export * from './tools/createEventRateLimiter'
 export * from './tools/browserDetection'
 export { sendToExtension } from './tools/sendToExtension'
+export { getZoneJsOriginalValue } from './tools/getZoneJsOriginalValue'
 export { instrumentMethod, instrumentMethodAndCallOriginal, instrumentSetter } from './tools/instrumentMethod'
 export {
   ErrorSource,

packages/core/src/tools/getZoneJsOriginalValue.spec.ts

@@ -0,0 +1,35 @@
+import { stubZoneJs } from '../../test/specHelper'
+
+import { getZoneJsOriginalValue } from './getZoneJsOriginalValue'
+import { noop } from './utils'
+
+describe('getZoneJsOriginalValue', () => {
+  let zoneJsStub: ReturnType<typeof stubZoneJs> | undefined
+
+  function originalValue() {
+    // just a function that does nothing but different than 'noop' as we'll want to differentiate
+    // them.
+  }
+  const object = {
+    name: originalValue,
+  }
+
+  afterEach(() => {
+    zoneJsStub?.restore()
+  })
+
+  it('returns the original value directly if Zone is not not defined', () => {
+    expect(getZoneJsOriginalValue(object, 'name')).toBe(originalValue)
+  })
+
+  it("returns undefined if Zone is defined but didn't patch that method", () => {
+    zoneJsStub = stubZoneJs()
+    expect(getZoneJsOriginalValue(object, 'name')).toBe(originalValue)
+  })
+
+  it('returns the original value if Zone did patch the method', () => {
+    zoneJsStub = stubZoneJs()
+    zoneJsStub.replaceProperty(object, 'name', noop)
+    expect(getZoneJsOriginalValue(object, 'name')).toBe(originalValue)
+  })
+})

packages/core/src/tools/getZoneJsOriginalValue.ts

@@ -0,0 +1,33 @@
+export interface BrowserWindowWithZoneJs extends Window {
+  Zone?: {
+    __symbol__: (name: string) => string
+  }
+}
+
+/**
+ * Gets the original value for a DOM API that was potentially patched by Zone.js.
+ *
+ * Zone.js[1] is a library that patches a bunch of JS and DOM APIs. It usually stores the original
+ * value of the patched functions/constructors/methods in a hidden property prefixed by
+ * __zone_symbol__.
+ *
+ * In multiple occasions, we observed that Zone.js is the culprit of important issues leading to
+ * browser resource exhaustion (memory leak, high CPU usage). This method is used as a workaround to
+ * use the original DOM API instead of the one patched by Zone.js.
+ *
+ * [1]: https://github.com/angular/angular/tree/main/packages/zone.js
+ */
+export function getZoneJsOriginalValue<Target, Name extends keyof Target & string>(
+  target: Target,
+  name: Name
+): Target[Name] {
+  const browserWindow = window as BrowserWindowWithZoneJs
+  let original: Target[Name] | undefined
+  if (browserWindow.Zone) {
+    original = (target as any)[browserWindow.Zone.__symbol__(name)]
+  }
+  if (!original) {
+    original = target[name]
+  }
+  return original
+}

packages/core/test/specHelper.ts

@@ -4,6 +4,7 @@ import { resetNavigationStart } from '../src/tools/timeUtils'
 import { buildUrl } from '../src/tools/urlPolyfill'
 import { noop, objectEntries, assign } from '../src/tools/utils'
 import type { BrowserWindowWithEventBridge } from '../src/transport'
+import type { BrowserWindowWithZoneJs } from '../src/tools/getZoneJsOriginalValue'
 
 // to simulate different build env behavior
 export interface BuildEnvWindow {
@@ -485,3 +486,31 @@ export function interceptRequests() {
     },
   }
 }
+
+export function stubZoneJs() {
+  const browserWindow = window as BrowserWindowWithZoneJs
+  const restorers: Array<() => void> = []
+
+  function getSymbol(name: string) {
+    return `__zone_symbol__${name}`
+  }
+
+  browserWindow.Zone = { __symbol__: getSymbol }
+
+  return {
+    restore: () => {
+      delete browserWindow.Zone
+      restorers.forEach((restorer) => restorer())
+    },
+    getSymbol,
+    replaceProperty<Target, Name extends keyof Target & string>(target: Target, name: Name, replacement: Target[Name]) {
+      const original = target[name]
+      target[name] = replacement
+      ;(target as any)[getSymbol(name)] = original
+      restorers.push(() => {
+        delete (target as any)[getSymbol(name)]
+        target[name] = original
+      })
+    },
+  }
+}

packages/rum-core/src/browser/domMutationObservable.spec.ts

@@ -1,5 +1,5 @@
 import { isIE } from '@datadog/browser-core'
-import type { BrowserWindow } from './domMutationObservable'
+import { stubZoneJs } from '../../../core/test/specHelper'
 import { createDOMMutationObservable, getMutationObserverConstructor } from './domMutationObservable'
 
 // The MutationObserver invokes its callback in an event loop microtask, making this asynchronous.
@@ -108,38 +108,31 @@ describe('domMutationObservable', () => {
   )
 
   describe('Zone.js support', () => {
-    const browserWindow = window as BrowserWindow
-    const OriginalMutationObserverConstructor = browserWindow.MutationObserver!
+    let zoneJsStub: ReturnType<typeof stubZoneJs>
+    const OriginalMutationObserverConstructor = window.MutationObserver
 
     beforeEach(() => {
-      browserWindow.Zone = { __symbol__: zoneSymbol }
+      zoneJsStub = stubZoneJs()
     })
 
     afterEach(() => {
-      delete browserWindow.Zone
-      delete browserWindow[zoneSymbol('MutationObserver') as any]
-      browserWindow.MutationObserver = OriginalMutationObserverConstructor
+      zoneJsStub.restore()
+      window.MutationObserver = OriginalMutationObserverConstructor
     })
 
     it('gets the original MutationObserver constructor from the "window" object (Zone.js >= 0.8.6)', () => {
-      browserWindow.MutationObserver = function () {
+      zoneJsStub.replaceProperty(window, 'MutationObserver', function () {
         // This won't be instantiated.
-      } as any
-      browserWindow[zoneSymbol('MutationObserver') as any] = OriginalMutationObserverConstructor as any
-
+      } as any)
       expect(getMutationObserverConstructor()).toBe(OriginalMutationObserverConstructor)
     })
 
     it('gets the original MutationObserver constructor from a patched instance (Zone.js < 0.8.6)', () => {
-      browserWindow.MutationObserver = function (this: any, callback: () => void) {
-        this[zoneSymbol('originalInstance')] = new OriginalMutationObserverConstructor(callback)
+      window.MutationObserver = function (this: any, callback: () => void) {
+        this[zoneJsStub.getSymbol('originalInstance')] = new OriginalMutationObserverConstructor(callback)
       } as any
 
       expect(getMutationObserverConstructor()).toBe(OriginalMutationObserverConstructor)
     })
-
-    function zoneSymbol(name: string) {
-      return `__zone_symbol__${name}`
-    }
   })
 })

packages/rum-core/src/browser/domMutationObservable.ts

@@ -1,4 +1,4 @@
-import { monitor, noop, Observable } from '@datadog/browser-core'
+import { monitor, noop, Observable, getZoneJsOriginalValue } from '@datadog/browser-core'
 
 export function createDOMMutationObservable() {
   const MutationObserver = getMutationObserverConstructor()
@@ -21,11 +21,10 @@ export function createDOMMutationObservable() {
 }
 
 type MutationObserverConstructor = new (callback: MutationCallback) => MutationObserver
+
 export interface BrowserWindow extends Window {
   MutationObserver?: MutationObserverConstructor
-  Zone?: {
-    __symbol__: (name: string) => string
-  }
+  Zone?: unknown
 }
 
 export function getMutationObserverConstructor(): MutationObserverConstructor | undefined {
@@ -44,28 +43,24 @@ export function getMutationObserverConstructor(): MutationObserverConstructor |
   // [1] https://github.com/angular/angular/issues/26948
   // [2] https://github.com/angular/angular/issues/31712
   if (browserWindow.Zone) {
-    const zoneSymbol = browserWindow.Zone.__symbol__
-
     // Zone.js 0.8.6+ is storing original class constructors into the browser 'window' object[3].
     //
     // [3] https://github.com/angular/angular/blob/6375fa79875c0fe7b815efc45940a6e6f5c9c9eb/packages/zone.js/lib/common/utils.ts#L288
-    constructor = browserWindow[zoneSymbol('MutationObserver') as any] as unknown as
-      | MutationObserverConstructor
-      | undefined
+    constructor = getZoneJsOriginalValue(browserWindow, 'MutationObserver')
 
-    if (!constructor && browserWindow.MutationObserver) {
+    if (browserWindow.MutationObserver && constructor === browserWindow.MutationObserver) {
       // Anterior Zone.js versions (used in Angular 2) does not expose the original MutationObserver
       // in the 'window' object. Luckily, the patched MutationObserver class is storing an original
       // instance in its properties[4]. Let's get the original MutationObserver constructor from
       // there.
       //
       // [4] https://github.com/angular/zone.js/blob/v0.8.5/lib/common/utils.ts#L412
 
-      const patchedInstance = new browserWindow.MutationObserver(noop) as any
-      const originalInstance = patchedInstance[zoneSymbol('originalInstance')] as
-        | { constructor: MutationObserverConstructor }
-        | undefined
+      const patchedInstance = new browserWindow.MutationObserver(noop) as {
+        originalInstance?: { constructor: MutationObserverConstructor }
+      }
 
+      const originalInstance = getZoneJsOriginalValue(patchedInstance, 'originalInstance')
       constructor = originalInstance && originalInstance.constructor
     }
   }