From 2c974915a3a9df5a620e2c9157901aac2f99c5be Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Fri, 28 Feb 2020 23:10:21 +0000
Subject: [PATCH] fix: do not call close on sheets themselves (#22410)

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

diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm
index f199331f2c23..1260be11d8c2 100644
--- a/shell/browser/native_window_mac.mm
+++ b/shell/browser/native_window_mac.mm
@@ -588,7 +588,14 @@ void NativeWindowMac::SetContentView(views::View* view) {
 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 8b93af07be01..8dd42886f44e 100644
--- a/spec-main/api-browser-window-spec.ts
+++ b/spec-main/api-browser-window-spec.ts
@@ -3105,16 +3105,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 freeze or 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', () => {
@@ -3124,7 +3161,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', () => {
@@ -3135,7 +3173,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 })