From 899ce2a27a38d3039d9c2e36a742002fabac105b Mon Sep 17 00:00:00 2001 From: Ben Gotow <bengotow@gmail.com> Date: Thu, 24 Sep 2015 10:40:38 -0700 Subject: [PATCH] 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 | 10 +-- internal_packages/composer/lib/main.cjsx | 2 +- internal_packages/events/lib/main.cjsx | 6 +- .../notification-mailto/lib/main.coffee | 2 +- internal_packages/undo-redo/lib/main.cjsx | 6 +- spec/deserializer-manager-spec.coffee | 44 ---------- spec/window-spec.coffee | 4 +- src/atom.coffee | 85 +++++-------------- src/browser/application.coffee | 2 +- src/browser/atom-window.coffee | 14 ++- src/deserializer-manager.coffee | 67 --------------- src/window-event-handler.coffee | 15 ++-- 12 files changed, 46 insertions(+), 211 deletions(-) delete mode 100644 spec/deserializer-manager-spec.coffee delete mode 100644 src/deserializer-manager.coffee diff --git a/internal_packages/attachments/lib/main.cjsx b/internal_packages/attachments/lib/main.cjsx index 73cd05d0d..cf736ad11 100644 --- a/internal_packages/attachments/lib/main.cjsx +++ b/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 ImageAttachmentComponent + ComponentRegistry.unregister(AttachmentComponent) + ComponentRegistry.unregister(ImageAttachmentComponent) serialize: -> @state diff --git a/internal_packages/composer/lib/main.cjsx b/internal_packages/composer/lib/main.cjsx index 45467b0f5..5b30b946f 100644 --- a/internal_packages/composer/lib/main.cjsx +++ b/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"> diff --git a/internal_packages/events/lib/main.cjsx b/internal_packages/events/lib/main.cjsx index e82bcc661..1937d517f 100644 --- a/internal_packages/events/lib/main.cjsx +++ b/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 diff --git a/internal_packages/notification-mailto/lib/main.coffee b/internal_packages/notification-mailto/lib/main.coffee index 7402a6b4b..91b833518 100644 --- a/internal_packages/notification-mailto/lib/main.coffee +++ b/internal_packages/notification-mailto/lib/main.coffee @@ -33,7 +33,7 @@ module.exports = }] deactivate: -> - @_unlisten() + @_unlisten?() serialize: -> @state diff --git a/internal_packages/undo-redo/lib/main.cjsx b/internal_packages/undo-redo/lib/main.cjsx index 2440b708d..3da233e81 100644 --- a/internal_packages/undo-redo/lib/main.cjsx +++ b/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 diff --git a/spec/deserializer-manager-spec.coffee b/spec/deserializer-manager-spec.coffee deleted file mode 100644 index 15c8a2648..000000000 --- a/spec/deserializer-manager-spec.coffee +++ /dev/null @@ -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() diff --git a/spec/window-spec.coffee b/spec/window-spec.coffee index d7a99b46f..fdeefc0a7 100644 --- a/spec/window-spec.coffee +++ b/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 diff --git a/src/atom.coffee b/src/atom.coffee index b4facdca3..a13419a96 100644 --- a/src/atom.coffee +++ b/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 - {initialPath} = @getLoadSettings() - if initialPath - sha1 = crypto.createHash('sha1').update(initialPath).digest('hex') - filename = "application-#{sha1}" - - if filename - path.join(@getStorageDirPath(), filename) + else if mainWindow + path.join(@getConfigDirPath(), 'main-window-state.json') 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() - [width, height] = browserWindow.getSize() + {x, y, width, height} = browserWindow.getBounds() 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() - return windowDimensions if windowDimensions? - - 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} + 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() diff --git a/src/browser/application.coffee b/src/browser/application.coffee index bea423dcf..dd9fa5dc2 100644 --- a/src/browser/application.coffee +++ b/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() diff --git a/src/browser/atom-window.coffee b/src/browser/atom-window.coffee index b6c8d4f59..606005661 100644 --- a/src/browser/atom-window.coffee +++ b/src/browser/atom-window.coffee @@ -99,20 +99,16 @@ class AtomWindow @browserWindow.once 'window:loaded', => @emit 'window:loaded' @loaded = true - if @browserWindow.loadSettingsChangedSinceGetURL - @browserWindow.webContents.send('load-settings-changed', @browserWindow.loadSettings) - - @browserWindow.once 'window:main-window-content-loaded', => - @emit 'window:main-window-content-loaded' - @mainWindowContentLoaded = true - @browserWindow.setResizable(true) + if @mainWindow + @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, diff --git a/src/deserializer-manager.coffee b/src/deserializer-manager.coffee deleted file mode 100644 index 1f5b4ad81..000000000 --- a/src/deserializer-manager.coffee +++ /dev/null @@ -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] diff --git a/src/window-event-handler.coffee b/src/window-event-handler.coffee index 64edd231e..3c717aa6a 100644 --- a/src/window-event-handler.coffee +++ b/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() - atom.hide() if confirmed and not @reloadRequested and atom.getCurrentWindow().isWebViewFocused() + if atom.getCurrentWindow().isWebViewFocused() and not @reloadRequested + atom.hide() @reloadRequested = false - - atom.storeDefaultWindowDimensions() atom.storeWindowDimensions() - atom.unloadEditorWindow() if confirmed + atom.saveStateAndUnloadWindow() + true - confirmed - - @subscribe $(window), 'unload', -> atom.removeEditorWindow() + @subscribe $(window), 'unload', => + atom.windowEventHandler?.unsubscribe() @subscribeToCommand $(window), 'window:toggle-full-screen', -> atom.toggleFullScreen()