From 63c0095efb2133484c6c87c0b37611384a99bce4 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Dec 2015 16:45:49 -0800 Subject: [PATCH 01/10] Emit process exit event with app exit code --- atom/browser/api/atom_api_app.cc | 2 +- atom/browser/api/lib/app.coffee | 12 ++++++++++++ atom/browser/lib/init.coffee | 6 ------ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 697d6eca6aab..0c3623583c62 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -344,7 +344,7 @@ mate::ObjectTemplateBuilder App::GetObjectTemplateBuilder( auto browser = base::Unretained(Browser::Get()); return mate::ObjectTemplateBuilder(isolate) .SetMethod("quit", base::Bind(&Browser::Quit, browser)) - .SetMethod("exit", base::Bind(&Browser::Exit, browser)) + .SetMethod("_exit", base::Bind(&Browser::Exit, browser)) .SetMethod("focus", base::Bind(&Browser::Focus, browser)) .SetMethod("getVersion", base::Bind(&Browser::GetVersion, browser)) .SetMethod("setVersion", base::Bind(&Browser::SetVersion, browser)) diff --git a/atom/browser/api/lib/app.coffee b/atom/browser/api/lib/app.coffee index d0ec41c4d23c..fc0a78298c4d 100644 --- a/atom/browser/api/lib/app.coffee +++ b/atom/browser/api/lib/app.coffee @@ -34,6 +34,18 @@ app.setAppPath = (path) -> app.getAppPath = -> appPath +appExitCode = undefined +app.exit = (exitCode) -> + appExitCode = exitCode + app._exit(exitCode) + +# Map process.exit to app.exit, which quits gracefully. +process.exit = app.exit + +# Emit a process 'exit' event on app quit. +app.on 'quit', -> + process.emit 'exit', appExitCode + # Routes the events to webContents. for name in ['login', 'certificate-error', 'select-client-certificate'] do (name) -> diff --git a/atom/browser/lib/init.coffee b/atom/browser/lib/init.coffee index 85faf0f038ad..9487849e5e69 100644 --- a/atom/browser/lib/init.coffee +++ b/atom/browser/lib/init.coffee @@ -51,13 +51,7 @@ process.on 'uncaughtException', (error) -> message = "Uncaught Exception:\n#{stack}" dialog.showErrorBox 'A JavaScript error occurred in the main process', message -# Emit 'exit' event on quit. {app} = require 'electron' -app.on 'quit', -> - process.emit 'exit' - -# Map process.exit to app.exit, which quits gracefully. -process.exit = app.exit # Load the RPC server. require './rpc-server' From aa82eddca8500a6d3fae9b7dbbcc6bfcd6895ddc Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 7 Dec 2015 17:05:08 -0800 Subject: [PATCH 02/10] Add spec for exit code on event --- spec/api-app-spec.coffee | 19 +++++++++++++++++++ spec/fixtures/api/quit-app/main.js | 9 +++++++++ spec/fixtures/api/quit-app/package.json | 4 ++++ 3 files changed, 32 insertions(+) create mode 100644 spec/fixtures/api/quit-app/main.js create mode 100644 spec/fixtures/api/quit-app/package.json diff --git a/spec/api-app-spec.coffee b/spec/api-app-spec.coffee index 490727488d99..adb64d363eb1 100644 --- a/spec/api-app-spec.coffee +++ b/spec/api-app-spec.coffee @@ -1,4 +1,6 @@ assert = require 'assert' +ChildProcess = require 'child_process' +path = require 'path' {remote} = require 'electron' {app, BrowserWindow} = remote.require 'electron' @@ -29,6 +31,23 @@ describe 'app module', -> it 'should not be empty', -> assert.notEqual app.getLocale(), '' + describe 'app.exit(exitCode)', -> + appProcess = null + afterEach -> + appProcess?.kill() + + it 'emits a process exit event with the code', (done) -> + appPath = path.join(__dirname, 'fixtures', 'api', 'quit-app') + electronPath = remote.getGlobal('process').execPath + appProcess = ChildProcess.spawn(electronPath, [appPath]) + + output = '' + appProcess.stdout.on 'data', (data) -> output += data + appProcess.on 'close', (code) -> + assert.notEqual output.indexOf('Exit event with code: 123'), -1 + assert.equal code, 123 + done() + describe 'BrowserWindow events', -> w = null afterEach -> diff --git a/spec/fixtures/api/quit-app/main.js b/spec/fixtures/api/quit-app/main.js new file mode 100644 index 000000000000..4bdeb93a5e30 --- /dev/null +++ b/spec/fixtures/api/quit-app/main.js @@ -0,0 +1,9 @@ +var app = require('electron').app + +app.on('ready', function () { + app.exit(123) +}) + +process.on('exit', function (code) { + console.log('Exit event with code: ' + code) +}) diff --git a/spec/fixtures/api/quit-app/package.json b/spec/fixtures/api/quit-app/package.json new file mode 100644 index 000000000000..ea5bb1643b9b --- /dev/null +++ b/spec/fixtures/api/quit-app/package.json @@ -0,0 +1,4 @@ +{ + "name": "quit-app", + "main": "main.js" +} From 92433be8888e3d5eaaaaf93599aab7bcd1e568d6 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:09:59 -0800 Subject: [PATCH 03/10] Include exit code with quit event --- atom/browser/api/atom_api_app.cc | 6 +++--- atom/browser/api/atom_api_app.h | 2 +- atom/browser/api/lib/app.coffee | 9 ++------- atom/browser/atom_browser_main_parts.cc | 4 ++++ atom/browser/atom_browser_main_parts.h | 3 +++ atom/browser/browser.cc | 3 ++- atom/browser/browser_observer.h | 2 +- spec/api-app-spec.coffee | 1 + spec/fixtures/api/quit-app/main.js | 2 +- spec/static/index.html | 1 + 10 files changed, 19 insertions(+), 14 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 0c3623583c62..81ec3fefd510 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -180,8 +180,8 @@ void App::OnWindowAllClosed() { Emit("window-all-closed"); } -void App::OnQuit() { - Emit("quit"); +void App::OnQuit(const int code) { + Emit("quit", code); if (process_singleton_.get()) { process_singleton_->Cleanup(); @@ -344,7 +344,7 @@ mate::ObjectTemplateBuilder App::GetObjectTemplateBuilder( auto browser = base::Unretained(Browser::Get()); return mate::ObjectTemplateBuilder(isolate) .SetMethod("quit", base::Bind(&Browser::Quit, browser)) - .SetMethod("_exit", base::Bind(&Browser::Exit, browser)) + .SetMethod("exit", base::Bind(&Browser::Exit, browser)) .SetMethod("focus", base::Bind(&Browser::Focus, browser)) .SetMethod("getVersion", base::Bind(&Browser::GetVersion, browser)) .SetMethod("setVersion", base::Bind(&Browser::SetVersion, browser)) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index a6f99d65e0af..f04a19e67cdf 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -42,7 +42,7 @@ class App : public AtomBrowserClient::Delegate, void OnBeforeQuit(bool* prevent_default) override; void OnWillQuit(bool* prevent_default) override; void OnWindowAllClosed() override; - void OnQuit() override; + void OnQuit(int code) override; void OnOpenFile(bool* prevent_default, const std::string& file_path) override; void OnOpenURL(const std::string& url) override; void OnActivate(bool has_visible_windows) override; diff --git a/atom/browser/api/lib/app.coffee b/atom/browser/api/lib/app.coffee index fc0a78298c4d..9a7fc6e71141 100644 --- a/atom/browser/api/lib/app.coffee +++ b/atom/browser/api/lib/app.coffee @@ -34,17 +34,12 @@ app.setAppPath = (path) -> app.getAppPath = -> appPath -appExitCode = undefined -app.exit = (exitCode) -> - appExitCode = exitCode - app._exit(exitCode) - # Map process.exit to app.exit, which quits gracefully. process.exit = app.exit # Emit a process 'exit' event on app quit. -app.on 'quit', -> - process.emit 'exit', appExitCode +app.on 'quit', (event, exitCode) -> + process.emit 'exit', exitCode # Routes the events to webContents. for name in ['login', 'certificate-error', 'select-client-certificate'] diff --git a/atom/browser/atom_browser_main_parts.cc b/atom/browser/atom_browser_main_parts.cc index fd72f5e4ae9a..eadd52ac44c8 100644 --- a/atom/browser/atom_browser_main_parts.cc +++ b/atom/browser/atom_browser_main_parts.cc @@ -61,6 +61,10 @@ bool AtomBrowserMainParts::SetExitCode(int code) { return true; } +int AtomBrowserMainParts::GetExitCode() { + return exit_code_ != nullptr ? *exit_code_ : 0; +} + base::Closure AtomBrowserMainParts::RegisterDestructionCallback( const base::Closure& callback) { auto iter = destructors_.insert(destructors_.end(), callback); diff --git a/atom/browser/atom_browser_main_parts.h b/atom/browser/atom_browser_main_parts.h index fbc59f7f811d..c1c0c89c6786 100644 --- a/atom/browser/atom_browser_main_parts.h +++ b/atom/browser/atom_browser_main_parts.h @@ -34,6 +34,9 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts { // Sets the exit code, will fail if the the message loop is not ready. bool SetExitCode(int code); + // Gets the exit code + int GetExitCode(); + // Register a callback that should be destroyed before JavaScript environment // gets destroyed. // Returns a closure that can be used to remove |callback| from the list. diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index c77f359760c9..ee00efa07110 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -72,7 +72,8 @@ void Browser::Shutdown() { is_shutdown_ = true; is_quiting_ = true; - FOR_EACH_OBSERVER(BrowserObserver, observers_, OnQuit()); + int exitCode = AtomBrowserMainParts::Get()->GetExitCode(); + FOR_EACH_OBSERVER(BrowserObserver, observers_, OnQuit(exitCode)); if (base::MessageLoop::current()) { base::MessageLoop::current()->PostTask( diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h index f6d76bc13fb3..f3ae2e364de4 100644 --- a/atom/browser/browser_observer.h +++ b/atom/browser/browser_observer.h @@ -24,7 +24,7 @@ class BrowserObserver { virtual void OnWindowAllClosed() {} // The browser is quitting. - virtual void OnQuit() {} + virtual void OnQuit(const int code) {} // The browser has opened a file by double clicking in Finder or dragging the // file to the Dock icon. (OS X only) diff --git a/spec/api-app-spec.coffee b/spec/api-app-spec.coffee index adb64d363eb1..7b7e5fa483df 100644 --- a/spec/api-app-spec.coffee +++ b/spec/api-app-spec.coffee @@ -44,6 +44,7 @@ describe 'app module', -> output = '' appProcess.stdout.on 'data', (data) -> output += data appProcess.on 'close', (code) -> + console.log output assert.notEqual output.indexOf('Exit event with code: 123'), -1 assert.equal code, 123 done() diff --git a/spec/fixtures/api/quit-app/main.js b/spec/fixtures/api/quit-app/main.js index 4bdeb93a5e30..85939acd7348 100644 --- a/spec/fixtures/api/quit-app/main.js +++ b/spec/fixtures/api/quit-app/main.js @@ -5,5 +5,5 @@ app.on('ready', function () { }) process.on('exit', function (code) { - console.log('Exit event with code: ' + code) + console.log('Exit event with code: ' + JSON.stringify(code, null, 2)) }) diff --git a/spec/static/index.html b/spec/static/index.html index ea86f6ee302d..2053ca41355c 100644 --- a/spec/static/index.html +++ b/spec/static/index.html @@ -56,6 +56,7 @@ mocha.ui('bdd').reporter(isCi ? 'tap' : 'html'); var query = Mocha.utils.parseQuery(window.location.search || ''); + query.grep = 'app.exit' if (query.grep) mocha.grep(query.grep); if (query.invert) mocha.invert(); From fc724b51e8c206fe10a648bfd73ad0593e277601 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:11:38 -0800 Subject: [PATCH 04/10] Move event forwarding back to init --- atom/browser/api/lib/app.coffee | 7 ------- atom/browser/lib/init.coffee | 6 ++++++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/atom/browser/api/lib/app.coffee b/atom/browser/api/lib/app.coffee index 9a7fc6e71141..d0ec41c4d23c 100644 --- a/atom/browser/api/lib/app.coffee +++ b/atom/browser/api/lib/app.coffee @@ -34,13 +34,6 @@ app.setAppPath = (path) -> app.getAppPath = -> appPath -# Map process.exit to app.exit, which quits gracefully. -process.exit = app.exit - -# Emit a process 'exit' event on app quit. -app.on 'quit', (event, exitCode) -> - process.emit 'exit', exitCode - # Routes the events to webContents. for name in ['login', 'certificate-error', 'select-client-certificate'] do (name) -> diff --git a/atom/browser/lib/init.coffee b/atom/browser/lib/init.coffee index 9487849e5e69..41cd6fb22ca2 100644 --- a/atom/browser/lib/init.coffee +++ b/atom/browser/lib/init.coffee @@ -51,7 +51,13 @@ process.on 'uncaughtException', (error) -> message = "Uncaught Exception:\n#{stack}" dialog.showErrorBox 'A JavaScript error occurred in the main process', message +# Emit a process 'exit' event on app quit. {app} = require 'electron' +app.on 'quit', (event, exitCode) -> + process.emit 'exit', exitCode + +# Map process.exit to app.exit, which quits gracefully. +process.exit = app.exit # Load the RPC server. require './rpc-server' From c4389ad70f63b1aec9430b225bd084bf8b99833c Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:12:19 -0800 Subject: [PATCH 05/10] Remove grep value setting --- spec/static/index.html | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/static/index.html b/spec/static/index.html index 2053ca41355c..ea86f6ee302d 100644 --- a/spec/static/index.html +++ b/spec/static/index.html @@ -56,7 +56,6 @@ mocha.ui('bdd').reporter(isCi ? 'tap' : 'html'); var query = Mocha.utils.parseQuery(window.location.search || ''); - query.grep = 'app.exit' if (query.grep) mocha.grep(query.grep); if (query.invert) mocha.invert(); From 3e5caf7e544ca006a7a63660c10e9fbffbded505 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:19:51 -0800 Subject: [PATCH 06/10] Get exit code from within App::OnQuit --- atom/browser/api/atom_api_app.cc | 5 +++-- atom/browser/api/atom_api_app.h | 2 +- atom/browser/browser.cc | 3 +-- atom/browser/browser_observer.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 81ec3fefd510..256ecff53e10 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -180,8 +180,9 @@ void App::OnWindowAllClosed() { Emit("window-all-closed"); } -void App::OnQuit(const int code) { - Emit("quit", code); +void App::OnQuit() { + int exitCode = AtomBrowserMainParts::Get()->GetExitCode(); + Emit("quit", exitCode); if (process_singleton_.get()) { process_singleton_->Cleanup(); diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index f04a19e67cdf..a6f99d65e0af 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -42,7 +42,7 @@ class App : public AtomBrowserClient::Delegate, void OnBeforeQuit(bool* prevent_default) override; void OnWillQuit(bool* prevent_default) override; void OnWindowAllClosed() override; - void OnQuit(int code) override; + void OnQuit() override; void OnOpenFile(bool* prevent_default, const std::string& file_path) override; void OnOpenURL(const std::string& url) override; void OnActivate(bool has_visible_windows) override; diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index ee00efa07110..c77f359760c9 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -72,8 +72,7 @@ void Browser::Shutdown() { is_shutdown_ = true; is_quiting_ = true; - int exitCode = AtomBrowserMainParts::Get()->GetExitCode(); - FOR_EACH_OBSERVER(BrowserObserver, observers_, OnQuit(exitCode)); + FOR_EACH_OBSERVER(BrowserObserver, observers_, OnQuit()); if (base::MessageLoop::current()) { base::MessageLoop::current()->PostTask( diff --git a/atom/browser/browser_observer.h b/atom/browser/browser_observer.h index f3ae2e364de4..f6d76bc13fb3 100644 --- a/atom/browser/browser_observer.h +++ b/atom/browser/browser_observer.h @@ -24,7 +24,7 @@ class BrowserObserver { virtual void OnWindowAllClosed() {} // The browser is quitting. - virtual void OnQuit(const int code) {} + virtual void OnQuit() {} // The browser has opened a file by double clicking in Finder or dragging the // file to the Dock icon. (OS X only) From ea1479a651bfbe4666e13f4e68e2b1177f3f512e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:20:32 -0800 Subject: [PATCH 07/10] Revert comment tweak --- atom/browser/lib/init.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/lib/init.coffee b/atom/browser/lib/init.coffee index 41cd6fb22ca2..c3d56d9c7c82 100644 --- a/atom/browser/lib/init.coffee +++ b/atom/browser/lib/init.coffee @@ -51,7 +51,7 @@ process.on 'uncaughtException', (error) -> message = "Uncaught Exception:\n#{stack}" dialog.showErrorBox 'A JavaScript error occurred in the main process', message -# Emit a process 'exit' event on app quit. +# Emit 'exit' event on quit. {app} = require 'electron' app.on 'quit', (event, exitCode) -> process.emit 'exit', exitCode From e1654ee334962c9786756fab314f3b6649a34bb0 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:22:54 -0800 Subject: [PATCH 08/10] :memo: Document quit event includes exit code --- docs/api/app.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/api/app.md b/docs/api/app.md index 3faf4a1f0ac6..f21f37039705 100644 --- a/docs/api/app.md +++ b/docs/api/app.md @@ -64,6 +64,11 @@ the `will-quit` and `window-all-closed` events. ### Event: 'quit' +Returns: + +* `event` Event +* `exitCode` Integer + Emitted when the application is quitting. ### Event: 'open-file' _OS X_ From 516ff0644c9aca8e921c46fc70a928f0d1388966 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:23:50 -0800 Subject: [PATCH 09/10] Just include exit code in spec output --- spec/api-app-spec.coffee | 1 - spec/fixtures/api/quit-app/main.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/api-app-spec.coffee b/spec/api-app-spec.coffee index 7b7e5fa483df..adb64d363eb1 100644 --- a/spec/api-app-spec.coffee +++ b/spec/api-app-spec.coffee @@ -44,7 +44,6 @@ describe 'app module', -> output = '' appProcess.stdout.on 'data', (data) -> output += data appProcess.on 'close', (code) -> - console.log output assert.notEqual output.indexOf('Exit event with code: 123'), -1 assert.equal code, 123 done() diff --git a/spec/fixtures/api/quit-app/main.js b/spec/fixtures/api/quit-app/main.js index 85939acd7348..4bdeb93a5e30 100644 --- a/spec/fixtures/api/quit-app/main.js +++ b/spec/fixtures/api/quit-app/main.js @@ -5,5 +5,5 @@ app.on('ready', function () { }) process.on('exit', function (code) { - console.log('Exit event with code: ' + JSON.stringify(code, null, 2)) + console.log('Exit event with code: ' + code) }) From f5774e3fb24ca403de481e446c3bf68fa78b2c47 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 9 Dec 2015 18:39:59 -0800 Subject: [PATCH 10/10] Exit from a setImmediate callback --- spec/fixtures/api/quit-app/main.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/fixtures/api/quit-app/main.js b/spec/fixtures/api/quit-app/main.js index 4bdeb93a5e30..e2f97affe6de 100644 --- a/spec/fixtures/api/quit-app/main.js +++ b/spec/fixtures/api/quit-app/main.js @@ -1,7 +1,10 @@ var app = require('electron').app app.on('ready', function () { - app.exit(123) + // This setImmediate call gets the spec passing on Linux + setImmediate(function () { + app.exit(123) + }) }) process.on('exit', function (code) {