From 1d5172d9e472b94fa2c583911eaec17ba13f2cfc Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Mon, 14 Sep 2026 13:45:43 +0200 Subject: [PATCH 1/2] fix(spell-check): stop service views crashing while typing Since Electron 44 (Chromium 152), Blink cancels an in-flight spell check when a script moves the caret in a field the user didn't focus, as Slack and Telegram composers do. The late answer to the cancelled request completes the newer one in Electron's SpellCheckClient, whose own answer then dereferences a null pending request and segfaults the renderer, leaving a blank service view. Answer only the most recent request, and only once the renderer is idle, so a request Electron has accepted but not yet handed over always supersedes a stale answer before it is delivered. Signed-off-by: Marc Nuri --- .../__tests__/preload.spell-check.test.js | 165 +++++++++++++----- src/service-manager/preload.spell-check.js | 36 +++- 2 files changed, 152 insertions(+), 49 deletions(-) diff --git a/src/service-manager/__tests__/preload.spell-check.test.js b/src/service-manager/__tests__/preload.spell-check.test.js index 67324b78..76b8c7e7 100644 --- a/src/service-manager/__tests__/preload.spell-check.test.js +++ b/src/service-manager/__tests__/preload.spell-check.test.js @@ -19,9 +19,18 @@ describe('Browser Spell Check test suite', () => { let electron; let settings; let browserSpellCheck; + let idleCallbacks; + const runIdleCallbacks = () => { + for (const idleCallback of idleCallbacks.splice(0)) { + idleCallback(); + } + }; beforeEach(async () => { jest.resetModules(); globalThis.APP_EVENTS = require('../../constants').APP_EVENTS; + // jsdom has no requestIdleCallback: queue the callbacks so each test decides when the renderer is idle + idleCallbacks = []; + globalThis.requestIdleCallback = idleCallback => idleCallbacks.push(idleCallback); electron = require('../../__tests__').testElectron(); settings = await require('../../__tests__').testSettings(); electron.ipcMain.on('settingsLoad', settings.loadSettings); @@ -29,6 +38,9 @@ describe('Browser Spell Check test suite', () => { // Set the browser language to Esperanto Object.defineProperty(navigator, 'language', {value: 'eo'}); }); + afterEach(() => { + delete globalThis.requestIdleCallback; + }); describe('initSpellChecker', () => { test('not-native, should load settings and set SpellCheckProvider in webFrame for navigator language', async () => { // Given @@ -62,49 +74,118 @@ describe('Browser Spell Check test suite', () => { expect(electron.ipcRenderer.invoke).toHaveBeenCalledWith('settingsLoad'); }); }); - test('spellCheck, should invoke dictionaryGetMisspelled and trigger callback', async () => { - // Given - const dictionaryGetMisspelled = jest.fn(); - electron.ipcMain.on('dictionaryGetMisspelled', dictionaryGetMisspelled); - browserSpellCheck.initSpellChecker(); - await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1)); - const spellCheckCallback = jest.fn(); - // When - await electron.webFrame.spellCheckProviders.eo.spellCheck([], spellCheckCallback); - // Then - expect(dictionaryGetMisspelled).toHaveBeenCalledWith([]); - expect(spellCheckCallback).toHaveBeenCalledTimes(1); - }); - test('spellCheck, should trigger callback even if dictionaryGetMisspelled fails', async () => { - // Given - browserSpellCheck.initSpellChecker(); - await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1)); - electron.ipcRenderer.invoke = jest.fn(async () => { - throw new Error('Script failed to execute'); + describe('spellCheck', () => { + let dictionaryGetMisspelled; + let spellCheck; + beforeEach(async () => { + dictionaryGetMisspelled = jest.fn(async words => words.filter(word => word.startsWith('mis'))); + electron.ipcMain.handle('dictionaryGetMisspelled', dictionaryGetMisspelled); + browserSpellCheck.initSpellChecker(); + await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1)); + ({spellCheck} = electron.webFrame.spellCheckProviders.eo); }); - const spellCheckCallback = jest.fn(); - // When - await electron.webFrame.spellCheckProviders.eo.spellCheck(['word'], spellCheckCallback); - // Then - // Blink leaves the request pending forever if the callback never runs - await waitFor(() => expect(spellCheckCallback).toHaveBeenCalledWith([])); - }); - test('spellCheck, should answer a request only once even if the callback throws', async () => { - // Given - const dictionaryGetMisspelled = jest.fn(); - electron.ipcMain.on('dictionaryGetMisspelled', dictionaryGetMisspelled); - browserSpellCheck.initSpellChecker(); - await waitFor(() => expect(electron.webFrame.setSpellCheckProvider).toHaveBeenCalledTimes(1)); - const consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); - const spellCheckCallback = jest.fn(() => { - throw new Error('Blink blew up'); + describe('with a single request', () => { + let callback; + beforeEach(async () => { + callback = jest.fn(); + spellCheck(['correct', 'misspelled'], callback); + await waitFor(() => expect(idleCallbacks).toHaveLength(1)); + }); + test('sends the words to the dictionary', () => { + expect(dictionaryGetMisspelled).toHaveBeenCalledWith(['correct', 'misspelled']); + }); + test('does not answer before the renderer is idle', () => { + expect(callback).not.toHaveBeenCalled(); + }); + test('answers with the misspelled words once the renderer is idle', () => { + runIdleCallbacks(); + expect(callback).toHaveBeenCalledExactlyOnceWith(['misspelled']); + }); + }); + // Answering a superseded request crashes the renderer: Electron completes the newer request with + // it, and the newer request's own answer then finds nothing pending + describe('with a request superseded while its dictionary lookup is in flight', () => { + let supersededCallback; + let newerCallback; + beforeEach(async () => { + let resolveSuperseded; + dictionaryGetMisspelled.mockImplementationOnce(() => new Promise(resolve => { + resolveSuperseded = resolve; + })); + supersededCallback = jest.fn(); + newerCallback = jest.fn(); + spellCheck(['misspelled'], supersededCallback); + spellCheck(['correct', 'mistake'], newerCallback); + await waitFor(() => expect(idleCallbacks).toHaveLength(1)); + resolveSuperseded(['misspelled']); + await waitFor(() => expect(idleCallbacks).toHaveLength(2)); + runIdleCallbacks(); + }); + test('never answers the superseded request', () => { + expect(supersededCallback).not.toHaveBeenCalled(); + }); + test('answers the newer request', () => { + expect(newerCallback).toHaveBeenCalledExactlyOnceWith(['mistake']); + }); + }); + // Electron posts a task to hand a request over, so Blink may have issued a newer request while the + // answer to the previous one is waiting for the renderer to be idle + describe('with a request superseded while its answer waits for the renderer to be idle', () => { + let supersededCallback; + let newerCallback; + beforeEach(async () => { + supersededCallback = jest.fn(); + newerCallback = jest.fn(); + spellCheck(['misspelled'], supersededCallback); + await waitFor(() => expect(idleCallbacks).toHaveLength(1)); + spellCheck(['correct', 'mistake'], newerCallback); + await waitFor(() => expect(idleCallbacks).toHaveLength(2)); + runIdleCallbacks(); + }); + test('never answers the superseded request', () => { + expect(supersededCallback).not.toHaveBeenCalled(); + }); + test('answers the newer request', () => { + expect(newerCallback).toHaveBeenCalledExactlyOnceWith(['mistake']); + }); + }); + describe('with a dictionary that fails', () => { + let callback; + beforeEach(async () => { + dictionaryGetMisspelled.mockImplementation(async () => { + throw new Error('Script failed to execute'); + }); + callback = jest.fn(); + spellCheck(['misspelled'], callback); + await waitFor(() => expect(idleCallbacks).toHaveLength(1)); + runIdleCallbacks(); + }); + // Blink leaves the request pending forever if the callback never runs + test('answers with nothing misspelled', () => { + expect(callback).toHaveBeenCalledExactlyOnceWith([]); + }); + }); + describe('with a callback that throws', () => { + let consoleError; + let callback; + beforeEach(async () => { + consoleError = jest.spyOn(console, 'error').mockImplementation(() => {}); + callback = jest.fn(() => { + throw new Error('Blink blew up'); + }); + spellCheck(['misspelled'], callback); + await waitFor(() => expect(idleCallbacks).toHaveLength(1)); + runIdleCallbacks(); + }); + afterEach(() => { + consoleError.mockRestore(); + }); + test('reports the error', () => { + expect(consoleError).toHaveBeenCalledWith('Spell check callback failed', expect.any(Error)); + }); + test('answers the request only once', () => { + expect(callback).toHaveBeenCalledTimes(1); + }); }); - // When - await electron.webFrame.spellCheckProviders.eo.spellCheck(['word'], spellCheckCallback); - // Then - // Catching a throw from the callback would answer the very same request a second time - await waitFor(() => expect(consoleError).toHaveBeenCalledWith('Spell check callback failed', expect.any(Error))); - expect(spellCheckCallback).toHaveBeenCalledTimes(1); - consoleError.mockRestore(); }); }); diff --git a/src/service-manager/preload.spell-check.js b/src/service-manager/preload.spell-check.js index d7eed334..aa1e3ceb 100644 --- a/src/service-manager/preload.spell-check.js +++ b/src/service-manager/preload.spell-check.js @@ -16,16 +16,38 @@ /* eslint-disable no-undef */ const {ipcRenderer, webFrame} = require('electron'); +// Electron's SpellCheckClient tracks a single pending request, and dereferences it unchecked when a +// callback runs. Answering a request that is no longer the pending one completes the wrong request, +// and the next answer finds nothing pending: the renderer segfaults and the service turns blank. +// Blink supersedes requests routinely since Chromium 152 (Electron 44). It cancels its in-flight check +// whenever a script moves the caret in a field the user didn't focus themselves (Slack and Telegram +// focus their composers by script), and issues a new one on the next keystroke. +// So only the most recent request is ever answered, exactly once, and only when the renderer is idle. +// Electron hands a request over to this function from a task it posts, so Blink may have issued a +// request that hasn't reached it yet. That task has normal priority and always runs before an idle +// callback, which then finds its own request superseded. +let pendingRequest = null; + const spellCheckFunction = (words, callback) => { + const request = {}; + pendingRequest = request; + const answer = misspelled => requestIdleCallback(() => { + if (pendingRequest !== request) { + return; + } + pendingRequest = null; + try { + callback(misspelled); + } catch (error) { + // Blink's side of the contract breaking. The request is no longer pending, so it can't be + // answered twice, only reported. + console.error('Spell check callback failed', error); + } + }); ipcRenderer.invoke(APP_EVENTS.dictionaryGetMisspelled, words) // Blink keeps the request pending until the callback runs, so a rejection (the dictionary - // renderer failing to load, say) must still answer, reporting nothing as misspelled. The - // rejection handler is passed to then rather than chained through catch, which would also - // catch a throw from the callback itself and answer the same request a second time. - .then(misspelled => callback(misspelled), () => callback([])) - // Only ever reached if the callback itself threw, which is Blink's side of the contract - // breaking. Report it rather than leaving an unhandled rejection, and never answer again. - .catch(error => console.error('Spell check callback failed', error)); + // renderer failing to load, say) must still answer, reporting nothing as misspelled. + .then(answer, () => answer([])); }; const initSpellChecker = () => new Promise(resolve => { From 23bf8217ff62efabfd3519988b71815b27e1b69b Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Mon, 14 Sep 2026 15:06:52 +0200 Subject: [PATCH 2/2] test(spell-check): guard against an idle callback timeout A timed-out idle callback runs as a regular task, which could beat Electron's hand-over of a newer spell check request and reopen the renderer crash. State the requirement next to the delivery and fail a test if a timeout is ever passed. Signed-off-by: Marc Nuri --- .../__tests__/preload.spell-check.test.js | 12 +++++++++++- src/service-manager/preload.spell-check.js | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/service-manager/__tests__/preload.spell-check.test.js b/src/service-manager/__tests__/preload.spell-check.test.js index 76b8c7e7..2decd566 100644 --- a/src/service-manager/__tests__/preload.spell-check.test.js +++ b/src/service-manager/__tests__/preload.spell-check.test.js @@ -20,6 +20,7 @@ describe('Browser Spell Check test suite', () => { let settings; let browserSpellCheck; let idleCallbacks; + let idleCallbackOptions; const runIdleCallbacks = () => { for (const idleCallback of idleCallbacks.splice(0)) { idleCallback(); @@ -30,7 +31,11 @@ describe('Browser Spell Check test suite', () => { globalThis.APP_EVENTS = require('../../constants').APP_EVENTS; // jsdom has no requestIdleCallback: queue the callbacks so each test decides when the renderer is idle idleCallbacks = []; - globalThis.requestIdleCallback = idleCallback => idleCallbacks.push(idleCallback); + idleCallbackOptions = []; + globalThis.requestIdleCallback = (idleCallback, options) => { + idleCallbacks.push(idleCallback); + idleCallbackOptions.push(options); + }; electron = require('../../__tests__').testElectron(); settings = await require('../../__tests__').testSettings(); electron.ipcMain.on('settingsLoad', settings.loadSettings); @@ -97,6 +102,11 @@ describe('Browser Spell Check test suite', () => { test('does not answer before the renderer is idle', () => { expect(callback).not.toHaveBeenCalled(); }); + // A timed-out idle callback runs as a regular task, which could beat Electron's hand-over of a + // newer request and answer a superseded one + test('waits for the renderer to be idle without a timeout', () => { + expect(idleCallbackOptions[0]?.timeout).toBeUndefined(); + }); test('answers with the misspelled words once the renderer is idle', () => { runIdleCallbacks(); expect(callback).toHaveBeenCalledExactlyOnceWith(['misspelled']); diff --git a/src/service-manager/preload.spell-check.js b/src/service-manager/preload.spell-check.js index aa1e3ceb..ec810c03 100644 --- a/src/service-manager/preload.spell-check.js +++ b/src/service-manager/preload.spell-check.js @@ -25,7 +25,8 @@ const {ipcRenderer, webFrame} = require('electron'); // So only the most recent request is ever answered, exactly once, and only when the renderer is idle. // Electron hands a request over to this function from a task it posts, so Blink may have issued a // request that hasn't reached it yet. That task has normal priority and always runs before an idle -// callback, which then finds its own request superseded. +// callback, which then finds its own request superseded. No timeout: a timed-out idle callback runs as +// a regular task and could beat Electron's hand-over. let pendingRequest = null; const spellCheckFunction = (words, callback) => {