Merge pull request #10888 from atom/mb-fix-package-deserializers

Load packages before deserializing state

spec/atom-environment-spec.coffee

@@ -162,7 +162,6 @@ describe "AtomEnvironment", ->
 
       spyOn(atom, 'getLoadSettings').andCallFake -> loadSettings
       spyOn(atom, 'serialize').andReturn({stuff: 'cool'})
-      spyOn(atom, 'deserialize')
 
       atom.project.setPaths([dir1, dir2])
       # State persistence will fail if other Atom instances are running
@@ -172,16 +171,13 @@ describe "AtomEnvironment", ->
 
       waitsForPromise ->
         atom.saveState().then ->
-          atom.loadState()
-
-      runs ->
-        expect(atom.deserialize).not.toHaveBeenCalled()
+          atom.loadState().then (state) ->
+            expect(state).toBeNull()
 
       waitsForPromise ->
         loadSettings.initialPaths = [dir2, dir1]
-        atom.loadState()
-      runs ->
-        expect(atom.deserialize).toHaveBeenCalledWith({stuff: 'cool'})
+        atom.loadState().then (state) ->
+          expect(state).toEqual({stuff: 'cool'})
 
     it "saves state on keydown and mousedown events", ->
       spyOn(atom, 'saveState')

src/application-delegate.coffee

@@ -22,6 +22,12 @@ class ApplicationDelegate
   closeWindow: ->
     ipcRenderer.send("call-window-method", "close")
 
+  getTemporaryWindowState: ->
+    ipcHelpers.call('get-temporary-window-state')
+
+  setTemporaryWindowState: (state) ->
+    ipcHelpers.call('set-temporary-window-state', state)
+
   getWindowSize: ->
     [width, height] = remote.getCurrentWindow().getSize()
     {width, height}

src/atom-environment.coffee

@@ -537,21 +537,16 @@ class AtomEnvironment extends Model
   # Restores the full screen and maximized state after the window has resized to
   # prevent resize glitches.
   displayWindow: ->
-    @restoreWindowDimensions().then (dimensions) =>
+    @restoreWindowDimensions().then =>
       steps = [
         @restoreWindowBackground(),
         @show(),
         @focus()
       ]
       steps.push(@setFullScreen(true)) if @workspace.fullScreen
-      steps.push(@maximize()) if dimensions?.maximized and process.platform isnt 'darwin'
+      steps.push(@maximize()) if @windowDimensions?.maximized and process.platform isnt 'darwin'
       Promise.all(steps)
 
-      if @isFirstLoad()
-        loadSettings = getWindowLoadSettings()
-        loadSettings.firstLoad = false
-        setWindowLoadSettings(loadSettings)
-
   # Get the dimensions of this window.
   #
   # Returns an {Object} with the following keys:
@@ -592,10 +587,10 @@ class AtomEnvironment extends Model
   isValidDimensions: ({x, y, width, height}={}) ->
     width > 0 and height > 0 and x + width > 0 and y + height > 0
 
-  storeDefaultWindowDimensions: ->
-    dimensions = @getWindowDimensions()
-    if @isValidDimensions(dimensions)
-      localStorage.setItem("defaultWindowDimensions", JSON.stringify(dimensions))
+  storeWindowDimensions: ->
+    @windowDimensions = @getWindowDimensions()
+    if @isValidDimensions(@windowDimensions)
+      localStorage.setItem("defaultWindowDimensions", JSON.stringify(@windowDimensions))
 
   getDefaultWindowDimensions: ->
     {windowDimensions} = @getLoadSettings()
@@ -615,21 +610,9 @@ class AtomEnvironment extends Model
       {x: 0, y: 0, width: Math.min(1024, width), height}
 
   restoreWindowDimensions: ->
-    dimensions = null
-
-    # The first time the window's loaded we want to use the default dimensions.
-    # But after that, e.g., when the window's been reloaded, we want to use the
-    # dimensions we've saved for it.
-    if not @isFirstLoad()
-      dimensions = @windowDimensions
-
-    unless @isValidDimensions(dimensions)
-      dimensions = @getDefaultWindowDimensions()
-    @setWindowDimensions(dimensions).then -> dimensions
-
-  storeWindowDimensions: ->
-    dimensions = @getWindowDimensions()
-    @windowDimensions = dimensions if @isValidDimensions(dimensions)
+    unless @windowDimensions? and @isValidDimensions(@windowDimensions)
+      @windowDimensions = @getDefaultWindowDimensions()
+    @setWindowDimensions(@windowDimensions).then -> @windowDimensions
 
   restoreWindowBackground: ->
     if backgroundColor = window.localStorage.getItem('atom:window-background-color')
