fix: share main webview session with internal video chat window by rodrigok · Pull Request #3359 · RocketChat/Rocket.Chat.Electron

rodrigok · 2026-06-18T21:19:26Z

Problem

Internal conferences opened via openInternalVideoChatWindow ran in a webview hardcoded to the isolated persist:jitsi-session partition, so they did not share cookies or localStorage with the server webview they were opened from. Electron scopes cookies/localStorage to the (partition, origin) pair, so a same-origin conference loaded unauthenticated (the login token lives in localStorage under the server origin).

Fix

Load the call webview in the originating server's partition (persist:<serverUrl>, resolved from the caller webContents) so the call shares the main webview's session.

Because session-level handlers are per-session, sharing the session means the call window's handlers would otherwise clobber the main webview's:

Screen sharing now uses a single unified display-media handler that routes by originating frame: in-call requests open the picker in the call window; main-app requests fall back to the server-view picker. The plain server-view handler is restored when the call closes.
Permission handler teardown reset is no longer applied to the live main webview on a shared session.

Production hardening

The above started as a proof of concept. The following commit makes it correct for all users, covering the lifecycle edge cases that arise once the call and the main webview share one session.

Per-window state instead of a mutable global

The call's partition was tracked in a module-level global read at several lifecycle points that span async ticks (cleanup, screen-share routing, teardown restore). A second call opening could mutate it mid-flight. State is now captured per window in an ActiveCall record ({ partition, isSharedSession, serverWebContentsId }); every lifecycle decision reads the per-window value. The global is narrowed to renderer-handshake reads only (the loadFile query payload and the request-url IPC response).

Handler restore on every teardown path

The main webview's screen-share handler was restored only on the graceful closed event. A renderer crash or force-kill that skipped closed left the main app's screen sharing — and, on a shared session, its permission prompts — disabled until the next call opened.

Restore is now idempotent and fires from every teardown path: closed, cleanupVideoCallWindow, and a new render-process-gone listener. It restores both the server-view display-media handler and the permission handler (the permission handler was extracted into an exported setupServerViewPermissionHandler so the install site and the restore site share one source of truth). Fallback (isolated) sessions are skipped — they have nothing to restore.

Screen-share routing evaluated at request time

The shared-session routing flag was snapshotted when the webview attached but consumed later when a display-media request fired, so it could be stale. The per-window record is now snapshotted at attach and the routing branch is evaluated at request time; the call-window identity check stays live.

State null-out guarded by identity

A previous window's teardown nulls module state ~50 ms after close. With a new call already open, that could wipe the new call's record. Terminal teardowns now null only their own state (if (activeCall === capturedCall) activeCall = null).

Open-window race serialized

The open handler is async with multiple await points; two near-simultaneous opens could both pass the destruction/existing-window guards and both reach new BrowserWindow, orphaning the first window with leaked handlers on the shared session. The handler is now serialized through a promise-chain mutex, so each open runs to completion before the next begins. The existing wait/close logic stays as a second line of defense.

Unresolved-server fallback

When the originating server can't be resolved, the call now falls back to persist:jitsi-session (stable isolated storage) instead of the window-default session, and isSharedSession is false so none of the shared-session restore logic engages.

Tests

Added src/videoCallWindow/main/ipc.main.spec.ts covering: shared vs fallback partition assignment, handler restore across all teardown paths (closed / render-process-gone / cleanup), restore idempotency, the === capturedCall null-out guard, and the open-window serialization race. Full suite: 293 passing, 0 regressions; tsc and lint clean.

Files changed

src/videoCallWindow/ipc.ts — per-window state, idempotent restore from all teardown paths, request-time routing, null-out guards, open-handler serialization.
src/ui/main/serverView/index.ts — extracted/exported setupServerViewPermissionHandler (shared by the install site and the restore path).
src/videoCallWindow/video-call-window.ts — receives the partition from the main process instead of hardcoding it.
src/ipc/channels.ts — partition added to the request-url response type.
src/screenSharing/serverViewScreenSharing.ts — handleServerViewDisplayMediaRequest for routing main-app requests back to the server-view picker.
src/videoCallWindow/main/ipc.main.spec.ts — new regression tests.

Manual test checklist

Internal video conference loads authenticated.
Screen-share inside the call → call-window picker.
Screen-share from the main app after opening and after closing a call → works (restore).
Force-crash the call renderer → main-app screen share still works.
Quit app mid-call → clean teardown.
Rapid double-open (spam join) → exactly one live window, no orphan.

Summary by CodeRabbit

