fix(window-serialization): Restore window size, remove cruft from atom.coffee

Summary: Simplify the default window size, restore window size when the main window is loaded, fix serialization of packages in beforeunload.

Test Plan: Run tests

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D2061

internal_packages/attachments/lib/main.cjsx

@@ -1,10 +1,10 @@
 {ComponentRegistry} = require 'nylas-exports'
 
+AttachmentComponent = require "./attachment-component"
+ImageAttachmentComponent = require "./image-attachment-component"
+
 module.exports =
   activate: (@state={}) ->
-    AttachmentComponent = require "./attachment-component"
-    ImageAttachmentComponent = require "./image-attachment-component"
-
     ComponentRegistry.register AttachmentComponent,
       role: 'Attachment'
 
@@ -12,7 +12,7 @@ module.exports =
       role: 'Attachment:Image'
 
   deactivate: ->
-    ComponentRegistry.unregister AttachmentComponent
+    ComponentRegistry.unregister(AttachmentComponent)
-    ComponentRegistry.unregister ImageAttachmentComponent
+    ComponentRegistry.unregister(ImageAttachmentComponent)
 
   serialize: -> @state

internal_packages/composer/lib/main.cjsx

@@ -25,7 +25,7 @@ class ComposerWithWindowProps extends React.Component
         @_showInitialErrorDialog(errorMessage)
 
   componentWillUnmount: ->
-    @unlisten()
+    @unlisten?()
 
   render: ->
     <div className="composer-full-window">

internal_packages/events/lib/main.cjsx

@@ -1,13 +1,13 @@
 {ComponentRegistry, WorkspaceStore} = require 'nylas-exports'
 
+EventComponent = require "./event-component"
+
 module.exports =
   activate: (@state={}) ->
-    EventComponent = require "./event-component"
-
     ComponentRegistry.register EventComponent,
       role: 'Event'
 
   deactivate: ->
-    ComponentRegistry.unregister EventComponent
+    ComponentRegistry.unregister(EventComponent)
 
   serialize: -> @state

internal_packages/notification-mailto/lib/main.coffee

@@ -33,7 +33,7 @@ module.exports =
           }]
 
   deactivate: ->
-    @_unlisten()
+    @_unlisten?()
 
   serialize: -> @state
 

internal_packages/undo-redo/lib/main.cjsx

@@ -1,13 +1,13 @@
 {ComponentRegistry, WorkspaceStore} = require 'nylas-exports'
 
+UndoRedoComponent = require "./undo-redo-component"
+
 module.exports =
   activate: (@state={}) ->
-    UndoRedoComponent = require "./undo-redo-component"
-
     ComponentRegistry.register UndoRedoComponent,
       location: WorkspaceStore.Location.ThreadList
 
   deactivate: ->
-    ComponentRegistry.unregister UndoRedoComponent
+    ComponentRegistry.unregister(UndoRedoComponent)
 
   serialize: -> @state

spec/deserializer-manager-spec.coffee

@@ -1,44 +0,0 @@
-DeserializerManager = require '../src/deserializer-manager'
-
-describe "DeserializerManager", ->
-  manager = null
-
-  class Foo
-    @deserialize: ({name}) -> new Foo(name)
-    constructor: (@name) ->
-
-  beforeEach ->
-    manager = new DeserializerManager
-
-  describe "::add(deserializer)", ->
-    it "returns a disposable that can be used to remove the manager", ->
-      disposable = manager.add(Foo)
-      expect(manager.deserialize({deserializer: 'Foo', name: 'Bar'})).toBeDefined()
-      disposable.dispose()
-      spyOn(console, 'warn')
-      expect(manager.deserialize({deserializer: 'Foo', name: 'Bar'})).toBeUndefined()
-
-  describe "::deserialize(state)", ->
-    beforeEach ->
-      manager.add(Foo)
-
-    it "calls deserialize on the manager for the given state object, or returns undefined if one can't be found", ->
-      spyOn(console, 'warn')
-      object = manager.deserialize({deserializer: 'Foo', name: 'Bar'})
-      expect(object.name).toBe 'Bar'
-      expect(manager.deserialize({deserializer: 'Bogus'})).toBeUndefined()
-
-    describe "when the manager has a version", ->
-      beforeEach ->
-        Foo.version = 2
-
-      describe "when the deserialized state has a matching version", ->
-        it "attempts to deserialize the state", ->
-          object = manager.deserialize({deserializer: 'Foo', version: 2, name: 'Bar'})
-          expect(object.name).toBe 'Bar'
-
-      describe "when the deserialized state has a non-matching version", ->
-        it "returns undefined", ->
-          expect(manager.deserialize({deserializer: 'Foo', version: 3, name: 'Bar'})).toBeUndefined()
-          expect(manager.deserialize({deserializer: 'Foo', version: 1, name: 'Bar'})).toBeUndefined()
-          expect(manager.deserialize({deserializer: 'Foo', name: 'Bar'})).toBeUndefined()

