From f9a1dc10fe86a8dcac029d354d4dbc5c0695375b Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Fri, 13 Dec 2019 18:57:02 +0900
Subject: [PATCH] fix: quit after Chromium is fully started (#21488)

* fix: quit when chromium is fully started

* test: remove hacks on app.quit

* chore: RunUntilIdle is unnecessary
---
 shell/browser/browser.cc                      | 19 +++++++++++++++++--
 .../fixtures/api/leak-exit-webcontentsview.js |  2 +-
 spec/fixtures/api/command-line/main.js        |  4 +---
 spec/fixtures/api/cookie-app/main.js          |  4 +---
 spec/fixtures/api/ipc-main-listeners/main.js  |  4 +---
 spec/fixtures/api/leak-exit-browserview.js    |  2 +-
 spec/fixtures/api/leak-exit-webcontents.js    |  2 +-
 spec/fixtures/api/locale-check/main.js        |  4 +---
 8 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/shell/browser/browser.cc b/shell/browser/browser.cc
index 7ac78481d33d6..fcc3c73e2c092 100644
--- a/shell/browser/browser.cc
+++ b/shell/browser/browser.cc
@@ -26,6 +26,21 @@
 
 namespace electron {
 
+namespace {
+
+// Call |quit| after Chromium is fully started.
+//
+// This is important for quitting immediately in the "ready" event, when
+// certain initialization task may still be pending, and quitting at that time
+// could end up with crash on exit.
+void RunQuitClosure(base::OnceClosure quit) {
+  // On Linux/Windows the "ready" event is emitted in "PreMainMessageLoopRun",
+  // make sure we quit after message loop has run for once.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(quit));
+}
+
+}  // namespace
+
 Browser::LoginItemSettings::LoginItemSettings() = default;
 Browser::LoginItemSettings::~LoginItemSettings() = default;
 Browser::LoginItemSettings::LoginItemSettings(const LoginItemSettings& other) =
@@ -94,7 +109,7 @@ void Browser::Shutdown() {
     observer.OnQuit();
 
   if (quit_main_message_loop_) {
-    std::move(quit_main_message_loop_).Run();
+    RunQuitClosure(std::move(quit_main_message_loop_));
   } else {
     // There is no message loop available so we are in early stage, wait until
     // the quit_main_message_loop_ is available.
@@ -189,7 +204,7 @@ void Browser::PreMainMessageLoopRun() {
 
 void Browser::SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure) {
   if (is_shutdown_)
-    std::move(quit_closure).Run();
+    RunQuitClosure(std::move(quit_closure));
   else
     quit_main_message_loop_ = std::move(quit_closure);
 }
diff --git a/spec-main/fixtures/api/leak-exit-webcontentsview.js b/spec-main/fixtures/api/leak-exit-webcontentsview.js
index 1196cfcca839b..cec51aeef2764 100644
--- a/spec-main/fixtures/api/leak-exit-webcontentsview.js
+++ b/spec-main/fixtures/api/leak-exit-webcontentsview.js
@@ -3,5 +3,5 @@ app.on('ready', function () {
   const web = webContents.create({})
   new WebContentsView(web)  // eslint-disable-line
 
-  process.nextTick(() => app.quit())
+  app.quit()
 })
diff --git a/spec/fixtures/api/command-line/main.js b/spec/fixtures/api/command-line/main.js
index 39e62cafbbc87..3a38a2cba2564 100644
--- a/spec/fixtures/api/command-line/main.js
+++ b/spec/fixtures/api/command-line/main.js
@@ -9,7 +9,5 @@ app.on('ready', () => {
   process.stdout.write(JSON.stringify(payload))
   process.stdout.end()
 
-  setImmediate(() => {
-    app.quit()
-  })
+  app.quit()
 })
diff --git a/spec/fixtures/api/cookie-app/main.js b/spec/fixtures/api/cookie-app/main.js
index 3e63186537898..d3c3dcd467c1b 100644
--- a/spec/fixtures/api/cookie-app/main.js
+++ b/spec/fixtures/api/cookie-app/main.js
@@ -42,8 +42,6 @@ app.on('ready', async function () {
   } finally {
     process.stdout.end()
 
-    setImmediate(() => {
-      app.quit()
-    })
+    app.quit()
   }
 })
diff --git a/spec/fixtures/api/ipc-main-listeners/main.js b/spec/fixtures/api/ipc-main-listeners/main.js
index d56348136bc1d..afb459f74f5c8 100644
--- a/spec/fixtures/api/ipc-main-listeners/main.js
+++ b/spec/fixtures/api/ipc-main-listeners/main.js
@@ -4,7 +4,5 @@ app.on('ready', () => {
   process.stdout.write(JSON.stringify(ipcMain.eventNames()))
   process.stdout.end()
 
-  setImmediate(() => {
-    app.quit()
-  })
+  app.quit()
 })
diff --git a/spec/fixtures/api/leak-exit-browserview.js b/spec/fixtures/api/leak-exit-browserview.js
index 534fea59bbca7..8e5d5d2eb5a60 100644
--- a/spec/fixtures/api/leak-exit-browserview.js
+++ b/spec/fixtures/api/leak-exit-browserview.js
@@ -2,5 +2,5 @@ const { BrowserView, app } = require('electron')
 app.on('ready', function () {
   new BrowserView({})  // eslint-disable-line
 
-  process.nextTick(() => app.quit())
+  app.quit()
 })
diff --git a/spec/fixtures/api/leak-exit-webcontents.js b/spec/fixtures/api/leak-exit-webcontents.js
index 5bf9e205793ce..f08a8249dde87 100644
--- a/spec/fixtures/api/leak-exit-webcontents.js
+++ b/spec/fixtures/api/leak-exit-webcontents.js
@@ -2,5 +2,5 @@ const { app, webContents } = require('electron')
 app.on('ready', function () {
   webContents.create({})
 
-  process.nextTick(() => app.quit())
+  app.quit()
 })
diff --git a/spec/fixtures/api/locale-check/main.js b/spec/fixtures/api/locale-check/main.js
index c89e129904245..47b820c61ddd2 100644
--- a/spec/fixtures/api/locale-check/main.js
+++ b/spec/fixtures/api/locale-check/main.js
@@ -4,7 +4,5 @@ app.on('ready', () => {
   process.stdout.write(app.getLocale())
   process.stdout.end()
 
-  setImmediate(() => {
-    app.quit()
-  })
+  app.quit()
 })