From 5c26f9561e7aee6518e342a40d4453393e27115d Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Mon, 2 Mar 2020 11:28:21 -0800
Subject: [PATCH] fix: prevent potential modal window close segfault

---
 shell/browser/native_window_mac.mm   |  9 ++++-
 spec-main/api-browser-window-spec.ts | 50 +++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm
index 93638fdc1b3e3..7e86b6759e96e 100644
--- a/shell/browser/native_window_mac.mm
+++ b/shell/browser/native_window_mac.mm
@@ -529,7 +529,14 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) {
 void NativeWindowMac::Close() {
   // When this is a sheet showing, performClose won't work.
   if (is_modal() && parent() && IsVisible()) {
-    [parent()->GetNativeWindow().GetNativeNSWindow() endSheet:window_];
+    NSWindow* window = parent()->GetNativeWindow().GetNativeNSWindow();
+    if (NSWindow* sheetParent = [window sheetParent]) {
+      base::ThreadTaskRunnerHandle::Get()->PostTask(
+          FROM_HERE, base::BindOnce(base::RetainBlock(^{
+            [sheetParent endSheet:window];
+          })));
+    }
+
     // Manually emit close event (not triggered from close fn)
     NotifyWindowCloseButtonClicked();
     CloseImmediately();
diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts
index c3f83ed5ed106..66701ca4edbb9 100644
--- a/spec-main/api-browser-window-spec.ts
+++ b/spec-main/api-browser-window-spec.ts
@@ -3008,15 +3008,53 @@ describe('BrowserWindow module', () => {
     })
 
     // The isEnabled API is not reliable on macOS.
-    ifdescribe(process.platform !== 'darwin')('modal option', () => {
-      it('disables parent window', () => {
+    describe('modal option', () => {
+      it('does not crash', async () => {
+        const parentWindow = new BrowserWindow()
+
+        const createTwo = async () => {
+          const two = new BrowserWindow({
+            width: 300,
+            height: 200,
+            parent: parentWindow,
+            modal: true,
+            show: false
+          })
+
+          const twoShown = emittedOnce(two, 'show')
+          two.show()
+          await twoShown
+          setTimeout(() => two.close(), 500)
+
+          await emittedOnce(two, 'closed')
+        }
+
+        const one = new BrowserWindow({
+          width: 600,
+          height: 400,
+          parent: parentWindow,
+          modal: true,
+          show: false
+        })
+
+        const oneShown = emittedOnce(one, 'show')
+        one.show()
+        await oneShown
+        setTimeout(() => one.destroy(), 500)
+
+        await emittedOnce(one, 'closed')
+        await createTwo()
+      })
+
+      ifit(process.platform !== 'darwin')('disables parent window', () => {
         const w = new BrowserWindow({show: false})
         const c = new BrowserWindow({ show: false, parent: w, modal: true })
         expect(w.isEnabled()).to.be.true('w.isEnabled')
         c.show()
         expect(w.isEnabled()).to.be.false('w.isEnabled')
       })
-      it('re-enables an enabled parent window when closed', (done) => {
+
+      ifit(process.platform !== 'darwin')('re-enables an enabled parent window when closed', (done) => {
         const w = new BrowserWindow({show: false})
         const c = new BrowserWindow({ show: false, parent: w, modal: true })
         c.once('closed', () => {
@@ -3026,7 +3064,8 @@ describe('BrowserWindow module', () => {
         c.show()
         c.close()
       })
-      it('does not re-enable a disabled parent window when closed', (done) => {
+
+      ifit(process.platform !== 'darwin')('does not re-enable a disabled parent window when closed', (done) => {
         const w = new BrowserWindow({show: false})
         const c = new BrowserWindow({ show: false, parent: w, modal: true })
         c.once('closed', () => {
@@ -3037,7 +3076,8 @@ describe('BrowserWindow module', () => {
         c.show()
         c.close()
       })
-      it('disables parent window recursively', () => {
+
+      ifit(process.platform !== 'darwin')('disables parent window recursively', () => {
         const w = new BrowserWindow({show: false})
         const c = new BrowserWindow({ show: false, parent: w, modal: true })
         const c2 = new BrowserWindow({ show: false, parent: w, modal: true })