Fix: WidgetMessaging not properly closed causing side effects and bugs (#31598)

* test: Add a failing test reproducing the multi messaging leak bug

* fix: Missing widgetApi stop causing leaks

playwright/e2e/voip/element-call.spec.ts

@@ -13,6 +13,7 @@ import { SettingLevel } from "../../../src/settings/SettingLevel";
 import { test, expect } from "../../element-web-test";
 import type { Credentials } from "../../plugins/homeserver";
 import { Bot } from "../../pages/bot";
+import { isDendrite } from "../../plugins/homeserver/dendrite";
 
 // Load a copy of our fake Element Call app, and the latest widget API.
 // The fake call app does *just* enough to convince Element Web that a call is ongoing
@@ -578,4 +579,84 @@ test.describe("Element Call", () => {
             await openAndJoinCall(page, true);
         });
     });
+
+    test.describe("Widget leak bug reproduction", { tag: ["@no-firefox", "@no-webkit"] }, () => {
+        test.skip(isDendrite, "No need to test on other HS, this is a client bug reproduction");
+        test.use({
+            config: {
+                features: {
+                    feature_video_rooms: true,
+                    feature_element_call_video_rooms: true,
+                },
+            },
+        });
+
+        const fakeCallClientSend = readFile("playwright/sample-files/fake-element-call-with-send.html", "utf-8");
+
+        let charlie: Bot;
+        test.use({
+            room: async ({ page, app, user, homeserver, bot }, use) => {
+                charlie = new Bot(page, homeserver, { displayName: "Charlie" });
+                await charlie.prepareClient();
+                const roomId = await app.client.createRoom({
+                    name: "VideoRoom",
+                    invite: [bot.credentials.userId, charlie.credentials.userId],
+                    creation_content: {
+                        type: "org.matrix.msc3417.call",
+                    },
+                });
+                await app.client.createRoom({
+                    name: "OtherRoom",
+                });
+                await use({ roomId });
+            },
+        });
+
+        test.beforeEach(async ({ page, user, app }) => {
+            // use a specific widget to reproduce the bug.
+            // Mock a widget page. We use a fake version of Element Call here.
+            // We should match on things after .html as these widgets get a ton of extra params.
+            await page.route(/\/widget-with-send.html.+/, async (route) => {
+                await route.fulfill({
+                    status: 200,
+                    // Do enough to
+                    body: (await fakeCallClientSend).replace("widgetCodeHere", await widgetApi),
+                });
+            });
+            await app.settings.setValue(
+                "Developer.elementCallUrl",
+                null,
+                SettingLevel.DEVICE,
+                new URL("/widget-with-send.html#", page.url()).toString(),
+            );
+        });
+
+        test("Switching rooms should not leak widgets", async ({ page, user, room, app }) => {
+            await app.viewRoomByName("VideoRoom");
+
+            await expect(page.getByRole("heading", { name: "Approve widget permissions" })).toBeVisible();
+            // approve
+            await page.getByTestId("dialog-primary-button").click();
+
+            // Switch back and forth a few times to trigger the bug.
+
+            await app.viewRoomByName("OtherRoom");
+            await app.viewRoomByName("VideoRoom");
+            await app.viewRoomByName("OtherRoom");
+            await app.viewRoomByName("VideoRoom");
+
+            // For this test we want to display the chat area alongside the widget
+            await page.getByRole("button", { name: "Chat" }).click();
+
+            await page
+                .locator('iframe[title="Element Call"]')
+                .contentFrame()
+                .getByRole("button", { name: "Send Room Message" })
+                .click();
+
+            const messageSent = await page.getByText("I sent this once!!").count();
+
+            expect(messageSent).toBe(1);
+        });
+    });
 });

playwright/sample-files/fake-element-call-with-send.html

@@ -0,0 +1,53 @@
+<!doctype html>
+<style>
+    body {
+        background: rgb(139, 192, 253);
+    }
+</style>
+
+<!-- element-call.spec.ts will insert the widget API in this block -->
+<script>
+    widgetCodeHere;
+</script>
+
+<div>
+    <p>Fake Element Call</p>
+    <p>State: <span id="state">Loading</span></p>
+    <button id="send-button">Send Room Message</button>
+</div>
+
+<!-- Minimal fake implementation of Element Call. Just enough for testing the leagkin widgets.-->
+<script>
+    const stateIndicator = document.querySelector("#state");
+    const { WidgetApi, WidgetApiToWidgetAction, MatrixCapabilities } = mxwidgets();
+    const widgetId = new URLSearchParams(window.location.search).get("widgetId");
+    const params = new URLSearchParams(window.location.hash.slice(1));
+
+    const roomId = params.get("roomId");
+    const api = new WidgetApi(widgetId, "*");
+
+    document.querySelector("#send-button").onclick = async () => {
+        await api.sendRoomEvent(
+            "m.room.message",
+            { msgtype: "m.text", body: "I sent this once!!" },
+            roomId,
+            undefined,
+            undefined,
+            undefined,
+        );
+    };
+
+    api.requestCapability(MatrixCapabilities.AlwaysOnScreen);
+    api.requestCapability(`org.matrix.msc2762.timeline:${roomId}`);
+    api.requestCapabilityToSendMessage("m.text");
+
+    api.on("ready", (ev) => {
+        stateIndicator.innerHTML = "Ready";
+    });
+
+    // Start the messaging
+    api.start();
+
+    // If waitForIframeLoad is false, tell the client that we're good to go
+    api.sendContentLoaded();
+</script>

src/stores/widgets/WidgetMessaging.ts

@@ -508,6 +508,8 @@ export class WidgetMessaging extends TypedEventEmitter<WidgetMessagingEvent, Wid
         }
 
         this.emit(WidgetMessagingEvent.Stop, this.widgetApi);
+        this.widgetApi?.stop();
+        // XXX is the removeAllListeners necessary here?
         this.widgetApi?.removeAllListeners(); // Insurance against resource leaks
         this.widgetApi = null;
         this.iframe = null;