From c6cae5b8fdfea982084b068a0b38a7c988c52f51 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 7 Feb 2017 13:15:27 -0700 Subject: [PATCH] Store represented directory paths directly on AtomWindow in main process Fixes #13729 Previously, when adding a window, we were unable to read its current project paths out of the hash of the URL during window initialization because the window still considered itself to be loading. Rather than fixing this issue, we decided to completely eliminate the sharing of state between processes in the window.location and instead switch to cached synchronous RPC for the loadSettings and a dedicated RPC-based mechanism for the project paths. --- spec/main-process/atom-application.test.js | 2 +- src/application-delegate.coffee | 19 ++++----- src/atom-environment.coffee | 3 +- src/get-window-load-settings.js | 10 +++++ src/initialize-application-window.coffee | 2 +- src/initialize-benchmark-window.js | 2 +- src/initialize-test-window.coffee | 2 +- src/main-process/atom-application.coffee | 3 +- src/main-process/atom-window.coffee | 46 ++++++++++++---------- src/window-load-settings-helpers.coffee | 8 ---- static/index.js | 44 +++++++-------------- 11 files changed, 63 insertions(+), 78 deletions(-) create mode 100644 src/get-window-load-settings.js delete mode 100644 src/window-load-settings-helpers.coffee diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index 77d7987a7..88fce27a4 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -260,7 +260,7 @@ describe('AtomApplication', function () { }) assert.equal(window1EditorTitle, 'untitled') - const window2 = atomApplication.launch(parseCommandLine([])) + const window2 = atomApplication.openWithOptions(parseCommandLine([])) await focusWindow(window2) const window2EditorTitle = await evalInWebContents(window1.browserWindow.webContents, function (sendBackToMainProcess) { sendBackToMainProcess(atom.workspace.getActiveTextEditor().getTitle()) diff --git a/src/application-delegate.coffee b/src/application-delegate.coffee index 185db5059..766ba7aa8 100644 --- a/src/application-delegate.coffee +++ b/src/application-delegate.coffee @@ -2,10 +2,12 @@ _ = require 'underscore-plus' {screen, ipcRenderer, remote, shell, webFrame} = require 'electron' ipcHelpers = require './ipc-helpers' {Disposable} = require 'event-kit' -{getWindowLoadSettings, setWindowLoadSettings} = require './window-load-settings-helpers' +getWindowLoadSettings = require './get-window-load-settings' module.exports = class ApplicationDelegate + getWindowLoadSettings: -> getWindowLoadSettings() + open: (params) -> ipcRenderer.send('open', params) @@ -109,10 +111,7 @@ class ApplicationDelegate ipcRenderer.send("add-recent-document", filename) setRepresentedDirectoryPaths: (paths) -> - loadSettings = getWindowLoadSettings() - loadSettings['initialPaths'] = paths - setWindowLoadSettings(loadSettings) - ipcRenderer.send("did-change-paths") + ipcHelpers.call('window-method', 'setRepresentedDirectoryPaths', paths) setAutoHideWindowMenuBar: (autoHide) -> ipcHelpers.call('window-method', 'setAutoHideMenuBar', autoHide) @@ -149,13 +148,9 @@ class ApplicationDelegate showMessageDialog: (params) -> showSaveDialog: (params) -> - if _.isString(params) - params = defaultPath: params - else - params = _.clone(params) - params.title ?= 'Save File' - params.defaultPath ?= getWindowLoadSettings().initialPaths[0] - remote.dialog.showSaveDialog remote.getCurrentWindow(), params + if typeof params is 'string' + params = {defaultPath: params} + @getCurrentWindow().showSaveDialog(params) playBeepSound: -> shell.beep() diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee index a740b22d5..2c637b0d6 100644 --- a/src/atom-environment.coffee +++ b/src/atom-environment.coffee @@ -11,7 +11,6 @@ Model = require './model' WindowEventHandler = require './window-event-handler' StateStore = require './state-store' StorageFolder = require './storage-folder' -{getWindowLoadSettings} = require './window-load-settings-helpers' registerDefaultCommands = require './register-default-commands' {updateProcessEnv} = require './update-process-env' @@ -458,7 +457,7 @@ class AtomEnvironment extends Model # # Returns an {Object} containing all the load setting key/value pairs. getLoadSettings: -> - getWindowLoadSettings() + @applicationDelegate.getWindowLoadSettings() ### Section: Managing The Atom Window diff --git a/src/get-window-load-settings.js b/src/get-window-load-settings.js new file mode 100644 index 000000000..7ee465141 --- /dev/null +++ b/src/get-window-load-settings.js @@ -0,0 +1,10 @@ +const {remote} = require('electron') + +let windowLoadSettings = null + +module.exports = () => { + if (!windowLoadSettings) { + windowLoadSettings = remote.getCurrentWindow().loadSettings + } + return windowLoadSettings +} diff --git a/src/initialize-application-window.coffee b/src/initialize-application-window.coffee index 7d3a23db7..be13ce6c6 100644 --- a/src/initialize-application-window.coffee +++ b/src/initialize-application-window.coffee @@ -3,7 +3,7 @@ module.exports = ({blobStore}) -> {updateProcessEnv} = require('./update-process-env') path = require 'path' require './window' - {getWindowLoadSettings} = require './window-load-settings-helpers' + getWindowLoadSettings = require './get-window-load-settings' {ipcRenderer} = require 'electron' {resourcePath, devMode, env} = getWindowLoadSettings() require './electron-shims' diff --git a/src/initialize-benchmark-window.js b/src/initialize-benchmark-window.js index 29a210904..a223e0b03 100644 --- a/src/initialize-benchmark-window.js +++ b/src/initialize-benchmark-window.js @@ -6,7 +6,7 @@ import ipcHelpers from './ipc-helpers' import util from 'util' export default async function () { - const {getWindowLoadSettings} = require('./window-load-settings-helpers') + const getWindowLoadSettings = require('./get-window-load-settings') const {test, headless, resourcePath, benchmarkPaths} = getWindowLoadSettings() try { const Clipboard = require('../src/clipboard') diff --git a/src/initialize-test-window.coffee b/src/initialize-test-window.coffee index 39a408fea..794db3174 100644 --- a/src/initialize-test-window.coffee +++ b/src/initialize-test-window.coffee @@ -18,7 +18,7 @@ module.exports = ({blobStore}) -> try path = require 'path' {ipcRenderer} = require 'electron' - {getWindowLoadSettings} = require './window-load-settings-helpers' + getWindowLoadSettings = require './get-window-load-settings' CompileCache = require './compile-cache' AtomEnvironment = require '../src/atom-environment' ApplicationDelegate = require '../src/application-delegate' diff --git a/src/main-process/atom-application.coffee b/src/main-process/atom-application.coffee index 42358409b..e2515ccb9 100644 --- a/src/main-process/atom-application.coffee +++ b/src/main-process/atom-application.coffee @@ -587,8 +587,7 @@ class AtomApplication states = [] for window in @windows unless window.isSpec - if loadSettings = window.getLoadSettings() - states.push(initialPaths: loadSettings.initialPaths) + states.push({initialPaths: window.representedDirectoryPaths}) if states.length > 0 or allowEmpty @storageFolder.storeSync('application.json', states) diff --git a/src/main-process/atom-window.coffee b/src/main-process/atom-window.coffee index 9c937e4f6..7f786b12d 100644 --- a/src/main-process/atom-window.coffee +++ b/src/main-process/atom-window.coffee @@ -46,9 +46,7 @@ class AtomWindow if @shouldHideTitleBar() options.titleBarStyle = 'hidden' - @browserWindow = new BrowserWindow options - @atomApplication.addWindow(this) - + @browserWindow = new BrowserWindow(options) @handleEvents() loadSettings = Object.assign({}, settings) @@ -64,7 +62,6 @@ class AtomWindow path.dirname(pathToOpen) else pathToOpen - loadSettings.initialPaths.sort() # Only send to the first non-spec window created @@ -72,33 +69,31 @@ class AtomWindow @constructor.includeShellLoadTime = false loadSettings.shellLoadTime ?= Date.now() - global.shellStartTime + @representedDirectoryPaths = loadSettings.initialPaths + @env = loadSettings.env if loadSettings.env? + @browserWindow.loadSettings = loadSettings @browserWindow.on 'window:loaded', => @emit 'window:loaded' @resolveLoadedPromise() - @setLoadSettings(loadSettings) - @env = loadSettings.env if loadSettings.env? + @browserWindow.loadURL url.format + protocol: 'file' + pathname: "#{@resourcePath}/static/index.html" + slashes: true + + @browserWindow.showSaveDialog = @showSaveDialog.bind(this) + @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() + + @atomApplication.addWindow(this) - setLoadSettings: (loadSettings) -> - @browserWindow.loadURL url.format - protocol: 'file' - pathname: "#{@resourcePath}/static/index.html" - slashes: true - hash: encodeURIComponent(JSON.stringify(loadSettings)) - - getLoadSettings: -> - if @browserWindow.webContents? and not @browserWindow.webContents.isLoading() - hash = url.parse(@browserWindow.webContents.getURL()).hash.substr(1) - JSON.parse(decodeURIComponent(hash)) - - hasProjectPath: -> @getLoadSettings().initialPaths?.length > 0 + hasProjectPath: -> @representedDirectoryPaths.length > 0 setupContextMenu: -> ContextMenu = require './context-menu' @@ -112,7 +107,7 @@ class AtomWindow true containsPath: (pathToCheck) -> - @getLoadSettings()?.initialPaths?.some (projectPath) -> + @representedDirectoryPaths.some (projectPath) -> if not projectPath false else if not pathToCheck @@ -265,6 +260,13 @@ class AtomWindow @saveState().then => @browserWindow.reload() @loadedPromise + showSaveDialog: (params) -> + params = Object.assign({ + title: 'Save File', + defaultPath: @representedDirectoryPaths[0] + }, params) + dialog.showSaveDialog(this, params) + toggleDevTools: -> @browserWindow.toggleDevTools() openDevTools: -> @browserWindow.openDevTools() @@ -275,4 +277,8 @@ class AtomWindow setRepresentedFilename: (representedFilename) -> @browserWindow.setRepresentedFilename(representedFilename) + setRepresentedDirectoryPaths: (@representedDirectoryPaths) -> + @representedDirectoryPaths.sort() + @atomApplication.saveState() + copy: -> @browserWindow.copy() diff --git a/src/window-load-settings-helpers.coffee b/src/window-load-settings-helpers.coffee deleted file mode 100644 index 639dc751d..000000000 --- a/src/window-load-settings-helpers.coffee +++ /dev/null @@ -1,8 +0,0 @@ -windowLoadSettings = null - -exports.getWindowLoadSettings = -> - windowLoadSettings ?= JSON.parse(window.decodeURIComponent(window.location.hash.substr(1))) - -exports.setWindowLoadSettings = (settings) -> - windowLoadSettings = settings - location.hash = encodeURIComponent(JSON.stringify(settings)) diff --git a/static/index.js b/static/index.js index 2966eafdf..aa57a594a 100644 --- a/static/index.js +++ b/static/index.js @@ -2,9 +2,8 @@ var path = require('path') var FileSystemBlobStore = require('../src/file-system-blob-store') var NativeCompileCache = require('../src/native-compile-cache') + var getWindowLoadSettings = require('../src/get-window-load-settings') - var loadSettings = null - var loadSettingsError = null var blobStore = null window.onload = function () { @@ -25,20 +24,16 @@ // Normalize to make sure drive letter case is consistent on Windows process.resourcesPath = path.normalize(process.resourcesPath) - if (loadSettingsError) { - throw loadSettingsError - } - - var devMode = loadSettings.devMode || !loadSettings.resourcePath.startsWith(process.resourcesPath + path.sep) + var devMode = getWindowLoadSettings().devMode || !getWindowLoadSettings().resourcePath.startsWith(process.resourcesPath + path.sep) if (devMode) { setupDeprecatedPackages() } - if (loadSettings.profileStartup) { - profileStartup(loadSettings, Date.now() - startTime) + if (getWindowLoadSettings().profileStartup) { + profileStartup(Date.now() - startTime) } else { - setupWindow(loadSettings) + setupWindow() setLoadTime(Date.now() - startTime) } } catch (error) { @@ -61,23 +56,23 @@ console.error(error.stack || error) } - function setupWindow (loadSettings) { + function setupWindow () { var CompileCache = require('../src/compile-cache') CompileCache.setAtomHomeDirectory(process.env.ATOM_HOME) var ModuleCache = require('../src/module-cache') - ModuleCache.register(loadSettings) - ModuleCache.add(loadSettings.resourcePath) + ModuleCache.register(getWindowLoadSettings()) + ModuleCache.add(getWindowLoadSettings().resourcePath) // By explicitly passing the app version here, we could save the call // of "require('remote').require('app').getVersion()". var startCrashReporter = require('../src/crash-reporter-start') - startCrashReporter({_version: loadSettings.appVersion}) + startCrashReporter({_version: getWindowLoadSettings().appVersion}) setupVmCompatibility() setupCsonCache(CompileCache.getCacheDirectory()) - var initialize = require(loadSettings.windowInitializationScript) + var initialize = require(getWindowLoadSettings().windowInitializationScript) return initialize({blobStore: blobStore}).then(function () { require('electron').ipcRenderer.send('window-command', 'window:loaded') }) @@ -105,11 +100,11 @@ } } - function profileStartup (loadSettings, initialTime) { + function profileStartup (initialTime) { function profile () { console.profile('startup') var startTime = Date.now() - setupWindow(loadSettings).then(function () { + setupWindow().then(function () { setLoadTime(Date.now() - startTime + initialTime) console.profileEnd('startup') console.log('Switch to the Profiles tab to view the created startup profile') @@ -125,16 +120,6 @@ } } - function parseLoadSettings () { - var rawLoadSettings = decodeURIComponent(window.location.hash.substr(1)) - try { - loadSettings = JSON.parse(rawLoadSettings) - } catch (error) { - console.error('Failed to parse load settings: ' + rawLoadSettings) - loadSettingsError = error - } - } - var setupAtomHome = function () { if (process.env.ATOM_HOME) { return @@ -143,11 +128,10 @@ // Ensure ATOM_HOME is always set before anything else is required // This is because of a difference in Linux not inherited between browser and render processes // https://github.com/atom/atom/issues/5412 - if (loadSettings && loadSettings.atomHome) { - process.env.ATOM_HOME = loadSettings.atomHome + if (getWindowLoadSettings() && getWindowLoadSettings().atomHome) { + process.env.ATOM_HOME = getWindowLoadSettings().atomHome } } - parseLoadSettings() setupAtomHome() })()