@@ -647,32 +630,39 @@ class AtomEnvironment extends Model
 
   # Call this method when establishing a real application window.
   startEditorWindow: ->
-    @commandInstaller.installAtomCommand false, (error) ->
-      console.warn error.message if error?
-    @commandInstaller.installApmCommand false, (error) ->
-      console.warn error.message if error?
+    @loadState().then (state) =>
+      @windowDimensions = state?.windowDimensions
+      @displayWindow().then =>
+        @commandInstaller.installAtomCommand false, (error) ->
+          console.warn error.message if error?
+        @commandInstaller.installApmCommand false, (error) ->
+          console.warn error.message if error?
 
-    @disposables.add(@applicationDelegate.onDidOpenLocations(@openLocations.bind(this)))
-    @disposables.add(@applicationDelegate.onApplicationMenuCommand(@dispatchApplicationMenuCommand.bind(this)))
-    @disposables.add(@applicationDelegate.onContextMenuCommand(@dispatchContextMenuCommand.bind(this)))
-    @listenForUpdates()
+        @disposables.add(@applicationDelegate.onDidOpenLocations(@openLocations.bind(this)))
+        @disposables.add(@applicationDelegate.onApplicationMenuCommand(@dispatchApplicationMenuCommand.bind(this)))
+        @disposables.add(@applicationDelegate.onContextMenuCommand(@dispatchContextMenuCommand.bind(this)))
+        @listenForUpdates()
 
-    @registerDefaultTargetForKeymaps()
+        @registerDefaultTargetForKeymaps()
 
-    @packages.loadPackages()
+        @packages.loadPackages()
 
-    @document.body.appendChild(@views.getView(@workspace))
-    @backgroundStylesheet?.remove()
+        startTime = Date.now()
+        @deserialize(state) if state?
+        @deserializeTimings.atom = Date.now() - startTime
 
-    @watchProjectPath()
+        @document.body.appendChild(@views.getView(@workspace))
+        @backgroundStylesheet?.remove()
 
-    @packages.activate()
-    @keymaps.loadUserKeymap()
-    @requireUserInitScript() unless @getLoadSettings().safeMode
+        @watchProjectPath()
 
-    @menu.update()
+        @packages.activate()
+        @keymaps.loadUserKeymap()
+        @requireUserInitScript() unless @getLoadSettings().safeMode
 
-    @openInitialEmptyEditorIfNecessary()
+        @menu.update()
+
+        @openInitialEmptyEditorIfNecessary()
 
   serialize: ->
     version: @constructor.version
@@ -828,30 +818,16 @@ class AtomEnvironment extends Model
     if storageKey = @getStateKey(@project?.getPaths())
       @stateStore.save(storageKey, state)
     else
-      @getCurrentWindow().loadSettings.windowState = JSON.stringify(state)
-      Promise.resolve()
+      @applicationDelegate.setTemporaryWindowState(state)
 
   loadState: ->
-    return Promise.resolve() unless @enablePersistence
-
-    startTime = Date.now()
-
-    statePromise = null
-    if stateKey = @getStateKey(@getLoadSettings().initialPaths)
-      statePromise = @stateStore.load(stateKey)
-
-    if not statePromise? and windowState = @getLoadSettings().windowState
-      try
-        statePromise = Promise.resolve(JSON.parse(@getLoadSettings().windowState))
-      catch error
-        console.warn "Error parsing window state: #{statePath} #{error.stack}", error
-
-    if statePromise?
-      statePromise.then (state) =>
-        @deserializeTimings.atom = Date.now() - startTime
-        @deserialize(state) if state?
+    if @enablePersistence
+      if stateKey = @getStateKey(@getLoadSettings().initialPaths)
+        @stateStore.load(stateKey)
+      else
+        @applicationDelegate.getTemporaryWindowState()
     else
-      Promise.resolve()
+      Promise.resolve(null)
 
   deserialize: (state) ->
     if grammarOverridesByPath = state.grammars?.grammarOverridesByPath
@@ -859,8 +835,6 @@ class AtomEnvironment extends Model
 
     @setFullScreen(state.fullScreen)
 
-    @windowDimensions = state.windowDimensions if state.windowDimensions
-
     @packages.packageStates = state.packageStates ? {}
 
     startTime = Date.now()

src/browser/atom-application.coffee

@@ -280,6 +280,12 @@ class AtomApplication
     ipcHelpers.respondTo 'hide-window', (win) ->
       win.hide()
 
+    ipcHelpers.respondTo 'get-temporary-window-state', (win) ->
+      win.temporaryState
+
+    ipcHelpers.respondTo 'set-temporary-window-state', (win, state) ->
+      win.temporaryState = state
+
     ipcMain.on 'did-cancel-window-unload', =>
       @quitting = false
 