spec/window-spec.coffee

@@ -92,13 +92,13 @@ describe "Window", ->
           $(window).trigger(beforeUnloadEvent)
           expect(atom.confirm).toHaveBeenCalled()
 
-  describe ".unloadEditorWindow()", ->
+  describe ".saveStateAndUnloadWindow()", ->
     it "saves the serialized state of the window so it can be deserialized after reload", ->
       workspaceState = atom.workspace.serialize()
       syntaxState = atom.grammars.serialize()
       projectState = atom.project.serialize()
 
-      atom.unloadEditorWindow()
+      atom.saveStateAndUnloadWindow()
 
       expect(atom.state.workspace).toEqual workspaceState
       expect(atom.state.grammars).toEqual syntaxState

src/atom.coffee

@@ -36,7 +36,6 @@ class Atom extends Model
     else
       app = new this({@version})
 
-    app.deserializeTimings.app = Date.now() -  startTime
     return app
 
   # Loads and returns the serialized state corresponding to this window
@@ -60,16 +59,11 @@ class Atom extends Model
   # Returns the path where the state for the current window will be
   # located if it exists.
   @getStatePath: ->
-    if @getLoadSettings().isSpec
+    {isSpec, mainWindow} = @getLoadSettings()
+    if isSpec
       filename = 'spec'
-    else
+    else if mainWindow
-      {initialPath} = @getLoadSettings()
+      path.join(@getConfigDirPath(), 'main-window-state.json')
-      if initialPath
-        sha1 = crypto.createHash('sha1').update(initialPath).digest('hex')
-        filename = "application-#{sha1}"
-
-    if filename
-      path.join(@getStorageDirPath(), filename)
     else
       null
 
@@ -79,12 +73,6 @@ class Atom extends Model
   @getConfigDirPath: ->
     @configDirPath ?= fs.absolute('~/.nylas')
 
-  # Get the path to Atom's storage directory.
-  #
-  # Returns the absolute path to ~/.nylas/storage
-  @getStorageDirPath: ->
-    @storageDirPath ?= path.join(@getConfigDirPath(), 'storage')
-
   # Returns the load settings hash associated with the current window.
   @getLoadSettings: ->
     @loadSettings ?= JSON.parse(decodeURIComponent(location.search.substr(14)))
@@ -131,9 +119,6 @@ class Atom extends Model
   # Public: A {StyleManager} instance
   styles: null
 
-  # Public: A {DeserializerManager} instance
-  deserializers: null
-
   ###
   Section: Construction and Destruction
   ###
@@ -142,9 +127,6 @@ class Atom extends Model
   constructor: (@savedState={}) ->
     {@version} = @savedState
     @emitter = new Emitter
-    DeserializerManager = require './deserializer-manager'
-    @deserializers = new DeserializerManager()
-    @deserializeTimings = {}
 
   # Sets up the basic services that should be available in all modes
   # (both spec and application).
@@ -568,8 +550,7 @@ class Atom extends Model
   #   * `height` The window's height {Number}.
   getWindowDimensions: ->
     browserWindow = @getCurrentWindow()
-    [x, y] = browserWindow.getPosition()
+    {x, y, width, height} = browserWindow.getBounds()
-    [width, height] = browserWindow.getSize()
     maximized = browserWindow.isMaximized()
     {x, y, width, height, maximized}
 
@@ -585,9 +566,11 @@ class Atom extends Model
   #   * `width` The new width.
   #   * `height` The new height.
   setWindowDimensions: ({x, y, width, height}) ->
-    if width? and height?
+    if x? and y? and width? and height?
+      @getCurrentWindow().setBounds({x, y, width, height})
+    else if width? and height?
       @setSize(width, height)
-    if x? and y?
+    else if x? and y?
       @setPosition(x, y)
     else
       @center()
@@ -597,36 +580,17 @@ class Atom extends Model
   isValidDimensions: ({x, y, width, height}={}) ->
     width > 0 and height > 0 and x + width > 0 and y + height > 0
 
-  storeDefaultWindowDimensions: ->
-    return unless @isMainWindow()
-    dimensions = @getWindowDimensions()
-    if @isValidDimensions(dimensions)
-      localStorage.setItem("defaultWindowDimensions", JSON.stringify(dimensions))
-
   getDefaultWindowDimensions: ->
