fix: ensure that functions are not retained beyond their context being released (#23207) (#23232)

shell/renderer/api/context_bridge/render_frame_context_bridge_store.cc

@@ -92,6 +92,16 @@ void RenderFramePersistenceStore::OnDestruct() {
   delete this;
 }
 
+void RenderFramePersistenceStore::WillReleaseScriptContext(
+    v8::Local<v8::Context> context,
+    int32_t world_id) {
+  base::EraseIf(functions_, [context](auto const& pair) {
+    v8::Local<v8::Context> func_owning_context =
+        std::get<1>(pair.second).Get(context->GetIsolate());
+    return func_owning_context == context;
+  });
+}
+
 void RenderFramePersistenceStore::CacheProxiedObject(
     v8::Local<v8::Value> from,
     v8::Local<v8::Value> proxy_value) {

shell/renderer/api/context_bridge/render_frame_context_bridge_store.h

@@ -40,6 +40,8 @@ class RenderFramePersistenceStore final : public content::RenderFrameObserver {
 
   // RenderFrameObserver implementation.
   void OnDestruct() override;
+  void WillReleaseScriptContext(v8::Local<v8::Context> context,
+                                int32_t world_id) override;
 
   size_t take_func_id() { return next_func_id_++; }
 

spec-main/api-context-bridge-spec.ts

@@ -1,6 +1,8 @@
 import { contextBridge, BrowserWindow, ipcMain } from 'electron'
 import { expect } from 'chai'
 import * as fs from 'fs-extra'
+import * as http from 'http'
+import { AddressInfo } from 'net'
 import * as os from 'os'
 import * as path from 'path'
 
@@ -12,6 +14,20 @@ const fixturesPath = path.resolve(__dirname, 'fixtures', 'api', 'context-bridge'
 describe('contextBridge', () => {
   let w: BrowserWindow
   let dir: string
+  let server: http.Server
+
+  before(async () => {
+    server = http.createServer((req, res) => {
+      res.setHeader('Content-Type', 'text/html')
+      res.end('')
+    })
+    await new Promise(resolve => server.listen(0, resolve))
+  })
+
+  after(async () => {
+    if (server) await new Promise(resolve => server.close(resolve))
+    server = null as any
+  })
 
   afterEach(async () => {
     await closeWindow(w)
@@ -64,7 +80,7 @@ describe('contextBridge', () => {
             preload: path.resolve(tmpDir, 'preload.js')
           }
         })
-        await w.loadFile(path.resolve(fixturesPath, 'empty.html'))
+        await w.loadURL(`http://127.0.0.1:${(server.address() as AddressInfo).port}`)
       }
     
       const callWithBindings = async (fn: Function) => {
@@ -503,7 +519,28 @@ describe('contextBridge', () => {
           expect(info.objectCount).to.equal(6)
         })
       }
-    
+
+      if (useSandbox) {
+        it('should not leak the global hold on methods sent across contexts when reloading a sandboxed renderer', async () => {
+          await makeBindingWindow(() => {
+            require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()))
+            contextBridge.exposeInMainWorld('example', {
+              getFunction: () => () => 123
+            })
+            require('electron').ipcRenderer.send('window-ready-for-tasking')
+          })
+          const loadPromise = emittedOnce(ipcMain, 'window-ready-for-tasking')
+          expect((await getGCInfo()).functionCount).to.equal(1)
+          await callWithBindings((root: any) => {
+            root.location.reload()
+          })
+          await loadPromise
+          // If this is ever "2" it means we leaked the exposed function and
+          // therefore the entire context after a reload
+          expect((await getGCInfo()).functionCount).to.equal(1)
+        })
+      }
+
       it('it should not let you overwrite existing exposed things', async () => {
         await makeBindingWindow(() => {
           let threw = false