fix: Invalidate weak ptrs before window Javascript object is destroyed (#14532)

* fix: Invalidate weak ptrs before window Javascript object is destroyed

* chore: add regression test for #14513

This test is similar to the original gist at https://gist.github.com/bpasero/a02a645e11f4946dcca1331d0299149d -- the key is to open multiple windows and add an `app.on('browser-window-focus') listener that accesses window.id.

* fix: last commit didn't test the right thing.

The test needs to run in the main process to reproduce the
conditions reported in #14513

atom/browser/api/atom_api_top_level_window.cc

@@ -142,6 +142,11 @@ void TopLevelWindow::WillCloseWindow(bool* prevent_default) {
 }
 
 void TopLevelWindow::OnWindowClosed() {
+  // Invalidate weak ptrs before the Javascript object is destroyed,
+  // there might be some delayed emit events which shouldn't be
+  // triggered after this.
+  weak_factory_.InvalidateWeakPtrs();
+
   RemoveFromWeakMap();
   window_->RemoveObserver(this);
 

spec/api-browser-window-spec.js

@@ -221,6 +221,11 @@ describe('BrowserWindow module', () => {
         contents.getProcessId()
       }, /Object has been destroyed/)
     })
+    it('should not crash when destroying windows with pending events', (done) => {
+      const responseEvent = 'destroy-test-completed'
+      ipcRenderer.on(responseEvent, () => done())
+      ipcRenderer.send('test-browserwindow-destroy', { responseEvent })
+    })
   })
 
   describe('BrowserWindow.loadURL(url)', () => {

spec/static/main.js

@@ -381,6 +381,26 @@ ipcMain.on('test-webcontents-navigation-observer', (event, options) => {
   contents.loadURL(options.url)
 })
 
+ipcMain.on('test-browserwindow-destroy', (event, testOptions) => {
+  let focusListener = (event, win) => win.id
+  app.on('browser-window-focus', focusListener)
+  const windowCount = 3
+  const windowOptions = {
+    show: false,
+    width: 400,
+    height: 400,
+    webPreferences: {
+      backgroundThrottling: false
+    }
+  }
+  const windows = Array.from(Array(windowCount)).map(x => new BrowserWindow(windowOptions))
+  windows.forEach(win => win.show())
+  windows.forEach(win => win.focus())
+  windows.forEach(win => win.destroy())
+  app.removeListener('browser-window-focus', focusListener)
+  event.sender.send(testOptions.responseEvent)
+})
+
 // Suspend listeners until the next event and then restore them
 const suspendListeners = (emitter, eventName, callback) => {
   const listeners = emitter.listeners(eventName)