Ensure createWindow is never called before the app is ready

Author: pimterry

src/index.ts

@@ -32,6 +32,7 @@ registerContextMenu({
 
 import { reportStartupEvents } from './report-install-event';
 import { menu } from './menu';
+import { getDeferred } from './util';
 
 const rmRF = promisify(rimraf);
 
@@ -141,18 +142,6 @@ if (!amMainInstance) {
         process.exit(1); // Safe to hard exit - we haven't opened/started anything yet.
     }
 
-    app.on('ready', () => {
-        Menu.setApplicationMenu(menu);
-    });
-
-    app.on('window-all-closed', () => {
-        // On OS X it is common for applications and their menu bar
-        // to stay active until the user quits explicitly with Cmd + Q
-        if (process.platform !== 'darwin') {
-            app.quit();
-        }
-    });
-
     let serverKilled = false;
     app.on('will-quit', (event) => {
         if (server && !serverKilled) {
@@ -186,14 +175,6 @@ if (!amMainInstance) {
         }
     });
 
-    app.on('activate', () => {
-        // On OS X it's common to re-create a window in the app when the
-        // dock icon is clicked and there are no other windows open.
-        if (windows.length === 0) {
-            createWindow();
-        }
-    });
-
     app.on('web-contents-created', (_event, contents) => {
         function injectValue(name: string, value: string) {
             // Set a variable globally, and self-postmessage it too (to ping
@@ -400,17 +381,47 @@ if (!amMainInstance) {
     reportStartupEvents();
 
     cleanupOldServers().catch(console.log)
-    .then(() => startServer())
-    .catch((err) => {
+    .then(() =>
+        startServer()
+    ).catch((err) => {
         console.error('Failed to start server, exiting.', err);
 
         // Hide immediately, shutdown entirely after a brief pause for Sentry
         windows.forEach(window => window.hide());
         setTimeout(() => process.exit(1), 500);
     });
 
-    app.on('ready', () => createWindow());
+    // Use a promise to organize events around 'ready', and ensure they never
+    // fire before, as Electron will refuse to do various things if they do.
+    const appReady = getDeferred();
+    app.on('ready', () => appReady.resolve());
+
+    appReady.promise.then(() => {
+        Menu.setApplicationMenu(menu);
+        createWindow();
+    });
+
     // We use a single process instance to manage the server, but we
     // do allow multiple windows.
-    app.on('second-instance', () => createWindow());
+    app.on('second-instance', () =>
+        appReady.promise.then(() => createWindow())
+    );
+
+    app.on('activate', () => {
+        // On OS X it's common to re-create a window in the app when the
+        // dock icon is clicked and there are no other windows open.
+        if (windows.length === 0) {
+            // Wait until the ready event - it's possible that this can fire
+            // before the app is ready (not sure how) in which case things break!
+            appReady.promise.then(() => createWindow());
+        }
+    });
+
+    app.on('window-all-closed', () => {
+        // On OS X it is common for applications and their menu bar
+        // to stay active until the user quits explicitly with Cmd + Q
+        if (process.platform !== 'darwin') {
+            app.quit();
+        }
+    });
 }

src/util.ts

@@ -0,0 +1,21 @@
+// Taken from HTTP Toolkit UI's util/promise.ts:
+
+export interface Deferred<T> {
+    resolve: (arg: T) => void,
+    reject: (e?: Error) => void,
+    promise: Promise<T>
+}
+
+export function getDeferred<T = void>(): Deferred<T> {
+    let resolve: undefined | ((arg: T) => void) = undefined;
+    let reject: undefined | ((e?: Error) => void) = undefined;
+
+    let promise = new Promise<T>((resolveCb, rejectCb) => {
+        resolve = resolveCb;
+        reject = rejectCb;
+    });
+
+    // TS thinks we're using these before they're assigned, which is why
+    // we need the undefined types, and the any here.
+    return { resolve, reject, promise } as any;
+}