New Features
- Added in-app navigation support to open video call routes in the main window, with stricter route validation.
- Added route callback wiring from the main process into the web app, including buffered delivery for early events.
Bug Fixes
- Improved DevTools behavior to target the currently focused window when available.
- Strengthened shared-session handling and server-view media/permission behavior during video calls, screen sharing, and window restore/reopen flows.
- Enhanced external link and popup handling for conference webviews.

Internal conferences opened via openInternalVideoChatWindow ran in a webview hardcoded to the isolated `persist:jitsi-session` partition, so they did not share cookies or localStorage with the server webview they were opened from. Electron scopes cookies/localStorage to the (partition, origin) pair, so a same-origin conference loaded unauthenticated (the login token lives in localStorage under the server origin). Load the call webview in the originating server's partition (`persist:<serverUrl>`, resolved from the caller webContents) so the call shares the main webview's session. Because session-level handlers are per-session, sharing the session means the call window's handlers would otherwise clobber the main webview's: - Screen sharing now uses a single unified display-media handler that routes by originating frame (in-call requests open the picker in the call window; main-app requests fall back to the server-view picker), and the plain server-view handler is restored when the call closes. - The teardown permission-handler reset is skipped on a shared session so it can't disable permissions on the live main webview. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-06-18T21:19:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c18d8a43-6ee8-4bd8-96fe-564b7c6e6cc6

📥 Commits

Reviewing files that changed from the base of the PR and between 6db25a2 and 5e6f239.

📒 Files selected for processing (4)

src/ui/main/serverView/index.ts
src/videoCallWindow/ipc.ts
src/videoCallWindow/main/ipc.main.spec.ts
src/videoCallWindow/preload/index.ts

🚧 Files skipped from review as they are similar to previous changes (4)

src/ui/main/serverView/index.ts
src/videoCallWindow/preload/index.ts
src/videoCallWindow/main/ipc.main.spec.ts
src/videoCallWindow/ipc.ts

📜 Recent review details

⏰ Context from checks skipped due to timeout. (4)

GitHub Check: build (windows-latest, windows)
GitHub Check: build (ubuntu-latest, linux)
GitHub Check: check (ubuntu-latest)
GitHub Check: check (windows-latest)

Walkthrough

Adds a shared-session partition flow from the main process to the video call window webview, a buffered navigate-to-route IPC bridge, extracted server-view permission handling, unified display-media routing, and video call window lifecycle hardening. Also adds route-sanitized open-in-main-window handling and Jest coverage.

Changes

Video Call Window Shared-Session Hardening

Layer / File(s)	Summary
IPC contracts and navigate-to-route bridge `src/ipc/channels.ts`, `src/preload.ts`, `src/servers/preload/api.ts`, `src/servers/preload/navigateToRoute.ts`, `src/servers/preload/navigateToRoute.spec.ts`	Adds `video-call-window/open-in-main-window` and expands `video-call-window/request-url` with optional `partition`. Introduces buffered `navigateToRoute.ts`, wires it into preload startup, exposes `onNavigateToRoute` on `window.RocketChatDesktop`, and adds spec coverage for listener and buffering behavior.
Server-view permission handler refactor and display-media export `src/ui/main/serverView/index.ts`, `src/screenSharing/serverViewScreenSharing.ts`	Extracts permission handling into `setupServerViewPermissionHandler` and adds `handleServerViewDisplayMediaRequest` with provider-ready dispatch and `{ video: false }` fallback behavior.
Partition propagation from main process to webview `src/videoCallWindow/ipc.ts`, `src/videoCallWindow/video-call-window.ts`	Carries `partition` through `pendingVideoCallPartition`, the `video-call-window/request-url` response, `window.location.search`, and `createWebview(url, partition)`.
Video call window lifecycle hardening `src/videoCallWindow/ipc.ts`	Adds `restoreServerViewHandler`, shared-session display-media routing, popup and navigation policy for attached webviews, shared-session-aware teardown, and partition-aware `openVideoCallWindow` behavior.
open-in-main-window IPC and route sanitization `src/videoCallWindow/preload/index.ts`, `src/videoCallWindow/ipc.ts`, `src/ui/main/menuBar.ts`	Rejects non-relative routes before IPC, routes `open-in-main-window` to the correct server webview, and changes DevTools targeting to prefer the focused window.
IPC and preload test suites `src/videoCallWindow/main/ipc.main.spec.ts`	Adds coverage for partition propagation, restore flows, null-out guard, serialization, open-in-main-window, close handling, reopen behavior, and webview link/permission policy.

Sequence Diagram(s)

