[MM-68740] Guard loading screen fade against undefined webContents

[nickmisasi]

Summary

Fixes an unhandled TypeError thrown from LoadingScreen.fade() when Electron has already torn down the loading screen's webContents.

Electron can leave WebContentsView.webContents undefined during teardown. fade() called isDestroyed() directly on it, so a fade triggered by setInitialized after teardown threw. The change caches the webContents and guards on existence as well as destroyed-ness before sending:

const webContents = this.view.webContents;
if (webContents && !webContents.isDestroyed()) {
    webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, false);
}

The this.state = LoadingScreenState.FADING transition is deliberately left where it was, ahead of the guard, so handleAnimationFinished and the state machine behave exactly as before. Scope is confined to fade().

Why the reported signature differs from what reproduces today

Sentry MATTERMOST-DESKTOP-1S reports Cannot read properties of undefined (reading 'send'), with culprit m.fade reached via IpcMainImpl → setInitialized → fadeLoadingScreen → fade. Every one of its 2,009 events comes from released 6.1.1–6.2.2 builds, where fade() called send() with no guard at all.

e6994098 (MM-68749, #3822) later wrapped that send() in an isDestroyed() check, which fixed the send variant but still assumed webContents exists. So on master the same teardown race now throws Cannot read properties of undefined (reading 'isDestroyed') instead — the signature tracked as MM-68927. Reverting loadingScreen.ts and running the new tests reproduces exactly that:

✕ should not throw when webContents is undefined during teardown
✕ should still leave the visible state when webContents is undefined
  TypeError: Cannot read properties of undefined (reading 'isDestroyed')
Tests: 2 failed, 5 passed, 7 total

Both pass with the fix applied.

Sibling this.view.webContents dereferences in show(), destroy(), and registerThemeManager() carry the same latent hazard but are intentionally left alone to keep this change minimal.

Ticket Link

https://mattermost.atlassian.net/browse/MM-68740

Checklist

• Added or updated unit tests (required for all new features)
• Has UI changes
• executed npm run lint:js for proper code formatting

Device Information

Verified on Linux (x64) via the unit suite only. npm run check is green: ESLint and tsc clean, Test Suites: 79 passed, 79 total, Tests: 1317 passed, 1317 total.

This is a teardown race with no practical manual reproduction, so correctness is demonstrated by the two new regression tests, which fail against the pre-fix code with the exact production crash and pass after it, rather than by manual GUI testing.

Release Note

Fixed a crash that could occur when the loading screen faded after its web contents had already been torn down.

  

Change Impact: 🟢 Low

Regression Risk: Low; the change is isolated to loading-screen teardown handling and is covered by regression tests.
QA Recommendation: Manual QA can be skipped; automated coverage is sufficient.

Generated by CodeRabbitAI

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 99c3e389-819d-451e-95c8-6891c29611c6

📥 Commits

Reviewing files that changed from the base of the PR and between dd4f35b and 608e837.

📒 Files selected for processing (2)

• src/app/views/loadingScreen.test.js
• src/app/views/loadingScreen.ts

---

📝 Walkthrough

Walkthrough

Changes

The loading screen fade path now safely handles missing or destroyed webContents before sending the visibility IPC message. Tests cover valid, destroyed, undefined, teardown, and repeated fade scenarios.

Loading screen fade lifecycle

Layer / File(s)	Summary
Guard fade IPC dispatch src/app/views/loadingScreen.ts, src/app/views/loadingScreen.test.js	LoadingScreen.fade checks captured webContents availability and destruction state before dispatching TOGGLE_LOADING_SCREEN_VISIBILITY; tests validate the supported lifecycle conditions.

Estimated code review effort: 2 (Simple) | ~10 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: guarding LoadingScreen.fade() against undefined webContents.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/mm-68740-loading-screen-fade-guard-26e4

---

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

[nickmisasi]

Merging as the playwright failures are unrelated