-    {windowDimensions} = @getLoadSettings()
+    screen = remote.require('screen')
-    return windowDimensions if windowDimensions?
+    {width, height} = screen.getPrimaryDisplay().workAreaSize
-
+    {x: 0, y: 0, width, height}
-    dimensions = null
-    try
-      dimensions = JSON.parse(localStorage.getItem("defaultWindowDimensions"))
-    catch error
-      console.warn "Error parsing default window dimensions", error
-      localStorage.removeItem("defaultWindowDimensions")
-
-    if @isValidDimensions(dimensions)
-      dimensions
-    else
-      screen = remote.require('screen')
-      {width, height} = screen.getPrimaryDisplay().workAreaSize
-      {x: 0, y: 0, width, height}
 
   restoreWindowDimensions: ->
     dimensions = @savedState.windowDimensions
     unless @isValidDimensions(dimensions)
       dimensions = @getDefaultWindowDimensions()
     @setWindowDimensions(dimensions)
-    dimensions
+    @maximize() if dimensions.maximized and process.platform isnt 'darwin'
 
   storeWindowDimensions: ->
     dimensions = @getWindowDimensions()
@@ -642,7 +606,8 @@ class Atom extends Model
     @keymaps.loadBundledKeymaps()
     @themes.loadBaseStylesheets()
     @packages.loadPackages(windowType)
-    @deserializeRootWindow()
+    @deserializePackageStates()
+    @deserializeSheetContainer()
     @packages.activate()
     @keymaps.loadUserKeymap()
     @requireUserInitScript() unless safeMode
@@ -650,16 +615,13 @@ class Atom extends Model
 
     @showRootWindow()
 
-    ipc.sendChannel('window-command', 'window:main-window-content-loaded')
+    ipc.sendChannel('window-command', 'window:loaded')
 
   showRootWindow: ->
-    dimensions = @restoreWindowDimensions()
-    @getCurrentWindow().setMinimumSize(875, 500)
-    maximize = dimensions?.maximized and process.platform isnt 'darwin'
-    @maximize() if maximize
-    @center()
     cover = document.getElementById("application-loading-cover")
     cover.classList.add('visible')
+    @restoreWindowDimensions()
+    @getCurrentWindow().setMinimumSize(875, 500)
 
   registerCommands: ->
     {resourcePath} = @getLoadSettings()
@@ -727,15 +689,12 @@ class Atom extends Model
   # Unregisters a hot window with the given windowType
   unregisterHotWindow: (windowType) -> ipc.send('unregister-hot-window', windowType)
 
-  unloadEditorWindow: ->
+  saveStateAndUnloadWindow: ->
     @packages.deactivatePackages()
     @savedState.packageStates = @packages.packageStates
     @saveSync()
     @windowState = null
 
-  removeEditorWindow: ->
-    @windowEventHandler?.unsubscribe()
-
   ###
   Section: Messaging the User
   ###
@@ -825,8 +784,6 @@ class Atom extends Model
   deserializeSheetContainer: ->
     startTime = Date.now()
     # Put state back into sheet-container? Restore app state here
-    @deserializeTimings.workspace = Date.now() - startTime
-
     @item = document.createElement("atom-workspace")
     @item.setAttribute("id", "sheet-container")
     @item.setAttribute("class", "sheet-container")
@@ -841,10 +798,6 @@ class Atom extends Model
     @packages.packageStates = @savedState.packageStates ? {}
     delete @savedState.packageStates
 
-  deserializeRootWindow: ->
-    @deserializePackageStates()
-    @deserializeSheetContainer()
-
   loadThemes: ->
     @themes.load()
 

src/browser/application.coffee

@@ -161,7 +161,7 @@ class Application
     else
       @windowManager.newOnboardingWindow()
       # The onboarding window automatically shows when it's ready
-      
+
   _resetConfigAndRelaunch: =>
     @setDatabasePhase('close')
     @windowManager.closeAllWindows()

src/browser/atom-window.coffee

@@ -99,20 +99,16 @@ class AtomWindow
     @browserWindow.once 'window:loaded', =>
       @emit 'window:loaded'
       @loaded = true