sequenceDiagram
  participant MainProcess
  participant BrowserWindow
  participant videoCallWindowPreload
  participant ServerWebContents

  MainProcess->>BrowserWindow: create video call window with partition query
  BrowserWindow->>videoCallWindowPreload: load window.location.search
  videoCallWindowPreload->>MainProcess: video-call-window/request-url
  MainProcess-->>videoCallWindowPreload: url + partition
  videoCallWindowPreload->>BrowserWindow: createWebview(url, partition)
  BrowserWindow->>ServerWebContents: open-in-main-window(path)
  ServerWebContents->>ServerWebContents: navigate-to-route(path)

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: sharing the main webview session with the internal video chat window.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Make the POC session-share fix correct for all users. - Thread per-window state (ActiveCall record) instead of reading the mutable pendingVideoCallPartition global at lifecycle points that span async ticks; narrow the global to renderer-handshake reads only. - Restore the server-view display-media AND permission handlers from every teardown path (closed, cleanup, render-process-gone) so a shared session can't leave the main webview's screen sharing or permission prompts disabled after a call ends. Restore is idempotent. - Fix isSharedSession staleness: snapshot the per-window record at webview attach and evaluate routing at display-media request time. - Guard terminal state null-out with identity (=== capturedCall) so a stale prior-window teardown can't wipe a newer call's state. - Serialize the open-window handler via a promise-chain mutex to close a rapid-double-open race that orphaned a window with leaked handlers. - Retain persist:jitsi-session as the unresolved-server fallback so such calls keep stable isolated storage. Add ipc.main.spec.ts covering restore (shared/fallback/destroyed/ idempotent), partition assignment, render-process-gone, the null-out guard, and the serialization race.

github-actions · 2026-06-18T23:41:16Z

Linux installer download

github-actions · 2026-06-18T23:47:04Z

Windows installer download

github-actions · 2026-06-19T00:29:13Z

macOS installer download

