fix(config): Apply config change from Atom, process locking

Summary:
d1c44dcb54

Also lock config across processes to prevent the user from being logged out when the main process reads config before it's finished writing. This is what is causing the login window to appear.

Test Plan: Rapidly tap between the two display modes. Note that it no longer reverts to the wrong one intermittently.

Reviewers: evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D1931
This commit is contained in:
Ben Gotow 2015-08-25 16:00:09 -07:00
parent 30ba6e6023
commit bb887a4ae8
4 changed files with 66 additions and 9 deletions

View file

@ -486,6 +486,7 @@ describe "Config", ->
atom.config.set('foo.bar.baz', "value 1") atom.config.set('foo.bar.baz', "value 1")
expect(observeHandler).toHaveBeenCalledWith("value 1") expect(observeHandler).toHaveBeenCalledWith("value 1")
advanceClock(100) # complete pending save that was requested in ::set
observeHandler.reset() observeHandler.reset()
atom.config.loadUserConfig() atom.config.loadUserConfig()
@ -772,6 +773,22 @@ describe "Config", ->
expect(warnSpy).toHaveBeenCalled() expect(warnSpy).toHaveBeenCalled()
expect(warnSpy.mostRecentCall.args[0]).toContain "foo.int" expect(warnSpy.mostRecentCall.args[0]).toContain "foo.int"
describe "when there is a pending save", ->
it "does not change the config settings", ->
fs.writeFileSync atom.config.configFilePath, "'*': foo: bar: 'baz'"
atom.config.set("foo.bar", "quux")
atom.config.loadUserConfig()
expect(atom.config.get("foo.bar")).toBe "quux"
advanceClock(100)
expect(atom.config.save.callCount).toBe 1
expect(atom.config.get("foo.bar")).toBe "quux"
atom.config.loadUserConfig()
expect(atom.config.get("foo.bar")).toBe "baz"
describe ".observeUserConfig()", -> describe ".observeUserConfig()", ->
updatedHandler = null updatedHandler = null

View file

@ -1,10 +1,10 @@
Config = require '../config'
AtomWindow = require './atom-window' AtomWindow = require './atom-window'
BrowserWindow = require 'browser-window' BrowserWindow = require 'browser-window'
WindowManager = require './window-manager' WindowManager = require './window-manager'
ApplicationMenu = require './application-menu' ApplicationMenu = require './application-menu'
AutoUpdateManager = require './auto-update-manager' AutoUpdateManager = require './auto-update-manager'
NylasProtocolHandler = require './nylas-protocol-handler' NylasProtocolHandler = require './nylas-protocol-handler'
SharedFileManager = require './shared-file-manager'
_ = require 'underscore' _ = require 'underscore'
fs = require 'fs-plus' fs = require 'fs-plus'
@ -73,13 +73,16 @@ class Application
global.application = this global.application = this
@sharedFileManager = new SharedFileManager()
@nylasProtocolHandler = new NylasProtocolHandler(@resourcePath, @safeMode)
Config = require '../config'
@config = new Config({configDirPath, @resourcePath}) @config = new Config({configDirPath, @resourcePath})
@config.load() @config.load()
@windowManager = new WindowManager({@resourcePath, @config, @devMode, @safeMode}) @windowManager = new WindowManager({@resourcePath, @config, @devMode, @safeMode})
@autoUpdateManager = new AutoUpdateManager(@version, @config, @specMode) @autoUpdateManager = new AutoUpdateManager(@version, @config, @specMode)
@applicationMenu = new ApplicationMenu(@version) @applicationMenu = new ApplicationMenu(@version)
@nylasProtocolHandler = new NylasProtocolHandler(@resourcePath, @safeMode)
@_databasePhase = 'setup' @_databasePhase = 'setup'
@listenForArgumentsFromNewProcess() @listenForArgumentsFromNewProcess()

View file

@ -0,0 +1,15 @@
class SharedFileManager
constructor: ->
@_inflight = {}
processWillWriteFile: (filePath) ->
@_inflight[filePath] += 1
processDidWriteFile: (filePath) ->
@_inflight[filePath] -= 1
processCanReadFile: (filePath) ->
!@_inflight[filePath] or @_inflight[filePath] is 0
module.exports = SharedFileManager

View file

