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
This commit is contained in:
Juan Tejada 2016-01-29 14:11:01 -08:00
parent 48a64e7fef
commit d8e139764a
3 changed files with 25 additions and 21 deletions

View file

@ -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

View file

@ -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.

View file

@ -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: ->