test: remove last usages of spec/window-helpers (#20792)

* test: remove last usages of spec/window-helpers

* move fixture

spec-main/api-remote-spec.ts

@@ -1,139 +1,194 @@
 import * as path from 'path'
 import { expect } from 'chai'
-import { closeWindow } from './window-helpers'
+import { closeWindow, closeAllWindows } from './window-helpers'
 import { ifdescribe } from './spec-helpers';
 
 import { ipcMain, BrowserWindow } from 'electron'
+import { emittedOnce } from './events-helpers';
 
 const features = process.electronBinding('features')
 
 ifdescribe(features.isRemoteModuleEnabled())('remote module', () => {
   const fixtures = path.join(__dirname, 'fixtures')
 
-  let w = null as unknown as BrowserWindow
-  beforeEach(async () => {
-    w = new BrowserWindow({show: false, webPreferences: {nodeIntegration: true}})
-    await w.loadURL('about:blank')
-  })
-  afterEach(async () => {
-    await closeWindow(w)
-  })
+  describe('', () => {
+    let w = null as unknown as BrowserWindow
+    beforeEach(async () => {
+      w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } })
+      w.loadURL('about:blank')
+    })
+    afterEach(async () => {
+      await closeWindow(w)
+    })
 
-  async function remotely(script: string) {
-    // executeJavaScript never returns if the script throws an error, so catch
-    // any errors manually.
-    const assembledScript = `(function() {
+    async function remotely(script: string) {
+      // executeJavaScript never returns if the script throws an error, so catch
+      // any errors manually.
+      const assembledScript = `(function() {
       try {
         return { result: ${script} }
       } catch (e) {
         return { error: e.message }
       }
     })()`
-    const {result, error} = await w.webContents.executeJavaScript(assembledScript)
-    if (error) {
-      throw new Error(error)
+      const { result, error } = await w.webContents.executeJavaScript(assembledScript)
+      if (error) {
+        throw new Error(error)
+      }
+      return result
     }
-    return result
-  }
 
-  describe('remote.getGlobal filtering', () => {
-    it('can return custom values', async () => {
-      w.webContents.once('remote-get-global', (event, name) => {
-        event.returnValue = name
+    describe('remote.getGlobal filtering', () => {
+      it('can return custom values', async () => {
+        w.webContents.once('remote-get-global', (event, name) => {
+          event.returnValue = name
+        })
+        expect(await remotely(`require('electron').remote.getGlobal('test')`)).to.equal('test')
       })
-      expect(await remotely(`require('electron').remote.getGlobal('test')`)).to.equal('test')
-    })
 
-    it('throws when no returnValue set', async () => {
-      w.webContents.once('remote-get-global', (event, name) => {
-        event.preventDefault()
+      it('throws when no returnValue set', async () => {
+        w.webContents.once('remote-get-global', (event, name) => {
+          event.preventDefault()
+        })
+        await expect(remotely(`require('electron').remote.getGlobal('test')`)).to.eventually.be.rejected(`Blocked remote.getGlobal('test')`)
       })
-      await expect(remotely(`require('electron').remote.getGlobal('test')`)).to.eventually.be.rejected(`Blocked remote.getGlobal('test')`)
     })
-  })
 
-  describe('remote.getBuiltin filtering', () => {
-    it('can return custom values', async () => {
-      w.webContents.once('remote-get-builtin', (event, name) => {
-        event.returnValue = name
+    describe('remote.getBuiltin filtering', () => {
+      it('can return custom values', async () => {
+        w.webContents.once('remote-get-builtin', (event, name) => {
+          event.returnValue = name
+        })
+        expect(await remotely(`require('electron').remote.getBuiltin('test')`)).to.equal('test')
       })
-      expect(await remotely(`require('electron').remote.getBuiltin('test')`)).to.equal('test')
-    })
 
-    it('throws when no returnValue set', async () => {
-      w.webContents.once('remote-get-builtin', (event, name) => {
-        event.preventDefault()
+      it('throws when no returnValue set', async () => {
+        w.webContents.once('remote-get-builtin', (event, name) => {
+          event.preventDefault()
+        })
+        await expect(remotely(`require('electron').remote.getBuiltin('test')`)).to.eventually.be.rejected(`Blocked remote.getGlobal('test')`)
       })
-      await expect(remotely(`require('electron').remote.getBuiltin('test')`)).to.eventually.be.rejected(`Blocked remote.getGlobal('test')`)
     })
-  })
 
-  describe('remote.require filtering', () => {
-    it('can return custom values', async () => {
-      w.webContents.once('remote-require', (event, name) => {
-        event.returnValue = name
+    describe('remote.require filtering', () => {
+      it('can return custom values', async () => {
+        w.webContents.once('remote-require', (event, name) => {
+          event.returnValue = name
+        })
+        expect(await remotely(`require('electron').remote.require('test')`)).to.equal('test')
       })
-      expect(await remotely(`require('electron').remote.require('test')`)).to.equal('test')
-    })
 
-    it('throws when no returnValue set', async () => {
-      w.webContents.once('remote-require', (event, name) => {
-        event.preventDefault()
+      it('throws when no returnValue set', async () => {
+        w.webContents.once('remote-require', (event, name) => {
+          event.preventDefault()
+        })
+        await expect(remotely(`require('electron').remote.require('test')`)).to.eventually.be.rejected(`Blocked remote.require('test')`)
       })
-      await expect(remotely(`require('electron').remote.require('test')`)).to.eventually.be.rejected(`Blocked remote.require('test')`)
     })
-  })
 
-  describe('remote.getCurrentWindow filtering', () => {
-    it('can return custom value', async () => {
-      w.webContents.once('remote-get-current-window', (e) => {
-        e.returnValue = 'some window'
+    describe('remote.getCurrentWindow filtering', () => {
+      it('can return custom value', async () => {
+        w.webContents.once('remote-get-current-window', (e) => {
+          e.returnValue = 'some window'
+        })
+        expect(await remotely(`require('electron').remote.getCurrentWindow()`)).to.equal('some window')
       })
-      expect(await remotely(`require('electron').remote.getCurrentWindow()`)).to.equal('some window')
-    })
 
-    it('throws when no returnValue set', async () => {
-      w.webContents.once('remote-get-current-window', (event) => {
-        event.preventDefault()
+      it('throws when no returnValue set', async () => {
+        w.webContents.once('remote-get-current-window', (event) => {
+          event.preventDefault()
+        })
+        await expect(remotely(`require('electron').remote.getCurrentWindow()`)).to.eventually.be.rejected(`Blocked remote.getCurrentWindow()`)
       })
-      await expect(remotely(`require('electron').remote.getCurrentWindow()`)).to.eventually.be.rejected(`Blocked remote.getCurrentWindow()`)
     })
-  })
 
-  describe('remote.getCurrentWebContents filtering', () => {
-    it('can return custom value', async () => {
-      w.webContents.once('remote-get-current-web-contents', (event) => {
-        event.returnValue = 'some web contents'
+    describe('remote.getCurrentWebContents filtering', () => {
+      it('can return custom value', async () => {
+        w.webContents.once('remote-get-current-web-contents', (event) => {
+          event.returnValue = 'some web contents'
+        })
+        expect(await remotely(`require('electron').remote.getCurrentWebContents()`)).to.equal('some web contents')
+      })
+
+      it('throws when no returnValue set', async () => {
+        w.webContents.once('remote-get-current-web-contents', (event) => {
+          event.preventDefault()
+        })
+        await expect(remotely(`require('electron').remote.getCurrentWebContents()`)).to.eventually.be.rejected(`Blocked remote.getCurrentWebContents()`)
       })
-      expect(await remotely(`require('electron').remote.getCurrentWebContents()`)).to.equal('some web contents')
     })
 
-    it('throws when no returnValue set', async () => {
-      w.webContents.once('remote-get-current-web-contents', (event) => {
-        event.preventDefault()
+    describe('remote references', () => {
+      it('render-view-deleted is sent when page is destroyed', (done) => {
+        w.webContents.once('render-view-deleted' as any, () => {
+          done()
+        })
+        w.destroy()
+      })
+
+      // The ELECTRON_BROWSER_CONTEXT_RELEASE message relies on this to work.
+      it('message can be sent on exit when page is being navigated', async () => {
+        after(() => { ipcMain.removeAllListeners('SENT_ON_EXIT') })
+        await emittedOnce(w.webContents, 'did-finish-load')
+        w.webContents.once('did-finish-load', () => {
+          w.webContents.loadURL('about:blank')
+        })
+        w.loadFile(path.join(fixtures, 'api', 'send-on-exit.html'))
+        await emittedOnce(ipcMain, 'SENT_ON_EXIT')
       })
-      await expect(remotely(`require('electron').remote.getCurrentWebContents()`)).to.eventually.be.rejected(`Blocked remote.getCurrentWebContents()`)
     })
   })
 
-  describe('remote references', () => {
-    it('render-view-deleted is sent when page is destroyed', (done) => {
-      w.webContents.once('render-view-deleted' as any, () => {
-        done()
+  describe('remote function in renderer', () => {
+    afterEach(() => {
+      ipcMain.removeAllListeners('done')
+    })
+    afterEach(closeAllWindows)
+
+    it('works when created in preload script', async () => {
+      const preload = path.join(fixtures, 'module', 'preload-remote-function.js')
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          preload
+        }
       })
-      w.destroy()
+      w.loadURL('about:blank')
+      await emittedOnce(ipcMain, 'done')
     })
+  })
 
-    // The ELECTRON_BROWSER_CONTEXT_RELEASE message relies on this to work.
-    it('message can be sent on exit when page is being navigated', (done) => {
-      after(() => { ipcMain.removeAllListeners('SENT_ON_EXIT') })
-      ipcMain.once('SENT_ON_EXIT', () => {
-        done()
-      })
-      w.webContents.once('did-finish-load', () => {
-        w.webContents.loadURL('about:blank')
+  describe('remote listeners', () => {
+    afterEach(closeAllWindows)
+
+    it('detaches listeners subscribed to destroyed renderers, and shows a warning', async () => {
+      const w = new BrowserWindow({
+        show: false,
+        webPreferences: {
+          nodeIntegration: true
+        }
       })
-      w.loadFile(path.join(fixtures, 'api', 'send-on-exit.html'))
+      await w.loadFile(path.join(__dirname, '..', 'spec', 'fixtures', 'api', 'remote-event-handler.html'))
+      w.webContents.reload()
+      await emittedOnce(w.webContents, 'did-finish-load')
+
+      const expectedMessage = [
+        'Attempting to call a function in a renderer window that has been closed or released.',
+        'Function provided here: remote-event-handler.html:11:33',
+        'Remote event names: remote-handler, other-remote-handler'
+      ].join('\n')
+
+      expect(w.webContents.listenerCount('remote-handler')).to.equal(2)
+      let warnMessage: string | null = null
+      let originalWarn = console.warn
+      try {
+        console.warn = (message: string) => warnMessage = message
+        w.webContents.emit('remote-handler', { sender: w.webContents })
+      } finally {
+        console.warn = originalWarn
+      }
+      expect(w.webContents.listenerCount('remote-handler')).to.equal(1)
+      expect(warnMessage).to.equal(expectedMessage)
     })
   })
 })

spec/api-remote-spec.js

@@ -3,7 +3,6 @@
 const chai = require('chai')
 const dirtyChai = require('dirty-chai')
 const path = require('path')
-const { closeWindow } = require('./window-helpers')
 const { resolveGetters } = require('./expect-helpers')
 const { ifdescribe } = require('./spec-helpers')
 
@@ -512,68 +511,10 @@ ifdescribe(features.isRemoteModuleEnabled())('remote module', () => {
     })
   })
 
-  describe('remote function in renderer', () => {
-    let w = null
-
-    afterEach(() => closeWindow(w).then(() => { w = null }))
-    afterEach(() => {
-      ipcMain.removeAllListeners('done')
-    })
-
-    it('works when created in preload script', (done) => {
-      ipcMain.once('done', () => w.close())
-      const preload = path.join(fixtures, 'module', 'preload-remote-function.js')
-      w = new BrowserWindow({
-        show: false,
-        webPreferences: {
-          preload
-        }
-      })
-      w.once('closed', () => done())
-      w.loadURL('about:blank')
-    })
-  })
-
   describe('constructing a Uint8Array', () => {
     it('does not crash', () => {
       const RUint8Array = remote.getGlobal('Uint8Array')
       const arr = new RUint8Array()
     })
   })
-
-  describe('remote listeners', () => {
-    let w = null
-    afterEach(() => closeWindow(w).then(() => { w = null }))
-
-    it('detaches listeners subscribed to destroyed renderers, and shows a warning', (done) => {
-      w = new BrowserWindow({
-        show: false,
-        webPreferences: {
-          nodeIntegration: true
-        }
-      })
-
-      w.webContents.once('did-finish-load', () => {
-        w.webContents.once('did-finish-load', () => {
-          const expectedMessage = [
-            'Attempting to call a function in a renderer window that has been closed or released.',
-            'Function provided here: remote-event-handler.html:11:33',
-            'Remote event names: remote-handler, other-remote-handler'
-          ].join('\n')
-
-          const results = ipcRenderer.sendSync('try-emit-web-contents-event', w.webContents.id, 'remote-handler')
-
-          expect(results).to.deep.equal({
-            warningMessage: expectedMessage,
-            listenerCountBefore: 2,
-            listenerCountAfter: 1
-          })
-          done()
-        })
-
-        w.webContents.reload()
-      })
-      w.loadFile(path.join(fixtures, 'api', 'remote-event-handler.html'))
-    })
-  })
 })

spec/static/main.js

@@ -200,25 +200,6 @@ ipcMain.on('disable-preload-on-next-will-attach-webview', (event, id) => {
   })
 })
 
-ipcMain.on('try-emit-web-contents-event', (event, id, eventName) => {
-  const consoleWarn = console.warn
-  const contents = webContents.fromId(id)
-  const listenerCountBefore = contents.listenerCount(eventName)
-
-  console.warn = (warningMessage) => {
-    console.warn = consoleWarn
-
-    const listenerCountAfter = contents.listenerCount(eventName)
-    event.returnValue = {
-      warningMessage,
-      listenerCountBefore,
-      listenerCountAfter
-    }
-  }
-
-  contents.emit(eventName, { sender: contents })
-})
-
 ipcMain.on('handle-uncaught-exception', (event, message) => {
   suspendListeners(process, 'uncaughtException', (error) => {
     event.returnValue = error.message

spec/window-helpers.js

@@ -1,44 +0,0 @@
-const { expect } = require('chai')
-const { remote } = require('electron')
-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 }) => {
-  await ensureWindowIsClosed(window)
-
-  if (assertSingleWindow) {
-    // Although we want to assert that no windows were left handing around
-    // we also want to clean up the left over windows so that no future
-    // tests fail as a side effect
-    const currentId = remote.getCurrentWindow().id
-    const windows = BrowserWindow.getAllWindows()
-    for (const win of windows) {
-      if (win.id !== currentId) {
-        await ensureWindowIsClosed(win)
-      }
-    }
-    expect(windows).to.have.lengthOf(1)
-  }
-}