From 902af1ac07585ca7d7feeccb57b0d2e9b50ffce1 Mon Sep 17 00:00:00 2001 From: Jeremy Rose Date: Wed, 26 Aug 2020 09:34:29 -0700 Subject: [PATCH 1/2] test: add tests for shell.moveItemToTrash (#25113) --- shell/common/platform_util_linux.cc | 3 +++ spec-main/api-shell-spec.ts | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/shell/common/platform_util_linux.cc b/shell/common/platform_util_linux.cc index df2a744b0e01c..70503bde126ae 100644 --- a/shell/common/platform_util_linux.cc +++ b/shell/common/platform_util_linux.cc @@ -12,6 +12,7 @@ #include "base/nix/xdg_util.h" #include "base/process/kill.h" #include "base/process/launch.h" +#include "base/threading/thread_restrictions.h" #include "ui/gtk/gtk_util.h" #include "url/gurl.h" @@ -51,6 +52,8 @@ bool XDGUtil(const std::vector& argv, return false; if (wait_for_exit) { + base::ScopedAllowBaseSyncPrimitivesForTesting + allow_sync; // required by WaitForExit int exit_code = -1; bool success = process.WaitForExit(&exit_code); if (!callback.is_null()) diff --git a/spec-main/api-shell-spec.ts b/spec-main/api-shell-spec.ts index 747add42f9d5c..29a052837aafe 100644 --- a/spec-main/api-shell-spec.ts +++ b/spec-main/api-shell-spec.ts @@ -1,9 +1,12 @@ -import { BrowserWindow } from 'electron/main'; +import { BrowserWindow, app } from 'electron/main'; import { shell } from 'electron/common'; import { closeAllWindows } from './window-helpers'; import { emittedOnce } from './events-helpers'; import * as http from 'http'; +import * as fs from 'fs-extra'; +import * as path from 'path'; import { AddressInfo } from 'net'; +import { expect } from 'chai'; describe('shell module', () => { describe('shell.openExternal()', () => { @@ -57,4 +60,21 @@ describe('shell module', () => { ]); }); }); + + describe('shell.moveItemToTrash()', () => { + 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'); + await fs.writeFile(filename, 'dummy-contents'); + const result = shell.moveItemToTrash(filename); + expect(result).to.be.true(); + expect(fs.existsSync(filename)).to.be.false(); + }); + + it('returns false when called with a nonexistent path', () => { + const filename = path.join(app.getPath('temp'), 'does-not-exist'); + const result = shell.moveItemToTrash(filename); + expect(result).to.be.false(); + }); + }); }); From ea5209983fac968083de8ceecd10f9155c9481ee Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 27 Aug 2020 09:55:33 -0700 Subject: [PATCH 2/2] fix: make shell.moveItemToTrash return false on Windows when move unsuccessful (#25124) --- shell/common/platform_util_win.cc | 6 +++++- spec-main/api-shell-spec.ts | 35 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/shell/common/platform_util_win.cc b/shell/common/platform_util_win.cc index edf6aaace30e2..de3e71b0a9864 100644 --- a/shell/common/platform_util_win.cc +++ b/shell/common/platform_util_win.cc @@ -387,10 +387,14 @@ bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) { if (!delete_sink) return false; + BOOL pfAnyOperationsAborted; + // Processes the queued command DeleteItem. This will trigger // the DeleteFileProgressSink to check for Recycle Bin. return SUCCEEDED(pfo->DeleteItem(delete_item.Get(), delete_sink.Get())) && - SUCCEEDED(pfo->PerformOperations()); + SUCCEEDED(pfo->PerformOperations()) && + SUCCEEDED(pfo->GetAnyOperationsAborted(&pfAnyOperationsAborted)) && + !pfAnyOperationsAborted; } bool GetFolderPath(int key, base::FilePath* result) { diff --git a/spec-main/api-shell-spec.ts b/spec-main/api-shell-spec.ts index 29a052837aafe..2392d0ea5e9c6 100644 --- a/spec-main/api-shell-spec.ts +++ b/spec-main/api-shell-spec.ts @@ -7,6 +7,8 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import { AddressInfo } from 'net'; import { expect } from 'chai'; +import { ifit } from './spec-helpers'; +import { execSync } from 'child_process'; describe('shell module', () => { describe('shell.openExternal()', () => { @@ -76,5 +78,38 @@ describe('shell module', () => { const result = shell.moveItemToTrash(filename); expect(result).to.be.false(); }); + + ifit(process.platform === 'darwin')('returns false when file has immutable flag', async () => { + const dir = await fs.mkdtemp(path.resolve(app.getPath('temp'), 'electron-shell-spec-')); + const tempPath = path.join(dir, 'locked-file'); + await fs.writeFile(tempPath, 'delete me if you can'); + + // https://ss64.com/osx/chflags.html + execSync(`chflags uchg ${tempPath}`); + expect(shell.moveItemToTrash(tempPath)).to.be.false(); + expect(await fs.pathExists(tempPath)).to.be.true(); + + execSync(`chflags nouchg ${tempPath}`); + expect(shell.moveItemToTrash(tempPath)).to.be.true(); + expect(await fs.pathExists(tempPath)).to.be.false(); + }); + + ifit(process.platform === 'win32')('returns false when path is in use', async () => { + const tempPath = await fs.mkdtemp(path.resolve(app.getPath('temp'), 'electron-shell-spec-')); + const cwd = process.cwd(); + try { + // A process working directory is automatically locked on Windows. + // This is a workaround to avoid pulling in fs-extras flock method. + process.chdir(tempPath); + + expect(shell.moveItemToTrash(tempPath)).to.be.false(); + expect(await fs.pathExists(tempPath)).to.be.true(); + } finally { + process.chdir(cwd); + } + + expect(shell.moveItemToTrash(tempPath)).to.be.true(); + expect(await fs.pathExists(tempPath)).to.be.false(); + }); }); });