…deo call window The standalone internal video-chat window is its own BrowserWindow with no window.opener, so the web app's window.open/opener trick can't reach the main window — it just spawns another window. Add an IPC path so the conference page can ask the main app window to focus itself and navigate to an in-app route. Caller (video-chat window): - window.videoCallWindow.openInMainWindow(path) — validates that path is an in-app relative route ("/..."), rejecting absolute/protocol-relative/scheme URLs, then invokes 'video-call-window/open-in-main-window'. Main process: - New handler resolves the target server webview (caller's own server, else the active currentView.url), shows/restores/focuses the main window, and emits 'navigate-to-route' (payload: path) to that server webview's webContents. It intentionally does NOT loadURL — that would hard-reload the SPA. No-ops safely with a warning when no window/webview is found, and re-validates the path. Receiver (server webview is contextIsolated, so a raw send lands in the preload, not the page): - New navigateToRoute preload relay listens on 'navigate-to-route' and forwards to a RocketChatDesktop.onNavigateToRoute(callback) the web client registers, buffering the latest path if it arrives before registration (mirrors the telephony relay). Also: Cmd/Ctrl+Shift+D ("Toggle Developer Tools") now targets the focused window (falling back to the main window), so it can open DevTools for the video call window instead of always the main one. Web-repo follow-up (out of scope here): the web client must call RocketChatDesktop.onNavigateToRoute(path => router.navigate(path)) for the route change to take effect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

When the standalone video call window asks the main window to navigate, the handler resolved the target server from the caller webContents and fell back to whichever server was active in the main window. In a multi-workspace setup that could navigate a *different* server than the one the call belongs to. Resolve in priority order: caller's own server, then the active call's origin server (authoritative, via activeCall.serverWebContentsId), then the active view as a last-resort guess (now logged as ambiguous). Also log in the preload bridge when a non-relative path is rejected, for parity with the main-process handler. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The internal video-chat window is a BrowserWindow created by the main process, so the renderer's own window.close() can't close it. Expose a close() method on the window.videoCallWindow bridge that asks the main process to do it. - Preload: close: () => ipcRenderer.send('video-call-window/close') (no payload). - Main: ipcMain.on('video-call-window/close') resolves the window from the sender (with a hostWebContents fallback for the webview-guest sender) and calls win.close() when it's live. Resolving from the sender means a renderer can only close its own window. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…ference Clicking "join" again from the main window while the video call window was already open tore the window down and recreated it. When the requested conference URL matches the one already open, focus the existing window instead (restore if minimized, show, focus) and return early, leaving activeCall, provider, credentials and partition untouched. A different URL still closes + recreates as before. Track the conference URL on activeCall so the decision uses lifecycle state rather than the renderer-handshake globals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…wser The internal video-chat window didn't route external links to the system browser like the main window does, so target="_blank" / window.open links from the conference chat (which run in the webview guest, whose setWindowOpenHandler was unset) spawned a new Electron window instead. Set the guest webview's window-open handler on attach: http(s) popups return { action: 'deny' } and open via the system browser (openExternal), smb:// is denied, anything else stays in-app. Also add a will-navigate handler that sends external-scheme target="_self" navigations (mailto:, tel:, custom schemes) to the browser, while leaving http(s) self-navigations in the webview so the conference's own flows (auth redirects, etc.) keep working. Extract the shared deny/openExternal policy into a helper reused by the host window's existing handler so host and guest behave identically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (1)

src/videoCallWindow/preload/index.ts (1)
17-26: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Optional: swallow the fire-and-forget invoke rejection.

ipcRenderer.invoke(...) returns a promise that is neither awaited nor caught. If the main handler ever rejects, this surfaces as an unhandled promise rejection in the renderer. The main-side handler currently resolves to undefined on every path, so this is benign today; a .catch makes it robust against future handler changes.
♻️ Optional hardening
   openInMainWindow: (path: string) => {
     if (isRelativeRoute(path)) {
-      ipcRenderer.invoke('video-call-window/open-in-main-window', path);
+      ipcRenderer
+        .invoke('video-call-window/open-in-main-window', path)
+        .catch((error) =>
+          console.warn('Video call window: open-in-main-window failed:', error)
+        );
       return;
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/videoCallWindow/preload/index.ts` around lines 17 - 26, The
fire-and-forget ipcRenderer.invoke call in openInMainWindow is not handled, so
any future rejection could become an unhandled promise rejection in the
renderer. Update the openInMainWindow method in the preload bridge to explicitly
swallow or catch the invoke promise when isRelativeRoute(path) passes, while
keeping the existing warning for non-relative paths, so the behavior stays safe
even if the main-side video-call-window/open-in-main-window handler changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/ui/main/serverView/index.ts`:
- Around line 132-140: The media branch in serverView’s permission handler can
leave requests hanging because `handleMediaPermissionRequest` is awaited without
error handling. Update the `switch` case for `'media'` in
`src/ui/main/serverView/index.ts` to mirror the video-call handling by catching
rejections from `handleMediaPermissionRequest` and always invoking the
permission callback with a deny/failure result when an error occurs.

In `@src/videoCallWindow/ipc.ts`:
- Around line 447-458: The call state is being mutated too early in the IPC
flow, before URL validation and the external-only guard in the call creation
path. In the call window setup logic around
activeCall/pendingVideoCallPartition, move those assignments to after new
URL(url) succeeds and after the g.co early return has been handled, using the
existing call creation path and partition resolution symbols to keep stale state
from being set when no window is created.
- Around line 264-276: The popup handling in handleVideoCallWindowOpen is too
permissive because it allows any non-http(s) URL, which can open unsafe Electron
windows for file:, data:, javascript:, and custom schemes. Update the logic to
keep the policy closed: continue routing http:// and https:// through
openExternal, keep denying smb://, and change the final fallback to deny unknown
protocols unless a specific internal scheme is explicitly required. Use the
handleVideoCallWindowOpen symbol to locate and tighten the decision branch.
- Around line 901-952: The permission handler is attached to the host window
session in the video call IPC flow, but the media request comes from the
conference webview’s separate partition. Update the setup around the permission
request handler in ipc.ts so it is registered on the webview’s actual session,
using the partition-backed session from
pendingVideoCallPartition/session.fromPartition(...) rather than
videoCallWindow.webContents.session, and keep the existing
handleMediaPermissionRequest path intact.

---

Nitpick comments:
In `@src/videoCallWindow/preload/index.ts`:
- Around line 17-26: The fire-and-forget ipcRenderer.invoke call in
openInMainWindow is not handled, so any future rejection could become an
unhandled promise rejection in the renderer. Update the openInMainWindow method
in the preload bridge to explicitly swallow or catch the invoke promise when
isRelativeRoute(path) passes, while keeping the existing warning for
non-relative paths, so the behavior stays safe even if the main-side
video-call-window/open-in-main-window handler changes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d21e4dd6-18e4-4ba4-8b27-8511d489e397

📥 Commits

Reviewing files that changed from the base of the PR and between 4856b6b and 6db25a2.

📒 Files selected for processing (12)

src/ipc/channels.ts
src/preload.ts
src/screenSharing/serverViewScreenSharing.ts
src/servers/preload/api.ts
src/servers/preload/navigateToRoute.spec.ts
src/servers/preload/navigateToRoute.ts
src/ui/main/menuBar.ts
src/ui/main/serverView/index.ts
src/videoCallWindow/ipc.ts
src/videoCallWindow/main/ipc.main.spec.ts
src/videoCallWindow/preload/index.ts
src/videoCallWindow/video-call-window.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (6)

**/*.ts