-      if @browserWindow.loadSettingsChangedSinceGetURL
+      if @mainWindow
-        @browserWindow.webContents.send('load-settings-changed', @browserWindow.loadSettings)
+        @browserWindow.setResizable(true)
-
-    @browserWindow.once 'window:main-window-content-loaded', =>
-      @emit 'window:main-window-content-loaded'
-      @mainWindowContentLoaded = true
-      @browserWindow.setResizable(true)
       if @browserWindow.loadSettingsChangedSinceGetURL
         @browserWindow.webContents.send('load-settings-changed', @browserWindow.loadSettings)
 
     @browserWindow.loadUrl(@getUrl(loadSettings))
     @browserWindow.focusOnWebView() if @isSpec
 
-  loadSettings: -> @browserWindow.loadSettings
+  loadSettings: ->
+    @browserWindow.loadSettings
 
   setLoadSettings: (loadSettings) ->
     @browserWindow.loadSettings = loadSettings
@@ -165,7 +161,7 @@ class AtomWindow
 
     @browserWindow.on 'unresponsive', =>
       return if @isSpec
-      return if (not @mainWindowContentLoaded) and @mainWindow
+      return if not @loaded
 
       dialog = require 'dialog'
       chosen = dialog.showMessageBox @browserWindow,

src/deserializer-manager.coffee

@@ -1,67 +0,0 @@
-{Disposable} = require 'event-kit'
-Grim = require 'grim'
-
-# Extended: Manages the deserializers used for serialized state
-#
-# An instance of this class is always available as the `atom.deserializers`
-# global.
-#
-# Section: Atom
-#
-# ## Examples
-#
-# ```coffee
-# class MyPackageView extends View
-#   atom.deserializers.add(this)
-#
-#   @deserialize: (state) ->
-#     new MyPackageView(state)
-#
-#   constructor: (@state) ->
-#
-#   serialize: ->
-#     @state
-# ```
-module.exports =
-class DeserializerManager
-  constructor: ->
-    @deserializers = {}
-
-  # Public: Register the given class(es) as deserializers.
-  #
-  # * `deserializers` One or more deserializers to register. A deserializer can
-  #   be any object with a `.name` property and a `.deserialize()` method. A
-  #   common approach is to register a *constructor* as the deserializer for its
-  #   instances by adding a `.deserialize()` class method.
-  add: (deserializers...) ->
-    @deserializers[deserializer.name] = deserializer for deserializer in deserializers
-    new Disposable =>
-      delete @deserializers[deserializer.name] for deserializer in deserializers
-
-  remove: (classes...) ->
-    Grim.deprecate("Call .dispose() on the Disposable return from ::add instead")
-    delete @deserializers[name] for {name} in classes
-
-  # Public: Deserialize the state and params.
-  #
-  # * `state` The state {Object} to deserialize.
-  # * `params` The params {Object} to pass as the second arguments to the
-  #   deserialize method of the deserializer.
-  deserialize: (state, params) ->
-    return unless state?
-
-    if deserializer = @get(state)
-      stateVersion = state.get?('version') ? state.version
-      return if deserializer.version? and deserializer.version isnt stateVersion
-      deserializer.deserialize(state, params)
-    else
-      console.warn "No deserializer found for", state
-
-  # Get the deserializer for the state.
-  #
-  # * `state` The state {Object} being deserialized.
-  get: (state) ->
-    return unless state?
-
-    name = state.get?('deserializer') ? state.deserializer
-    @deserializers[name]

src/window-event-handler.coffee

@@ -36,7 +36,6 @@ class WindowEventHandler
 
     @subscribe ipc, 'browser-window-blur', ->
       document.body.classList.add('is-blurred')
-      atom.storeDefaultWindowDimensions()
 
     @subscribe ipc, 'command', (command, args...) ->
       activeElement = document.activeElement
@@ -46,17 +45,15 @@ class WindowEventHandler
       atom.commands.dispatch(activeElement, command, args[0])
 
     @subscribe $(window), 'beforeunload', =>
-      confirmed = atom.workspace?.confirmClose()
+      if atom.getCurrentWindow().isWebViewFocused() and not @reloadRequested
-      atom.hide() if confirmed and not @reloadRequested and atom.getCurrentWindow().isWebViewFocused()
+        atom.hide()
       @reloadRequested = false
-
-      atom.storeDefaultWindowDimensions()
       atom.storeWindowDimensions()
-      atom.unloadEditorWindow() if confirmed
+      atom.saveStateAndUnloadWindow()
+      true
 
-      confirmed
+    @subscribe $(window), 'unload', =>
-
+      atom.windowEventHandler?.unsubscribe()
-    @subscribe $(window), 'unload', -> atom.removeEditorWindow()
 
     @subscribeToCommand $(window), 'window:toggle-full-screen', ->
       atom.toggleFullScreen()