From d8e139764abc5e2d6cb9365075b900363ee17213 Mon Sep 17 00:00:00 2001
From: Juan Tejada <juans.tejada@gmail.com>
Date: Fri, 29 Jan 2016 14:11:01 -0800
Subject: [PATCH] fix(electron): Update `beforeunload` event handler (see
 details):

- This fixes unresponsive draft items in the draft list, i.e. draft
  windows would not reopen after opening them the first time.
- The `beforeunload` handler will no longer hide a window before its
  actually closed. To reduce visible latency when closing, we moved
  saving window state to the `unload` event handler.
- Before unload was hiding the window before actually closing it. A
  hidden window causes chromium to throttle its renderer process for
  performance, even though `pageVisibility` was set to true (see
  https://github.com/atom/electron/issues/3225 for more detils).

  NylasEnv.finishUnload is used in this context: When a `beforeunload`
  callback prevents the window from closing, it can close it at some point
  in the future using finishUnload. NylasEnv.finishUnload uses _.defer to make
  sure we don't call `close` inside the `beforeunload` call stack so the
  window doesn't go crazy.

  However, since _.defer was being called in the renderer process of a
  hidden window, the deferred callback execution could end up delayed by
  a second or more, which effectively delayed closing the window by a
  second.

  If we tried to reopen a window with the same window props, e.g. a
  draft window, before it was actually closed, the app would go crazy
  and wouldn't open it.

  So now we just don't hide windows on beforeunload
---
 src/browser/nylas-window.coffee |  4 ++++
 src/nylas-env.coffee            | 18 ++----------------
 src/window-event-handler.coffee | 24 +++++++++++++++++++-----
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/src/browser/nylas-window.coffee b/src/browser/nylas-window.coffee
index fcd7fa88f..a28d1095d 100644
--- a/src/browser/nylas-window.coffee
+++ b/src/browser/nylas-window.coffee
@@ -65,6 +65,10 @@ class NylasWindow
     if @neverClose
       # Prevents DOM timers from being suspended when the main window is hidden.
       # Means there's not an awkward catch-up when you re-show the main window.
+      # TODO
+      # This option is no longer working according to
+      # https://github.com/atom/electron/issues/3225
+      # Look into using option --disable-renderer-backgrounding
       options.webPreferences.pageVisibility = true
 
     # Don't set icon on Windows so the exe's ico will be used as window and
diff --git a/src/nylas-env.coffee b/src/nylas-env.coffee
index 5a03c572e..565b189a1 100644
--- a/src/nylas-env.coffee
+++ b/src/nylas-env.coffee
@@ -212,9 +212,6 @@ class NylasEnvConstructor extends Model
     @subscribe @packages.onDidActivateInitialPackages => @watchThemes()
     @windowEventHandler = new WindowEventHandler
 
-    window.onbeforeunload = => @_unloading()
-    @_unloadCallbacks = []
-
   # Start our error reporting to the backend and attach error handlers
   # to the window and the Bluebird Promise library, converting things
   # back through the sourcemap as necessary.
@@ -915,19 +912,8 @@ class NylasEnvConstructor extends Model
   # work and then call finishUnload. We do not support cancelling quit!
   # https://phab.nylas.com/D1932#inline-11722
   #
-  onBeforeUnload: (callback) -> @_unloadCallbacks.push(callback)
-
-  _unloading: ->
-    continueUnload = true
-    for callback in @_unloadCallbacks
-      returnValue = callback()
-      if returnValue is true
-        continue
-      else if returnValue is false
-        continueUnload = false
-      else
-        console.warn "You registered an `onBeforeUnload` callback that does not return either exactly `true` or `false`. It returned #{returnValue}", callback
-    return continueUnload
+  onBeforeUnload: (callback) ->
+    @windowEventHandler.addUnloadCallback(callback)
 
   # Call this method to resume the close / quit process if you returned
   # false from a onBeforeUnload handler.
diff --git a/src/window-event-handler.coffee b/src/window-event-handler.coffee
index 14f8fff11..5d04ba223 100644
--- a/src/window-event-handler.coffee
+++ b/src/window-event-handler.coffee
@@ -14,6 +14,7 @@ class WindowEventHandler
 
   constructor: ->
     @reloadRequested = false
+    @unloadCallbacks = []
 
     _.defer =>
       @showDevModeMessages()
@@ -47,14 +48,12 @@ class WindowEventHandler
       NylasEnv.commands.dispatch(activeElement, command, args[0])
 
     @subscribe $(window), 'beforeunload', =>
-      if NylasEnv.getCurrentWindow().isWebViewFocused() and not @reloadRequested
-        NylasEnv.hide()
       @reloadRequested = false
-      NylasEnv.storeWindowDimensions()
-      NylasEnv.saveStateAndUnloadWindow()
-      true
+      return @runUnloadCallbacks()
 
     @subscribe $(window), 'unload', =>
+      NylasEnv.storeWindowDimensions()
+      NylasEnv.saveStateAndUnloadWindow()
       NylasEnv.windowEventHandler?.unsubscribe()
 
     @subscribeToCommand $(window), 'window:toggle-full-screen', ->
@@ -111,6 +110,21 @@ class WindowEventHandler
 
     @handleNativeKeybindings()
 
+  addUnloadCallback: (callback) ->
+    @unloadCallbacks.push(callback)
+
+  runUnloadCallbacks: ->
+    continueUnload = true
+    for callback in @unloadCallbacks
+      returnValue = callback()
+      if returnValue is true
+        continue
+      else if returnValue is false
+        continueUnload = false
+      else
+        console.warn "You registered an `onBeforeUnload` callback that does not return either exactly `true` or `false`. It returned #{returnValue}", callback
+    return continueUnload
+
   # Wire commands that should be handled by Chromium for elements with the
   # `.override-key-bindings` class.
   handleNativeKeybindings: ->