test: move beforeunload tests to main runner and fix flake (#18432)

spec-main/api-web-contents-spec.ts

@@ -9,7 +9,7 @@ const { expect } = chai
 
 chai.use(chaiAsPromised)
 
-const fixturesPath = path.resolve(__dirname, '../spec/fixtures')
+const fixturesPath = path.resolve(__dirname, '..', 'spec', 'fixtures')
 
 describe('webContents module', () => {
   let w: BrowserWindow = (null as unknown as BrowserWindow)
@@ -52,4 +52,25 @@ describe('webContents module', () => {
       expect(all[all.length - 1].getType()).to.equal('remote')
     })
   })
+
+  describe('will-prevent-unload event', () => {
+    it('does not emit if beforeunload returns undefined', (done) => {
+      w.once('closed', () => done())
+      w.webContents.once('will-prevent-unload', (e) => {
+        expect.fail('should not have fired')
+      })
+      w.loadFile(path.join(fixturesPath, 'api', 'close-beforeunload-undefined.html'))
+    })
+
+    it('emits if beforeunload returns false', (done) => {
+      w.webContents.once('will-prevent-unload', () => done())
+      w.loadFile(path.join(fixturesPath, 'api', 'close-beforeunload-false.html'))
+    })
+
+    it('supports calling preventDefault on will-prevent-unload events', (done) => {
+      w.webContents.once('will-prevent-unload', event => event.preventDefault())
+      w.once('closed', () => done())
+      w.loadFile(path.join(fixturesPath, 'api', 'close-beforeunload-false.html'))
+    })
+  })
 })

spec-main/window-helpers.ts

@@ -1,32 +1,38 @@
 import { expect } from 'chai'
-import { BrowserWindow, WebContents } from 'electron'
+import { BrowserWindow } from 'electron'
 import { emittedOnce } from './events-helpers';
 
+async function ensureWindowIsClosed(window: BrowserWindow | null) {
+  if (window && !window.isDestroyed()) {
+    if (window.webContents && !window.webContents.isDestroyed()) {
+      // If a window isn't destroyed already, and it has non-destroyed WebContents,
+      // then calling destroy() won't immediately destroy it, as it may have
+      // <webview> children which need to be destroyed first. In that case, we
+      // await the 'closed' event which signals the complete shutdown of the
+      // window.
+      const isClosed = emittedOnce(window, 'closed')
+      window.destroy()
+      await isClosed
+    } else {
+      // If there's no WebContents or if the WebContents is already destroyed,
+      // then the 'closed' event has already been emitted so there's nothing to
+      // wait for.
+      window.destroy()
+    }
+  }
+}
+
 export const closeWindow = async (
   window: BrowserWindow | null = null,
   { assertNotWindows } = { assertNotWindows: true }
 ) => {
-  if (window && !window.isDestroyed()) {
+  await ensureWindowIsClosed(window)
-    const isClosed = emittedOnce(window, 'closed')
-    window.setClosable(true)
-    window.close()
-    await isClosed
-  }
 
   if (assertNotWindows) {
     const windows = BrowserWindow.getAllWindows()
     for (const win of windows) {
-      const closePromise = emittedOnce(win, 'closed')
+      await ensureWindowIsClosed(win)
-      win.close()
-      await closePromise
     }
     expect(windows).to.have.lengthOf(0)
   }
 }
-
-exports.waitForWebContentsToLoad = async (webContents: WebContents) => {
-  const didFinishLoadPromise = emittedOnce(webContents, 'did-finish-load')
-  if (webContents.isLoadingMainFrame()) {
-    await didFinishLoadPromise
-  }
-}

spec/api-web-contents-spec.js

@@ -779,31 +779,6 @@ describe('webContents module', () => {
     })
   })
 
-  describe('will-prevent-unload event', () => {
-    it('does not emit if beforeunload returns undefined', (done) => {
-      w.once('closed', () => {
-        done()
-      })
-      w.webContents.on('will-prevent-unload', (e) => {
-        expect.fail('should not have fired')
-      })
-      w.loadFile(path.join(fixtures, 'api', 'close-beforeunload-undefined.html'))
-    })
-
-    it('emits if beforeunload returns false', (done) => {
-      w.webContents.on('will-prevent-unload', () => {
-        done()
-      })
-      w.loadFile(path.join(fixtures, 'api', 'close-beforeunload-false.html'))
-    })
-
-    it('supports calling preventDefault on will-prevent-unload events', (done) => {
-      ipcRenderer.send('prevent-next-will-prevent-unload', w.webContents.id)
-      w.once('closed', () => done())
-      w.loadFile(path.join(fixtures, 'api', 'close-beforeunload-false.html'))
-    })
-  })
-
   describe('render view deleted events', () => {
     let server = null
 

spec/window-helpers.js

@@ -4,15 +4,29 @@ const { BrowserWindow } = remote
 
 const { emittedOnce } = require('./events-helpers')
 
+async function ensureWindowIsClosed (window) {
+  if (window && !window.isDestroyed()) {
+    if (window.webContents && !window.webContents.isDestroyed()) {
+      // If a window isn't destroyed already, and it has non-destroyed WebContents,
+      // then calling destroy() won't immediately destroy it, as it may have
+      // <webview> children which need to be destroyed first. In that case, we
+      // await the 'closed' event which signals the complete shutdown of the
+      // window.
+      const isClosed = emittedOnce(window, 'closed')
+      window.destroy()
+      await isClosed
+    } else {
+      // If there's no WebContents or if the WebContents is already destroyed,
+      // then the 'closed' event has already been emitted so there's nothing to
+      // wait for.
+      window.destroy()
+    }
+  }
+}
+
 exports.closeWindow = async (window = null,
   { assertSingleWindow } = { assertSingleWindow: true }) => {
-  const windowExists = (window !== null) && !window.isDestroyed()
+  await ensureWindowIsClosed(window)
-  if (windowExists) {
-    const isClosed = emittedOnce(window, 'closed')
-    window.setClosable(true)
-    window.close()
-    await isClosed
-  }
 
   if (assertSingleWindow) {
     // Although we want to assert that no windows were left handing around
@@ -22,9 +36,7 @@ exports.closeWindow = async (window = null,
     const windows = BrowserWindow.getAllWindows()
     for (const win of windows) {
       if (win.id !== currentId) {
-        const closePromise = emittedOnce(win, 'closed')
+        await ensureWindowIsClosed(win)
-        win.close()
-        await closePromise
       }
     }
     expect(windows).to.have.lengthOf(1)