@ -13,6 +13,11 @@ Color = require './color'
ScopedPropertyStore = require 'scoped-property-store' ScopedPropertyStore = require 'scoped-property-store'
ScopeDescriptor = require './scope-descriptor' ScopeDescriptor = require './scope-descriptor'
if global.application
app = global.application
else
app = require('remote').getGlobal('application')
# Essential: Used to access all of Atom's configuration details. # Essential: Used to access all of Atom's configuration details.
# #
# An instance of this class is always available as the `atom.config` global. # An instance of this class is always available as the `atom.config` global.
@ -334,9 +339,17 @@ class Config
@configFilePath = fs.resolve(@configDirPath, 'config', ['json', 'cson']) @configFilePath = fs.resolve(@configDirPath, 'config', ['json', 'cson'])
@configFilePath ?= path.join(@configDirPath, 'config.cson') @configFilePath ?= path.join(@configDirPath, 'config.cson')
@transactDepth = 0 @transactDepth = 0
@savePending = false
@debouncedSave = _.debounce(@save, 100) @requestLoad = _.debounce(@loadUserConfig, 100)
@debouncedLoad = _.debounce(@loadUserConfig, 100) @debouncedLoad = _.debounce(@loadUserConfig, 100)
@requestSave = =>
@savePending = true
debouncedSave.call(this)
save = =>
@savePending = false
@save()
debouncedSave = _.debounce(save, 100)
### ###
Section: Config Subscription Section: Config Subscription
@ -607,7 +620,7 @@ class Config
else else
@setRawValue(keyPath, value) @setRawValue(keyPath, value)
@debouncedSave() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors @requestSave() if source is @getUserConfigPath() and shouldSave and not @configFileHasErrors
true true
# Essential: Restore the setting at `keyPath` to its default value. # Essential: Restore the setting at `keyPath` to its default value.
@ -637,7 +650,7 @@ class Config
_.setValueForKeyPath(settings, keyPath, undefined) _.setValueForKeyPath(settings, keyPath, undefined)
settings = withoutEmptyObjects(settings) settings = withoutEmptyObjects(settings)
@set(null, settings, {scopeSelector, source, priority: @priorityForSource(source)}) if settings? @set(null, settings, {scopeSelector, source, priority: @priorityForSource(source)}) if settings?
@debouncedSave() @requestSave()
else else
@scopedSettingsStore.removePropertiesForSourceAndSelector(source, scopeSelector) @scopedSettingsStore.removePropertiesForSourceAndSelector(source, scopeSelector)
@emitChangeEvent() @emitChangeEvent()
@ -816,14 +829,20 @@ class Config
fs.copySync(templateConfigDirPath, @configDirPath) fs.copySync(templateConfigDirPath, @configDirPath)
loadUserConfig: -> loadUserConfig: ->
manager = app.sharedFileManager
if not manager.processCanReadFile(@configFilePath)
@requestLoad()
return
unless fs.existsSync(@configFilePath) unless fs.existsSync(@configFilePath)
fs.makeTreeSync(path.dirname(@configFilePath)) fs.makeTreeSync(path.dirname(@configFilePath))
CSON.writeFileSync(@configFilePath, {}) CSON.writeFileSync(@configFilePath, {})
try try
userConfig = CSON.readFileSync(@configFilePath) unless @savePending
@resetUserSettings(userConfig) userConfig = CSON.readFileSync(@configFilePath)
@configFileHasErrors = false @resetUserSettings(userConfig)
@configFileHasErrors = false
catch error catch error
@configFileHasErrors = true @configFileHasErrors = true
message = "Failed to load `#{path.basename(@configFilePath)}`" message = "Failed to load `#{path.basename(@configFilePath)}`"
@ -840,7 +859,7 @@ class Config
observeUserConfig: -> observeUserConfig: ->
try try
@watchSubscription ?= pathWatcher.watch @configFilePath, (eventType) => @watchSubscription ?= pathWatcher.watch @configFilePath, (eventType) =>
@debouncedLoad() if eventType is 'change' and @watchSubscription? @requestLoad() if eventType is 'change' and @watchSubscription?
catch error catch error
@notifyFailure """ @notifyFailure """
Unable to watch path: `#{path.basename(@configFilePath)}`. Make sure you have permissions to Unable to watch path: `#{path.basename(@configFilePath)}`. Make sure you have permissions to
@ -857,9 +876,12 @@ class Config
console.log(errorMessage, detail) console.log(errorMessage, detail)
save: -> save: ->
manager = app.sharedFileManager
manager.processWillWriteFile(@configFilePath)
allSettings = {'*': @settings} allSettings = {'*': @settings}
allSettings = _.extend allSettings, @scopedSettingsStore.propertiesForSource(@getUserConfigPath()) allSettings = _.extend allSettings, @scopedSettingsStore.propertiesForSource(@getUserConfigPath())
CSON.writeFileSync(@configFilePath, allSettings) CSON.writeFileSync(@configFilePath, allSettings)
manager.processDidWriteFile(@configFilePath)
### ###
Section: Private methods managing global settings Section: Private methods managing global settings