From 18f717865c4ef228e397e666af2de55e4411de45 Mon Sep 17 00:00:00 2001
From: Samuel Attard <samuel.r.attard@gmail.com>
Date: Tue, 21 Apr 2020 14:22:58 -0700
Subject: [PATCH] fix: ensure that functions are not retained beyond their
 context being released

---
 .../render_frame_function_store.cc            | 10 +++++
 .../render_frame_function_store.h             |  2 +
 spec-main/api-context-bridge-spec.ts          | 39 ++++++++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/shell/renderer/api/context_bridge/render_frame_function_store.cc b/shell/renderer/api/context_bridge/render_frame_function_store.cc
index 804ccaf6fdfa1..6131544f590d6 100644
--- a/shell/renderer/api/context_bridge/render_frame_function_store.cc
+++ b/shell/renderer/api/context_bridge/render_frame_function_store.cc
@@ -32,6 +32,16 @@ void RenderFrameFunctionStore::OnDestruct() {
   delete this;
 }
 
+void RenderFrameFunctionStore::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;
+  });
+}
+
 }  // namespace context_bridge
 
 }  // namespace api
diff --git a/shell/renderer/api/context_bridge/render_frame_function_store.h b/shell/renderer/api/context_bridge/render_frame_function_store.h
index e4b9ea436a0a0..f121e726ac686 100644
--- a/shell/renderer/api/context_bridge/render_frame_function_store.h
+++ b/shell/renderer/api/context_bridge/render_frame_function_store.h
@@ -29,6 +29,8 @@ class RenderFrameFunctionStore 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_++; }
 
diff --git a/spec-main/api-context-bridge-spec.ts b/spec-main/api-context-bridge-spec.ts
index c471d5cb8a5b7..b6565f326db85 100644
--- a/spec-main/api-context-bridge-spec.ts
+++ b/spec-main/api-context-bridge-spec.ts
@@ -2,17 +2,33 @@ import { BrowserWindow, ipcMain } from 'electron/main';
 import { contextBridge } from 'electron/renderer';
 import { expect } from 'chai';
 import * as fs from 'fs-extra';
+import * as http from 'http';
 import * as os from 'os';
 import * as path from 'path';
 
 import { closeWindow } from './window-helpers';
 import { emittedOnce } from './events-helpers';
+import { AddressInfo } from 'net';
 
 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);
@@ -65,7 +81,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 = (fn: Function) =>
@@ -343,6 +359,27 @@ describe('contextBridge', () => {
         });
       }
 
+      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;