From fa7985e4a161b00986014d847c0bdf32264f92d1 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 1 Feb 2019 16:25:06 +0900 Subject: [PATCH] fix: shutdown after message loop is ready --- atom/browser/atom_browser_main_parts.cc | 2 +- atom/browser/browser.cc | 17 +++++++++-------- atom/browser/browser.h | 5 ++++- spec/api-app-spec.js | 7 ------- spec/api-browser-view-spec.js | 7 ------- spec/api-web-contents-spec.js | 7 ------- spec/api-web-contents-view-spec.js | 7 ------- 7 files changed, 14 insertions(+), 38 deletions(-) diff --git a/atom/browser/atom_browser_main_parts.cc b/atom/browser/atom_browser_main_parts.cc index 3140af8cbc272..41f41c85abf98 100644 --- a/atom/browser/atom_browser_main_parts.cc +++ b/atom/browser/atom_browser_main_parts.cc @@ -455,7 +455,7 @@ bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) { void AtomBrowserMainParts::PreDefaultMainMessageLoopRun( base::OnceClosure quit_closure) { - Browser::SetMainMessageLoopQuitClosure(std::move(quit_closure)); + Browser::Get()->SetMainMessageLoopQuitClosure(std::move(quit_closure)); } void AtomBrowserMainParts::PostMainMessageLoopStart() { diff --git a/atom/browser/browser.cc b/atom/browser/browser.cc index e37de81042881..e4962b12e8620 100644 --- a/atom/browser/browser.cc +++ b/atom/browser/browser.cc @@ -25,9 +25,6 @@ namespace atom { -// Null until/unless the default main message loop is running. -base::NoDestructor g_quit_main_message_loop; - Browser::LoginItemSettings::LoginItemSettings() = default; Browser::LoginItemSettings::~LoginItemSettings() = default; Browser::LoginItemSettings::LoginItemSettings(const LoginItemSettings& other) = @@ -95,11 +92,12 @@ void Browser::Shutdown() { for (BrowserObserver& observer : observers_) observer.OnQuit(); - if (*g_quit_main_message_loop) { - std::move(*g_quit_main_message_loop).Run(); + if (quit_main_message_loop_) { + std::move(quit_main_message_loop_).Run(); } else { - // There is no message loop available so we are in early stage. - exit(0); + // There is no message loop available so we are in early stage, wait until + // the quit_main_message_loop_ is available. + // Exiting now would leave defunct processes behind. } } @@ -196,7 +194,10 @@ void Browser::PreMainMessageLoopRun() { } void Browser::SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure) { - *g_quit_main_message_loop = std::move(quit_closure); + if (is_shutdown_) + std::move(quit_closure).Run(); + else + quit_main_message_loop_ = std::move(quit_closure); } void Browser::NotifyAndShutdown() { diff --git a/atom/browser/browser.h b/atom/browser/browser.h index 98fba93fc5138..f01ece367b526 100644 --- a/atom/browser/browser.h +++ b/atom/browser/browser.h @@ -244,7 +244,7 @@ class Browser : public WindowListObserver { // Stores the supplied |quit_closure|, to be run when the last Browser // instance is destroyed. - static void SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure); + void SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure); void AddObserver(BrowserObserver* obs) { observers_.AddObserver(obs); } @@ -287,6 +287,9 @@ class Browser : public WindowListObserver { // The browser is being shutdown. bool is_shutdown_ = false; + // Null until/unless the default main message loop is running. + base::OnceClosure quit_main_message_loop_; + int badge_count_ = 0; util::Promise* ready_promise_ = nullptr; diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 69041c517885f..d6c4e9d973e5f 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -1222,13 +1222,6 @@ describe('default behavior', () => { }) describe('window-all-closed', () => { - before(function () { - // FIXME(jkleinsc): Test is consistently failing on Windows 32 bit. - if (process.arch === 'ia32') { - this.skip() - } - }) - it('quits when the app does not handle the event', async () => { const result = await runTestApp('window-all-closed') expect(result).to.equal(false) diff --git a/spec/api-browser-view-spec.js b/spec/api-browser-view-spec.js index dd4175413da69..fbbb4ebe01808 100644 --- a/spec/api-browser-view-spec.js +++ b/spec/api-browser-view-spec.js @@ -228,13 +228,6 @@ describe('BrowserView module', () => { }) describe('new BrowserView()', () => { - before(function () { - // FIXME(jkleinsc): Test is consistently failing on Windows 32 bit. - if (process.arch === 'ia32') { - this.skip() - } - }) - it('does not crash on exit', async () => { const appPath = path.join(fixtures, 'api', 'leak-exit-browserview.js') const electronPath = remote.getGlobal('process').execPath diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index 026277a3eea9c..c744a3db59dcf 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -893,13 +893,6 @@ describe('webContents module', () => { }) describe('create()', () => { - before(function () { - // FIXME(jkleinsc): Test is consistently failing on Windows 32 bit. - if (process.arch === 'ia32') { - this.skip() - } - }) - it('does not crash on exit', async () => { const appPath = path.join(__dirname, 'fixtures', 'api', 'leak-exit-webcontents.js') const electronPath = remote.getGlobal('process').execPath diff --git a/spec/api-web-contents-view-spec.js b/spec/api-web-contents-view-spec.js index e75b3eccfe32e..8610dda5a7878 100644 --- a/spec/api-web-contents-view-spec.js +++ b/spec/api-web-contents-view-spec.js @@ -34,13 +34,6 @@ describe('WebContentsView', () => { }) describe('new WebContentsView()', () => { - before(function () { - // FIXME(jkleinsc): Test is consistently failing on Windows 32 bit. - if (process.arch === 'ia32') { - this.skip() - } - }) - it('does not crash on exit', async () => { const appPath = path.join(__dirname, 'fixtures', 'api', 'leak-exit-webcontentsview.js') const electronPath = remote.getGlobal('process').execPath