From 45471014cb22d0a64048777657ebe0cd7be95237 Mon Sep 17 00:00:00 2001
From: Markus Olsson <j.markus.olsson@gmail.com>
Date: Thu, 27 Aug 2020 09:55:33 -0700
Subject: [PATCH] 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();
+    });
   });
 });