From ac501e8194c2f2dbfddbfe6cee7608274e7aea5c Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Thu, 26 Mar 2020 18:30:21 -0700 Subject: [PATCH] fix: heap-use-after-free in tray.popUpContextMenu (#22842) --- shell/browser/ui/tray_icon_cocoa.mm | 8 ++++++++ spec-main/api-tray-spec.ts | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/shell/browser/ui/tray_icon_cocoa.mm b/shell/browser/ui/tray_icon_cocoa.mm index 56107ce538bd..cdea63c7c4ea 100644 --- a/shell/browser/ui/tray_icon_cocoa.mm +++ b/shell/browser/ui/tray_icon_cocoa.mm @@ -181,8 +181,16 @@ useDefaultAccelerator:NO]); // Hacky way to mimic design of ordinary tray menu. [statusItem_ setMenu:[menuController menu]]; + // -performClick: is a blocking call, which will run the task loop inside + // itself. This can potentially include running JS, which can result in + // this object being released. We take a temporary reference here to make + // sure we stay alive long enough to successfully return from this + // function. + // TODO(nornagon/codebytere): Avoid nesting task loops here. + [self retain]; [[statusItem_ button] performClick:self]; [statusItem_ setMenu:[menuController_ menu]]; + [self release]; return; } diff --git a/spec-main/api-tray-spec.ts b/spec-main/api-tray-spec.ts index 4d90c10ceaa6..35e73c7bc1ba 100644 --- a/spec-main/api-tray-spec.ts +++ b/spec-main/api-tray-spec.ts @@ -73,6 +73,13 @@ describe('tray module', () => { }); tray.popUpContextMenu(); }); + + it('can be called with a menu', () => { + const menu = Menu.buildFromTemplate([{ label: 'Test' }]); + expect(() => { + tray.popUpContextMenu(menu); + }).to.not.throw(); + }); }); describe('tray.closeContextMenu()', () => {