@@ -528,7 +534,7 @@ class AtomApplication
       if pack.urlMain
         packagePath = @packages.resolvePackagePath(packageName)
         windowInitializationScript = path.resolve(packagePath, pack.urlMain)
-        windowDimensions = @focusedWindow()?.getDimensions()
+        windowDimensions = @getDimensionsForNewWindow()
         new AtomWindow({windowInitializationScript, @resourcePath, devMode, safeMode, urlToOpen, windowDimensions})
       else
         console.log "Package '#{pack.name}' does not have a url main: #{urlToOpen}"

src/browser/atom-window.coffee

@@ -41,13 +41,11 @@ class AtomWindow
     @handleEvents()
 
     loadSettings = _.extend({}, settings)
-    loadSettings.windowState ?= '{}'
     loadSettings.appVersion = app.getVersion()
     loadSettings.resourcePath = @resourcePath
     loadSettings.devMode ?= false
     loadSettings.safeMode ?= false
     loadSettings.atomHome = process.env.ATOM_HOME
-    loadSettings.firstLoad = true
     loadSettings.clearWindowState ?= false
 
     # Only send to the first non-spec window created
@@ -64,23 +62,18 @@ class AtomWindow
 
     loadSettings.initialPaths.sort()
 
-    @browserWindow.loadSettings = loadSettings
     @browserWindow.once 'window:loaded', =>
       @emit 'window:loaded'
       @loaded = true
 
     @setLoadSettings(loadSettings)
     @browserWindow.focusOnWebView() if @isSpec
+    @browserWindow.temporaryState = {windowDimensions} if windowDimensions?
 
     hasPathToOpen = not (locationsToOpen.length is 1 and not locationsToOpen[0].pathToOpen?)
     @openLocations(locationsToOpen) if hasPathToOpen and not @isSpecWindow()
 
-  setLoadSettings: (loadSettingsObj) ->
-    # Ignore the windowState when passing loadSettings via URL, since it could
-    # be quite large.
-    loadSettings = _.clone(loadSettingsObj)
-    delete loadSettings['windowState']
-
+  setLoadSettings: (loadSettings) ->
     @browserWindow.loadURL url.format
       protocol: 'file'
       pathname: "#{@resourcePath}/static/index.html"

src/initialize-application-window.coffee

@@ -23,12 +23,10 @@ module.exports = ({blobStore}) ->
     enablePersistence: true
   })
 
-  atom.loadState().then ->
-    atom.displayWindow().then ->
-      atom.startEditorWindow()
+  atom.startEditorWindow().then ->
 
-      # Workaround for focus getting cleared upon window creation
-      windowFocused = ->
-        window.removeEventListener('focus', windowFocused)
-        setTimeout (-> document.querySelector('atom-workspace').focus()), 0
-      window.addEventListener('focus', windowFocused)
+    # Workaround for focus getting cleared upon window creation
+    windowFocused = ->
+      window.removeEventListener('focus', windowFocused)
+      setTimeout (-> document.querySelector('atom-workspace').focus()), 0
+    window.addEventListener('focus', windowFocused)

src/window-event-handler.coffee

@@ -133,7 +133,7 @@ class WindowEventHandler
 
   handleWindowBlur: =>
     @document.body.classList.add('is-blurred')
-    @atomEnvironment.storeDefaultWindowDimensions()
+    @atomEnvironment.storeWindowDimensions()
 
   handleWindowBeforeunload: =>
     confirmed = @atomEnvironment.workspace?.confirmClose(windowCloseRequested: true)
@@ -141,8 +141,8 @@ class WindowEventHandler
       @atomEnvironment.hide()
     @reloadRequested = false
 
-    @atomEnvironment.storeDefaultWindowDimensions()
     @atomEnvironment.storeWindowDimensions()
+    @atomEnvironment.saveState()
     if confirmed
       @atomEnvironment.unloadEditorWindow()
     else

src/window-load-settings-helpers.coffee

@@ -5,15 +5,6 @@ windowLoadSettings = null
 
 exports.getWindowLoadSettings = ->
   windowLoadSettings ?= JSON.parse(window.decodeURIComponent(window.location.hash.substr(1)))
-  clone = _.deepClone(windowLoadSettings)
-
-  # The windowLoadSettings.windowState could be large, request it only when needed.
-  clone.__defineGetter__ 'windowState', ->
-    remote.getCurrentWindow().loadSettings.windowState
-  clone.__defineSetter__ 'windowState', (value) ->
-    remote.getCurrentWindow().loadSettings.windowState = value
-
-  clone
 
 exports.setWindowLoadSettings = (settings) ->
   windowLoadSettings = settings