Add test coverage for event listener leaking (#455)

* Add test coverage for event listener leaking

Add test coverage to [#454][]. While the original fix was
important to ship, it was unclear how to guard against re-introducing
the leaks.

This commit introduces the `withoutChangingEventListenersCount()` test
helper to wrap a block within the test harness. While executing within
the block, the driven page's `document.addEventListener` and
`document.removeEventListener` functions are replaced with decorated
functions that increment and decrement a counter. At the start of the
block, capture and return how many times `addEventListener` has been
invoked without a matching `removeEventListener`. At the end, capture
and return the differential.

[#454]: #454

* Move the `disconnectedCallback()` call

The fix introduced in [#454][] resolved the issue, but
introduced a matching `disconnectedCallback()` far-off in the
`FrameRenderer`.

This commit moves that call to occur _immediately_ after it's paired
`element.connectedCallback()`.

[#454]: #454

Co-authored-by: David Heinemeier Hansson <[email protected]>

src/core/frames/frame_controller.ts

@@ -432,6 +432,7 @@ function activateElement(element: Element | null, currentURL?: string | null) {
 
     if (element instanceof FrameElement) {
       element.connectedCallback()
+      element.disconnectedCallback()
       return element
     }
   }

src/core/frames/frame_renderer.ts

@@ -29,7 +29,6 @@ export class FrameRenderer extends Renderer<FrameElement> {
     if (sourceRange) {
       sourceRange.selectNodeContents(frameElement)
       this.currentElement.appendChild(sourceRange.extractContents())
-      frameElement.disconnectedCallback()
     }
   }
 

src/tests/functional/frame_tests.ts

@@ -5,6 +5,17 @@ export class FrameTests extends TurboDriveTestCase {
     await this.goToLocation("/src/tests/fixtures/frames.html")
   }
 
+  async "test navigating a frame a second time does not leak event listeners"() {
+    await this.withoutChangingEventListenersCount(async () => {
+      await this.clickSelector("#outer-frame-link")
+      await this.nextEventOnTarget("frame", "turbo:frame-load")
+      await this.clickSelector("#outside-frame-form")
+      await this.nextEventOnTarget("frame", "turbo:frame-load")
+      await this.clickSelector("#outer-frame-link")
+      await this.nextEventOnTarget("frame", "turbo:frame-load")
+    })
+  }
+
   async "test following a link preserves the current <turbo-frame> element's attributes"() {
     const currentPath = await this.pathname
 
@@ -522,6 +533,47 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-response"))
   }
 
+  async withoutChangingEventListenersCount(callback: () => void) {
+    const name = "eventListenersAttachedToDocument"
+    const setup = () => {
+      return this.evaluate((name: string) => {
+        const context = window as any
+        context[name] = 0
+        context.originals = { addEventListener: document.addEventListener, removeEventListener: document.removeEventListener }
+
+        document.addEventListener = (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) => {
+          context.originals.addEventListener.call(document, type, listener, options)
+          context[name] += 1
+        }
+
+        document.removeEventListener = (type: string, listener: EventListenerOrEventListenerObject, options?: boolean | AddEventListenerOptions) => {
+          context.originals.removeEventListener.call(document, type, listener, options)
+          context[name] -= 1
+        }
+
+        return context[name] || 0
+      }, [name])
+    }
+
+    const teardown = () => {
+      return this.evaluate((name: string) => {
+        const context = window as any
+        const { addEventListener, removeEventListener } = context.originals
+
+        document.addEventListener = addEventListener
+        document.removeEventListener = removeEventListener
+
+        return context[name] || 0
+      }, [name])
+    }
+
+    const originalCount = await setup()
+    await callback()
+    const finalCount = await teardown()
+
+    this.assert.equal(finalCount, originalCount, "expected callback not to leak event listeners")
+  }
+
   async fillInSelector(selector: string, value: string) {
     const element = await this.querySelector(selector)