fix: shutdown after message loop is ready (backport: 5-0-x)

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() {

atom/browser/browser.cc

@@ -25,9 +25,6 @@
 
 namespace atom {
 
-// Null until/unless the default main message loop is running.
-base::NoDestructor<base::OnceClosure> 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() {

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;

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)

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

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

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