Key connections only on livekit_service_url if it is present

Make drag & drop of floating tiles less janky

docker-compose-playwright.yml

@@ -19,7 +19,7 @@ services:
       - ./backend/playwright_homeserver-othersite.yaml:/data/cfg/homeserver.yaml:Z
   element-web:
     # Pin to a SHA so that upstream cannot break our tests. Renovate handles regular updates.
-    image: ghcr.io/element-hq/element-web:develop@sha256:b2a5a30afb4f583a7560448b005d8332bee29672f15826e4a995b393058f0291
+    image: ghcr.io/element-hq/element-web:develop@sha256:79e47d8d5a93848403cb83042a405d8ed233a7727c499fa53b4b39a80f090be4
   element-web-1:
     # Pin to a SHA so that upstream cannot break our tests. Renovate handles regular updates.
-    image: ghcr.io/element-hq/element-web:develop@sha256:b2a5a30afb4f583a7560448b005d8332bee29672f15826e4a995b393058f0291
+    image: ghcr.io/element-hq/element-web:develop@sha256:79e47d8d5a93848403cb83042a405d8ed233a7727c499fa53b4b39a80f090be4

playwright/widget/test-helpers.ts

@@ -159,7 +159,7 @@ export class TestHelpers {
   public static async dismissStartupToasts(page: Page): Promise<void> {
     const expectedToasts = [
       { title: "Failed to load service worker", button: "OK" },
-      { title: "Back up your chats", button: "Dismiss" },
+      { title: "Back up your chats", button: "Not now" },
       { title: "Turn on key storage", button: "Dismiss" },
       { title: "Element does not support this browser", button: "Dismiss" },
     ];
@@ -178,7 +178,11 @@ export class TestHelpers {
         );
 
         if (toastConfig) {
-          await toast.getByRole("button", { name: toastConfig.button }).click();
+          // Bounded so that a renamed button leaves the toast in place and the
+          // next step reports it, instead of hanging until the test timeout.
+          await toast
+            .getByRole("button", { name: toastConfig.button })
+            .click({ timeout: 5000 });
         } else {
           // Unknown toast. We don't want to act on unknown toasts
           break;

sdk/src/media-backend/livekit/ConnectionMap.test.ts

@@ -207,7 +207,7 @@ describe("connections$ stream", () => {
     });
   });
 
-  test("Keeps one connection per homeserver for transports with a url", () => {
+  test("Keeps one connection per homeserver for transports with a url and no JWT service", () => {
     withTestScheduler(({ behavior, expectObservable }) => {
       const urlTransport: TransportLocator = {
         transport: { type: "livekit", url: "wss://sfu.example.org" },
@@ -243,6 +243,86 @@ describe("connections$ stream", () => {
     });
   });
 
+  test("Shares one connection between a JWT service named with and without its SFU's url", () => {
+    withTestScheduler(({ behavior, expectObservable }) => {
+      const withUrl: TransportLocator = {
+        transport: {
+          type: "livekit",
+          url: "wss://sfu.example.org",
+          livekit_service_url: "https://lk.example.org",
+        },
+        serverName: "example.org",
+      };
+      const connectionMap$ = createConnectionMap$({
+        scope: testScope,
+        connectionFactory: fakeConnectionFactory,
+        localTransport: withUrl,
+        remoteTransports$: behavior("a", {
+          a: new Epoch([TRANSPORT_1], 0),
+        }),
+        logger: logger,
+        ownMembershipIdentity: ownMemberMock,
+      });
+
+      expectObservable(
+        connectionMap$.pipe(mapEpoch((d) => d.getConnections())),
+      ).toBe("a", {
+        a: expect.toSatisfy((e: Epoch<Connection[]>) => {
+          // Both name one service, so one room under one identity: a second
+          // connection would only get the first dropped by the SFU
+          expect(e.value.length).toBe(1);
+          expect(
+            vi.mocked(fakeConnectionFactory).createConnection,
+          ).toHaveBeenCalledExactlyOnceWith(
+            expect.anything(),
+            "publisher",
+            withUrl.transport,
+            "example.org",
+            expect.anything(),
+            expect.anything(),
+          );
+          expect(e.value[0]).toBe(findConnection(TRANSPORT_1, e.value));
+          return true;
+        }),
+      });
+    });
+  });
+
+  test("Subscribes once to a JWT service two members name with and without its SFU's url", () => {
+    withTestScheduler(({ behavior, expectObservable }) => {
+      const withUrl: TransportLocator = {
+        transport: {
+          type: "livekit",
+          url: "wss://sfu.example.org",
+          livekit_service_url: "https://lk.example.org",
+        },
+        serverName: "example.org",
+      };
+      const connectionMap$ = createConnectionMap$({
+        scope: testScope,
+        connectionFactory: fakeConnectionFactory,
+        localTransport: TRANSPORT_2,
+        remoteTransports$: behavior("a", {
+          a: new Epoch([withUrl, TRANSPORT_1], 0),
+        }),
+        logger: logger,
+        ownMembershipIdentity: ownMemberMock,
+      });
+
+      expectObservable(connectionMap$).toBe("a", {
+        a: expect.toSatisfy((e: Epoch<ConnectionMap>) => {
+          const map = e.value;
+          expect(map.getConnections().length).toBe(2);
+          const shared = map.getConnectionForTransport(withUrl);
+          expect(shared).not.toBeNull();
+          expect(shared).not.toBe(map.getConnectionForTransport(TRANSPORT_2));
+          expect(map.getConnectionForTransport(TRANSPORT_1)).toBe(shared);
+          return true;
+        }),
+      });
+    });
+  });
+
   test("Should start connection only once", () => {
     withTestScheduler(({ behavior, expectObservable }) => {
       const connectionMap$ = createConnectionMap$({

sdk/src/media-backend/livekit/ConnectionMap.ts

@@ -27,22 +27,23 @@ export type TransportKey = string & { __transportKey: true };
 
 /**
  * The key a locator is identified by in a map or set. Locators with the same
- * key share a connection: the server name only takes part in authentication
- * for a transport with a `url`, where our homeserver asks that server for the
- * token. A JWT service is reached directly, so members of several homeservers
- * advertising the same one share a connection rather than each get their own.
+ * key share a connection. A JWT service names one SFU room under one
+ * identity, so a transport naming a service is keyed by that alone, whether
+ * or not it also carries the SFU's `url`: a second connection to the same
+ * room would only get the first one dropped by the SFU. Only a transport
+ * with no service is keyed by its `url` and server name, as our homeserver
+ * asks that server for the token.
  */
 export function keyFor({
   transport,
   serverName,
 }: TransportLocator): TransportKey {
-  const hasUrl = "url" in transport;
   // Serialize the fields in a consistent order
-  return JSON.stringify([
-    hasUrl ? transport.url : null,
-    "livekit_service_url" in transport ? transport.livekit_service_url : null,
-    hasUrl ? serverName : null,
-  ]) as TransportKey;
+  return JSON.stringify(
+    "livekit_service_url" in transport
+      ? [null, transport.livekit_service_url, null]
+      : [transport.url, null, serverName],
+  ) as TransportKey;
 }
 
 function removeDuplicateTransports(

sdk/src/media-backend/livekit/LivekitBackend.test.ts

@@ -487,6 +487,22 @@ describe("createLivekitBackend", () => {
       expect(connections).toHaveLength(1);
     });
 
+    it("reads a member naming our JWT service without its url off our own connection", async () => {
+      const b = await prepared();
+      const media = b.mediaFor(testScope(), bob(exampleTransport, [legacyId]));
+      await flushPromises();
+      expect(connections).toHaveLength(1);
+
+      connections[0].participants$.next([
+        mockRemoteParticipant({
+          identity: legacyId,
+          trackPublications: new Map(),
+        }),
+      ]);
+      await flushPromises();
+      expect(media.tracks$.value).toEqual([]);
+    });
+
     it("connects to a remote member's transport on the member's homeserver", async () => {
       const b = await prepared();
       const carlsTransport: UnstableLivekitTransport = {

src/AppBar.module.css

@@ -7,6 +7,7 @@
   padding-top: env(safe-area-inset-top);
   opacity: 1;
   transition: opacity 0.15s;
+  text-align: center;
 }
 
 /* Pseudo-element for the gradient background */
@@ -52,7 +53,7 @@
     ". . ."
     "primaryButton title secondaryButton"
     ". . .";
-  place-items: center;
+  place-items: center stretch;
   column-gap: var(--cpd-space-2x);
 }