fix(win-management): Track windowKey changes, add assertions

src/browser/window-launcher.es6

@@ -24,47 +24,35 @@ export default class WindowLauncher {
       safeMode: appOpts.safeMode,
       resizable: true,
       windowType: WindowLauncher.EMPTY_WINDOW,
+      bootstrapScript: require.resolve("../secondary-window-bootstrap"),
       resourcePath: appOpts.resourcePath,
       configDirPath: appOpts.configDirPath,
     }
-    this.hotWindow = new NylasWindow(this._hotWindowOpts());
-
-    if (DEBUG_SHOW_HOT_WINDOW) {
-      this.hotWindow.showWhenLoaded()
-    }
+    this.createHotWindow();
   }
 
   newWindow(options) {
     const opts = Object.assign({}, this.defaultWindowOpts, options);
+
     let win;
-    if (opts.bootstrapScript) {
+    if (this._mustUseColdWindow(opts)) {
       win = new NylasWindow(opts)
     } else {
-      opts.bootstrapScript = this._secondaryWindowBootstrap()
-      if (this._unableToModifyHotWindow(opts) || opts.coldStartOnly) {
-        // Useful for the Worker Window: A secondary window that shouldn't
-        // be hot-loaded
-        win = new NylasWindow(opts)
-      } else {
-        win = this.hotWindow;
+      win = this.hotWindow;
+      this.createHotWindow();
 
-        // Regenerate the hot window.
-        this.hotWindow = new NylasWindow(this._hotWindowOpts());
-        if (DEBUG_SHOW_HOT_WINDOW) {
-          this.hotWindow.showWhenLoaded()
-        }
-
-        const newLoadSettings = Object.assign({}, win.loadSettings(), opts)
-        if (newLoadSettings.windowType === WindowLauncher.EMPTY_WINDOW) {
-          throw new Error("Must specify a windowType")
-        }
-
-        // Reset the loaded state and update the load settings.
-        // This will fire `NylasEnv::populateHotWindow` and reload the
-        // packages.
-        win.setLoadSettings(newLoadSettings);
+      const newLoadSettings = Object.assign({}, win.loadSettings(), opts)
+      if (newLoadSettings.windowType === WindowLauncher.EMPTY_WINDOW) {
+        throw new Error("Must specify a windowType")
       }
+
+      // Reset the loaded state and update the load settings.
+      // This will fire `NylasEnv::populateHotWindow` and reload the
+      // packages.
+      win.windowKey = opts.windowKey;
+      win.setLoadSettings(newLoadSettings);
     }
+
     if (!opts.hidden) {
       // NOTE: In the case of a cold window, this will show it once
       // loaded. If it's a hotWindow, since hotWindows have a
@@ -76,6 +64,13 @@ export default class WindowLauncher {
     return win
   }
 
+  createHotWindow() {
+    this.hotWindow = new NylasWindow(this._hotWindowOpts());
+    if (DEBUG_SHOW_HOT_WINDOW) {
+      this.hotWindow.showWhenLoaded()
+    }
+  }
+
   // Note: This method calls `browserWindow.destroy()` which closes
   // windows without waiting for them to load or firing window lifecycle
   // events.  This is necessary for the app to quit promptly on Linux.
@@ -87,21 +82,19 @@ export default class WindowLauncher {
   // Some properties, like the `frame` or `toolbar` can't be updated once
   // a window has been setup. If we detect this case we have to bootup a
   // plain NylasWindow instead of using a hot window.
-  _unableToModifyHotWindow(opts) {
-    return this.defaultWindowOpts.frame !== (!!opts.frame)
-  }
+  _mustUseColdWindow(opts) {
+    const {bootstrapScript, frame} = this.defaultWindowOpts;
 
-  _secondaryWindowBootstrap() {
-    if (!this._bootstrap) {
-      this._bootstrap = require.resolve("../secondary-window-bootstrap")
-    }
-    return this._bootstrap
+    const usesOtherBootstrap = opts.bootstrapScript !== bootstrapScript;
+    const usesOtherFrame = (!!opts.frame) !== frame;
+    const requestsColdStart = opts.coldStartOnly;
+
+    return usesOtherBootstrap || usesOtherFrame || requestsColdStart;
   }
 
   _hotWindowOpts() {
     const hotWindowOpts = Object.assign({}, this.defaultWindowOpts);
     hotWindowOpts.packageLoadingDeferred = true;
-    hotWindowOpts.bootstrapScript = this._secondaryWindowBootstrap();
     hotWindowOpts.hidden = DEBUG_SHOW_HOT_WINDOW;
     return hotWindowOpts
   }

src/browser/window-manager.coffee

@@ -23,8 +23,9 @@ class WindowManager
     # window to not work and may even prevent window closure (like in the
     # case of the composer)
     @_registerWindow(@windowLauncher.hotWindow)
+    @_didCreateNewWindow(@windowLauncher.hotWindow)
 
-  get: (winId) -> @_windows[winId]
+  get: (windowKey) -> @_windows[windowKey]
 
   getOpenWindows: ->
     values = []
@@ -37,12 +38,26 @@ class WindowManager
 
   newWindow: (options={}) ->
     win = @windowLauncher.newWindow(options)
+
+    existingKey = @_registeredKeyForWindow(win)
+    delete @_windows[existingKey] if existingKey
     @_registerWindow(win)
+
+    if not existingKey
+      @_didCreateNewWindow(win)
+
     return win
 
   _registerWindow: (win) =>
+    unless win.windowKey
+      throw new Error("WindowManager: You must provide a windowKey")
+
+    if @_windows[win.windowKey]
+      throw new Error("WindowManager: Attempting to register a new window for an existing windowKey (#{win.windowKey}). Use `get()` to retrieve the existing window instead.")
+
     @_windows[win.windowKey] = win
 
+  _didCreateNewWindow: (win) =>
     win.browserWindow.on "closed", =>
       delete @_windows[win.windowKey]
       @quitWinLinuxIfNoWindows()
@@ -52,8 +67,14 @@ class WindowManager
     # the browserWindow to unregister itself
     global.application.applicationMenu.addWindow(win.browserWindow)
 
-  ensureWindow: (winId, extraOpts) ->
-    win = @_windows[winId]
+  _registeredKeyForWindow: (win) =>
+    for key, otherWin of @_windows
+      if win is otherWin
+        return key
+    return null
+
+  ensureWindow: (windowKey, extraOpts) ->
+    win = @_windows[windowKey]
     if win
       return if win.loadSettings().hidden
       if win.isMinimized()
@@ -64,21 +85,21 @@ class WindowManager
       else
         win.focus()
     else
-      @newWindow(@_coreWindowOpts(winId, extraOpts))
+      @newWindow(@_coreWindowOpts(windowKey, extraOpts))
 
-  sendToWindow: (winId, args...) ->
-    if not @_windows[winId]
-      throw new Error("Can't find window: #{winId}")
-    @_windows[winId].sendMessage(args...)
+  sendToWindow: (windowKey, args...) ->
+    if not @_windows[windowKey]
+      throw new Error("Can't find window: #{windowKey}")
+    @_windows[windowKey].sendMessage(args...)
 
   sendToAllWindows: (msg, {except}, args...) ->
-    for winId, win of @_windows
+    for windowKey, win of @_windows
       continue if win.browserWindow == except
       continue unless win.browserWindow.webContents
       win.browserWindow.webContents.send(msg, args...)
 
   closeAllWindows: ->
-    win.close() for winId, win of @_windows
+    win.close() for windowKey, win of @_windows
 
   cleanupBeforeAppQuit: -> @windowLauncher.cleanupBeforeAppQuit()
 
@@ -104,7 +125,7 @@ class WindowManager
 
   focusedWindow: -> _.find(@_windows, (win) -> win.isFocused())
 
-  _coreWindowOpts: (winId, extraOpts={}) ->
+  _coreWindowOpts: (windowKey, extraOpts={}) ->
     coreWinOpts = {}
     coreWinOpts[WindowManager.MAIN_WINDOW] =
       windowKey: WindowManager.MAIN_WINDOW
@@ -149,7 +170,7 @@ class WindowManager
       devMode: true,
       toolbar: false
 
-    defaultOptions = coreWinOpts[winId] ? {}
+    defaultOptions = coreWinOpts[windowKey] ? {}
 
     return Object.assign({}, defaultOptions, extraOpts)