From ef690c035dc1fa783f12da1cbecfaa3cc46c48db Mon Sep 17 00:00:00 2001
From: Cheng Zhao <zcbenz@gmail.com>
Date: Wed, 2 Oct 2019 02:20:21 +0900
Subject: [PATCH] fix: correctly crash when there is no crashReporter (#20388)

* fix: correctly crash when there is no crashReporter

* test: correctly crash when there is crashReporter
---
 shell/common/crash_reporter/crash_reporter_win.cc | 12 ++++++++++--
 spec/api-crash-reporter-spec.js                   | 10 ++++++++++
 spec/fixtures/crash-app/main.js                   |  5 +++++
 spec/fixtures/crash-app/package.json              |  4 ++++
 4 files changed, 29 insertions(+), 2 deletions(-)
 create mode 100644 spec/fixtures/crash-app/main.js
 create mode 100644 spec/fixtures/crash-app/package.json

diff --git a/shell/common/crash_reporter/crash_reporter_win.cc b/shell/common/crash_reporter/crash_reporter_win.cc
index 045926683a..6231e4b71b 100644
--- a/shell/common/crash_reporter/crash_reporter_win.cc
+++ b/shell/common/crash_reporter/crash_reporter_win.cc
@@ -28,9 +28,17 @@ namespace {
 #if defined(_WIN64)
 int CrashForException(EXCEPTION_POINTERS* info) {
   auto* reporter = crash_reporter::CrashReporterWin::GetInstance();
-  if (reporter->IsInitialized())
+  if (reporter->IsInitialized()) {
     reporter->GetCrashpadClient().DumpAndCrash(info);
-  return EXCEPTION_CONTINUE_SEARCH;
+    return EXCEPTION_CONTINUE_SEARCH;
+  }
+
+  // When there is exception and we do not have crashReporter set up, we just
+  // let the execution continue and crash, which is the default behavior.
+  //
+  // We must not return EXCEPTION_CONTINUE_SEARCH here, as it would end up with
+  // busy loop when there is no exception handler in the program.
+  return EXCEPTION_CONTINUE_EXECUTION;
 }
 #endif
 
diff --git a/spec/api-crash-reporter-spec.js b/spec/api-crash-reporter-spec.js
index 4436cd200b..602af17141 100644
--- a/spec/api-crash-reporter-spec.js
+++ b/spec/api-crash-reporter-spec.js
@@ -415,6 +415,16 @@ describe('crashReporter module', () => {
       expect(crashReporter.getParameters()).to.not.have.a.property('hello')
     })
   })
+
+  describe('when not started', () => {
+    it('does not prevent process from crashing', (done) => {
+      const appPath = path.join(fixtures, 'api', 'cookie-app')
+      const appProcess = childProcess.spawn(process.execPath, [appPath])
+      appProcess.once('close', () => {
+        done()
+      })
+    })
+  })
 })
 
 const waitForCrashReport = () => {
diff --git a/spec/fixtures/crash-app/main.js b/spec/fixtures/crash-app/main.js
new file mode 100644
index 0000000000..e279216218
--- /dev/null
+++ b/spec/fixtures/crash-app/main.js
@@ -0,0 +1,5 @@
+const { app } = require('electron')
+
+app.on('ready', () => {
+  process.crash()
+})
diff --git a/spec/fixtures/crash-app/package.json b/spec/fixtures/crash-app/package.json
new file mode 100644
index 0000000000..e03fc08f1f
--- /dev/null
+++ b/spec/fixtures/crash-app/package.json
@@ -0,0 +1,4 @@
+{
+  "name": "electron-crash-app",
+  "main": "main.js"
+}