From 1e3a540a9d47ecae321734e1aa1283887aec8e05 Mon Sep 17 00:00:00 2001 From: Islam Sharabash Date: Wed, 31 Jan 2018 15:53:04 -0800 Subject: [PATCH 1/3] Don't send messages to destroyed webContents This commit: 1. Prevents sending messages to destroyed webContents (which result in a crash) 2. Allows registering multiple webContents for push notifications We need this because we are listening for notifcations in a specific webContents. If it crashes we create a new one and call setup again, without this change the next push notification that came in would cause a crash. --- src/index.js | 58 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/src/index.js b/src/index.js index 251fb34..2c15535 100644 --- a/src/index.js +++ b/src/index.js @@ -21,10 +21,14 @@ module.exports = { }; // To be sure that start is called only once +let starting = false; let started = false; +let webContentses = []; // To be call from the main process function setup(webContents) { + addWebConents(webContents) + // Will be called by the renderer process ipcMain.on(START_NOTIFICATION_SERVICE, async (_, senderId) => { // Retrieve saved credentials @@ -32,10 +36,15 @@ function setup(webContents) { // Retrieve saved senderId const savedSenderId = config.get('senderId'); if (started) { - webContents.send(NOTIFICATION_SERVICE_STARTED, (credentials.fcm || {}).token); + if (!webContents.isDestroyed()) { + webContents.send(NOTIFICATION_SERVICE_STARTED, (credentials.fcm || {}).token); + } return; } - started = true; + if (starting) { + return + } + starting = true; try { // Retrieve saved persistentId : avoid receiving all already received notifications on start const persistentIds = config.get('persistentIds') || []; @@ -46,28 +55,55 @@ function setup(webContents) { config.set('credentials', credentials); // Save senderId config.set('senderId', senderId); - // Notify the renderer process that the FCM token has changed - webContents.send(TOKEN_UPDATED, credentials.fcm.token); + // Notify the renderer processes that the FCM token has changed + send(TOKEN_UPDATED, credentials.fcm.token); } // Listen for GCM/FCM notifications await listen(Object.assign({}, credentials, { persistentIds }), onNotification(webContents)); - // Notify the renderer process that we are listening for notifications - webContents.send(NOTIFICATION_SERVICE_STARTED, credentials.fcm.token); + // Notify the renderer processes that we are listening for notifications + send(NOTIFICATION_SERVICE_STARTED, credentials.fcm.token); + started = true; } catch (e) { console.error('PUSH_RECEIVER:::Error while starting the service', e); - // Forward error to the renderer process - webContents.send(NOTIFICATION_SERVICE_ERROR, e.message); + // Forward error to the renderer processes + send(NOTIFICATION_SERVICE_ERROR, e.message); + starting = false; + started = false; } }); } +function addWebConents(webContents) { + webContentses.push(webContents) + webContents.on('destroyed', () => { + removeWebContents(webContents) + }) +} + +function removeWebContents(webContents) { + let i = webContentses.indexOf(webContents) + if (i > -1) { + webContentses.splice(i, 1) + } +} + +function send(channel, arg1) { + webContentses.forEach((webContents) => { + if (webContents.isDestroyed()) { + // sending to a destroyed web contents crashes the process, so be safe + return + } + webContents.send(channel, arg1) + }) +} + // Will be called on new notification -function onNotification(webContents) { +function onNotification() { return ({ notification, persistentId }) => { const persistentIds = config.get('persistentIds') || []; // Update persistentId config.set('persistentIds', [...persistentIds, persistentId]); - // Notify the renderer process that a new notification has been received - webContents.send(NOTIFICATION_RECEIVED, notification); + // Notify the renderer processes that a new notification has been received + send(NOTIFICATION_RECEIVED, notification); }; } From dbfa9e899d5769a37b12e7f02a1c142284ea1dd7 Mon Sep 17 00:00:00 2001 From: Islam Sharabash Date: Wed, 31 Jan 2018 17:02:23 -0800 Subject: [PATCH 2/3] Remove calls to isDestroyed That seems to be crashing the app ref: https://github.com/electron/electron/issues/11797 --- src/index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/index.js b/src/index.js index 2c15535..964d40b 100644 --- a/src/index.js +++ b/src/index.js @@ -36,8 +36,10 @@ function setup(webContents) { // Retrieve saved senderId const savedSenderId = config.get('senderId'); if (started) { - if (!webContents.isDestroyed()) { + try { webContents.send(NOTIFICATION_SERVICE_STARTED, (credentials.fcm || {}).token); + } catch (e) { + console.error('PUSH_RECEIVER:::Error while sending to webContents', e); } return; } @@ -89,11 +91,11 @@ function removeWebContents(webContents) { function send(channel, arg1) { webContentses.forEach((webContents) => { - if (webContents.isDestroyed()) { - // sending to a destroyed web contents crashes the process, so be safe - return + try { + webContents.send(channel, arg1) + } catch (e) { + console.error('PUSH_RECEIVER:::Error while sending to webContents', e); } - webContents.send(channel, arg1) }) } From 99208466ee254ba2637c883b2131e746d27c20cc Mon Sep 17 00:00:00 2001 From: Islam Sharabash Date: Wed, 7 Feb 2018 17:43:15 -0800 Subject: [PATCH 3/3] Add an option to throw errors --- src/index.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/index.js b/src/index.js index 964d40b..4d6ea7c 100644 --- a/src/index.js +++ b/src/index.js @@ -26,7 +26,7 @@ let started = false; let webContentses = []; // To be call from the main process -function setup(webContents) { +function setup(webContents, {throwErrors} = {}) { addWebConents(webContents) // Will be called by the renderer process @@ -61,16 +61,26 @@ function setup(webContents) { send(TOKEN_UPDATED, credentials.fcm.token); } // Listen for GCM/FCM notifications - await listen(Object.assign({}, credentials, { persistentIds }), onNotification(webContents)); + let client = await listen(Object.assign({}, credentials, { persistentIds }), onNotification(webContents)); + client.on('parserError', (error) => { + console.error('PUSH_RECEIVER:::Error from parser', error); + send(NOTIFICATION_SERVICE_ERROR, error.message); + if (throwErrors) { + throw error + } + }) // Notify the renderer processes that we are listening for notifications send(NOTIFICATION_SERVICE_STARTED, credentials.fcm.token); started = true; - } catch (e) { - console.error('PUSH_RECEIVER:::Error while starting the service', e); + } catch (error) { + console.error('PUSH_RECEIVER:::Error while starting the service', error); // Forward error to the renderer processes - send(NOTIFICATION_SERVICE_ERROR, e.message); + send(NOTIFICATION_SERVICE_ERROR, error.message); starting = false; started = false; + if (throwErrors) { + throw error + } } }); }