From be3c2fd0af60622444c3491f874ad8821faf9d65 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Thu, 22 Apr 2021 13:46:41 -0700 Subject: [PATCH] fix: shell.trashItem crash when called in renderer (#28748) --- shell/common/platform_util.cc | 30 +++++++++++++++++++----------- spec-main/api-shell-spec.ts | 8 ++++++++ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/shell/common/platform_util.cc b/shell/common/platform_util.cc index 0f092088ad55..276e43b4421e 100644 --- a/shell/common/platform_util.cc +++ b/shell/common/platform_util.cc @@ -14,24 +14,32 @@ namespace platform_util { -void TrashItemOnBlockingThread( - const base::FilePath& full_path, - base::OnceCallback callback) { +struct TrashItemResult { + bool success; + std::string error; +}; + +TrashItemResult TrashItemOnBlockingThread(const base::FilePath& full_path) { std::string error; bool success = internal::PlatformTrashItem(full_path, &error); - content::GetUIThreadTaskRunner({})->PostTask( - FROM_HERE, base::BindOnce(std::move(callback), success, error)); + return {success, error}; } void TrashItem(const base::FilePath& full_path, base::OnceCallback callback) { // XXX: is continue_on_shutdown right? - base::ThreadPool::PostTask(FROM_HERE, - {base::MayBlock(), base::WithBaseSyncPrimitives(), - base::TaskPriority::USER_BLOCKING, - base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, - base::BindOnce(&TrashItemOnBlockingThread, - full_path, std::move(callback))); + base::ThreadPool::PostTaskAndReplyWithResult( + FROM_HERE, + {base::MayBlock(), base::WithBaseSyncPrimitives(), + base::TaskPriority::USER_BLOCKING, + base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, + base::BindOnce(&TrashItemOnBlockingThread, full_path), + base::BindOnce( + [](base::OnceCallback callback, + TrashItemResult result) { + std::move(callback).Run(result.success, result.error); + }, + std::move(callback))); } } // namespace platform_util diff --git a/spec-main/api-shell-spec.ts b/spec-main/api-shell-spec.ts index de60e4e2b2e6..764802bbc339 100644 --- a/spec-main/api-shell-spec.ts +++ b/spec-main/api-shell-spec.ts @@ -62,6 +62,8 @@ describe('shell module', () => { }); describe('shell.trashItem()', () => { + afterEach(closeAllWindows); + it('moves an item to the trash', async () => { const dir = await fs.mkdtemp(path.resolve(app.getPath('temp'), 'electron-shell-spec-')); const filename = path.join(dir, 'temp-to-be-deleted'); @@ -74,5 +76,11 @@ describe('shell module', () => { const filename = path.join(app.getPath('temp'), 'does-not-exist'); await expect(shell.trashItem(filename)).to.eventually.be.rejected(); }); + + it('works in the renderer process', async () => { + const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true, contextIsolation: false } }); + w.loadURL('about:blank'); + await expect(w.webContents.executeJavaScript('require(\'electron\').shell.trashItem(\'does-not-exist\')')).to.be.rejectedWith(/does-not-exist|Failed to move item|Failed to create FileOperation/); + }); }); });