feat: add shell.trashItem() to replace shell.moveItemToTrash() (#25114)

docs/api/shell.md

@@ -45,15 +45,27 @@ Returns `Promise<void>`
 
 Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent).
 
-### `shell.moveItemToTrash(fullPath[, deleteOnFail])`
+### `shell.moveItemToTrash(fullPath[, deleteOnFail])` _Deprecated_
 
 * `fullPath` String
 * `deleteOnFail` Boolean (optional) - Whether or not to unilaterally remove the item if the Trash is disabled or unsupported on the volume. _macOS_
 
 Returns `Boolean` - Whether the item was successfully moved to the trash or otherwise deleted.
 
+> NOTE: This method is deprecated. Use `shell.trashItem` instead.
+
 Move the given file to trash and returns a boolean status for the operation.
 
+### `shell.trashItem(path)`
+
+* `path` String - path to the item to be moved to the trash.
+
+Returns `Promise<void>` - Resolves when the operation has been completed.
+Rejects if there was an error while deleting the requested item.
+
+This moves a path to the OS-specific trash location (Trash on macOS, Recycle
+Bin on Windows, and a desktop-environment-specific location on Linux).
+
 ### `shell.beep()`
 
 Play the beep sound.

filenames.gni

@@ -565,7 +565,9 @@ filenames = {
     "shell/common/node_util.h",
     "shell/common/options_switches.cc",
     "shell/common/options_switches.h",
+    "shell/common/platform_util.cc",
     "shell/common/platform_util.h",
+    "shell/common/platform_util_internal.h",
     "shell/common/platform_util_linux.cc",
     "shell/common/platform_util_mac.mm",
     "shell/common/platform_util_win.cc",

lib/common/api/shell.ts

@@ -1 +1,7 @@
-export default process._linkedBinding('electron_common_shell');
+import { deprecate } from 'electron/main';
+
+const shell = process._linkedBinding('electron_common_shell');
+
+shell.moveItemToTrash = deprecate.renameFunction(shell.moveItemToTrash, 'shell.trashItem');
+
+export default shell;

shell/common/api/electron_api_shell.cc

@@ -94,6 +94,25 @@ bool MoveItemToTrash(gin::Arguments* args) {
   return platform_util::MoveItemToTrash(full_path, delete_on_fail);
 }
 
+v8::Local<v8::Promise> TrashItem(v8::Isolate* isolate,
+                                 const base::FilePath& path) {
+  gin_helper::Promise<void> promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  platform_util::TrashItem(
+      path, base::BindOnce(
+                [](gin_helper::Promise<void> promise, bool success,
+                   const std::string& error) {
+                  if (success) {
+                    promise.Resolve();
+                  } else {
+                    promise.RejectWithErrorMessage(error);
+                  }
+                },
+                std::move(promise)));
+  return handle;
+}
+
 #if defined(OS_WIN)
 bool WriteShortcutLink(const base::FilePath& shortcut_path,
                        gin_helper::Arguments* args) {
@@ -158,6 +177,7 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("openPath", &OpenPath);
   dict.SetMethod("openExternal", &OpenExternal);
   dict.SetMethod("moveItemToTrash", &MoveItemToTrash);
+  dict.SetMethod("trashItem", &TrashItem);
   dict.SetMethod("beep", &platform_util::Beep);
 #if defined(OS_WIN)
   dict.SetMethod("writeShortcutLink", &WriteShortcutLink);

shell/common/platform_util.cc

@@ -0,0 +1,37 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/common/platform_util.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/task/thread_pool.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
+#include "shell/common/platform_util_internal.h"
+
+namespace platform_util {
+
+void TrashItemOnBlockingThread(
+    const base::FilePath& full_path,
+    base::OnceCallback<void(bool, const std::string&)> callback) {
+  std::string error;
+  bool success = internal::PlatformTrashItem(full_path, &error);
+  content::GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), success, error));
+}
+
+void TrashItem(const base::FilePath& full_path,
+               base::OnceCallback<void(bool, const std::string&)> 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)));
+}
+
+}  // namespace platform_util

shell/common/platform_util.h

@@ -40,9 +40,13 @@ void OpenExternal(const GURL& url,
                   const OpenExternalOptions& options,
                   OpenCallback callback);
 
-// Move a file to trash.
+// Move a file to trash. (Deprecated.)
 bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail);
 
+// Move a file to trash, asynchronously.
+void TrashItem(const base::FilePath& full_path,
+               base::OnceCallback<void(bool, const std::string&)> callback);
+
 void Beep();
 
 #if defined(OS_WIN)

shell/common/platform_util_internal.h

@@ -0,0 +1,26 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_COMMON_PLATFORM_UTIL_INTERNAL_H_
+#define SHELL_COMMON_PLATFORM_UTIL_INTERNAL_H_
+
+#include "shell/common/platform_util.h"
+
+#include <string>
+
+namespace base {
+class FilePath;
+}
+
+namespace platform_util {
+namespace internal {
+
+// Called by platform_util.cc on to invoke platform specific logic to move
+// |path| to trash using a suitable handler.
+bool PlatformTrashItem(const base::FilePath& path, std::string* error);
+
+}  // namespace internal
+}  // namespace platform_util
+
+#endif  // SHELL_COMMON_PLATFORM_UTIL_INTERNAL_H_

shell/common/platform_util_linux.cc

@@ -19,6 +19,7 @@
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
+#include "shell/common/platform_util_internal.h"
 #include "ui/gtk/gtk_util.h"
 #include "url/gurl.h"
 
@@ -220,6 +221,19 @@ bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
   return XDGUtil(argv, base::FilePath(), true, platform_util::OpenCallback());
 }
 
+namespace internal {
+
+bool PlatformTrashItem(const base::FilePath& full_path, std::string* error) {
+  if (!MoveItemToTrash(full_path, false)) {
+    // TODO(nornagon): at least include the exit code?
+    *error = "Failed to move item to trash";
+    return false;
+  }
+  return true;
+}
+
+}  // namespace internal
+
 void Beep() {
   // echo '\a' > /dev/console
   FILE* fp = fopen("/dev/console", "a");

shell/common/platform_util_mac.mm

@@ -117,10 +117,13 @@ void OpenExternal(const GURL& url,
                  });
 }
 
-bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
+bool MoveItemToTrashWithError(const base::FilePath& full_path,
+                              bool delete_on_fail,
+                              std::string* error) {
   NSString* path_string = base::SysUTF8ToNSString(full_path.value());
   if (!path_string) {
-    LOG(WARNING) << "Invalid file path " << full_path.value();
+    *error = "Invalid file path: " + full_path.value();
+    LOG(WARNING) << *error;
     return false;
   }
 
@@ -142,14 +145,27 @@ bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
   }
 
   if (!did_trash) {
+    *error = base::SysNSStringToUTF8([err localizedDescription]);
     LOG(WARNING) << "NSWorkspace failed to move file " << full_path.value()
-                 << " to trash: "
+                 << " to trash: " << *error;
-                 << base::SysNSStringToUTF8([err localizedDescription]);
   }
 
   return did_trash;
 }
 
+bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) {
+  std::string error;  // ignored
+  return MoveItemToTrashWithError(path, delete_on_fail, &error);
+}
+
+namespace internal {
+
+bool PlatformTrashItem(const base::FilePath& full_path, std::string* error) {
+  return MoveItemToTrashWithError(full_path, false, error);
+}
+
+}  // namespace internal
+
 void Beep() {
   NSBeep();
 }

shell/common/platform_util_win.cc

@@ -347,15 +347,15 @@ void OpenExternal(const GURL& url,
       std::move(callback));
 }
 
-bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) {
+bool MoveItemToTrashWithError(const base::FilePath& path,
-  base::win::ScopedCOMInitializer com_initializer;
+                              bool delete_on_fail,
-  if (!com_initializer.Succeeded())
+                              std::string* error) {
-    return false;
-
   Microsoft::WRL::ComPtr<IFileOperation> pfo;
   if (FAILED(::CoCreateInstance(CLSID_FileOperation, nullptr, CLSCTX_ALL,
-                                IID_PPV_ARGS(&pfo))))
+                                IID_PPV_ARGS(&pfo)))) {
+    *error = "Failed to create FileOperation instance";
     return false;
+  }
 
   // Elevation prompt enabled for UAC protected files.  This overrides the
   // SILENT, NO_UI and NOERRORUI flags.
@@ -365,38 +365,77 @@ bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) {
     // ALLOWUNDO in favor of ADDUNDORECORD.
     if (FAILED(pfo->SetOperationFlags(
             FOF_NO_UI | FOFX_ADDUNDORECORD | FOF_NOERRORUI | FOF_SILENT |
-            FOFX_SHOWELEVATIONPROMPT | FOFX_RECYCLEONDELETE)))
+            FOFX_SHOWELEVATIONPROMPT | FOFX_RECYCLEONDELETE))) {
+      *error = "Failed to set operation flags";
       return false;
+    }
   } else {
     // For Windows 7 and Vista, RecycleOnDelete is the default behavior.
     if (FAILED(pfo->SetOperationFlags(FOF_NO_UI | FOF_ALLOWUNDO |
                                       FOF_NOERRORUI | FOF_SILENT |
-                                      FOFX_SHOWELEVATIONPROMPT)))
+                                      FOFX_SHOWELEVATIONPROMPT))) {
+      *error = "Failed to set operation flags";
       return false;
+    }
   }
 
   // Create an IShellItem from the supplied source path.
   Microsoft::WRL::ComPtr<IShellItem> delete_item;
   if (FAILED(SHCreateItemFromParsingName(
           path.value().c_str(), NULL,
-          IID_PPV_ARGS(delete_item.GetAddressOf()))))
+          IID_PPV_ARGS(delete_item.GetAddressOf())))) {
+    *error = "Failed to parse path";
     return false;
+  }
 
   Microsoft::WRL::ComPtr<IFileOperationProgressSink> delete_sink(
       new DeleteFileProgressSink);
-  if (!delete_sink)
+  if (!delete_sink) {
+    *error = "Failed to create 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())) &&
+  if (!SUCCEEDED(pfo->DeleteItem(delete_item.Get(), delete_sink.Get()))) {
-         SUCCEEDED(pfo->PerformOperations()) &&
+    *error = "Failed to enqueue DeleteItem command";
-         SUCCEEDED(pfo->GetAnyOperationsAborted(&pfAnyOperationsAborted)) &&
+    return false;
-         !pfAnyOperationsAborted;
+  }
+
+  if (!SUCCEEDED(pfo->PerformOperations())) {
+    *error = "Failed to perform delete operation";
+    return false;
+  }
+
+  if (!SUCCEEDED(pfo->GetAnyOperationsAborted(&pfAnyOperationsAborted))) {
+    *error = "Failed to check operation status";
+    return false;
+  }
+
+  if (pfAnyOperationsAborted) {
+    *error = "Operation was aborted";
+    return false;
+  }
+
+  return true;
 }
 
+bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) {
+  std::string error;  // ignored
+  base::win::ScopedCOMInitializer com_initializer;
+  return MoveItemToTrashWithError(path, delete_on_fail, &error);
+}
+
+namespace internal {
+
+bool PlatformTrashItem(const base::FilePath& full_path, std::string* error) {
+  return MoveItemToTrashWithError(full_path, false, error);
+}
+
+}  // namespace internal
+
 bool GetFolderPath(int key, base::FilePath* result) {
   wchar_t system_buffer[MAX_PATH];
 

spec-main/api-shell-spec.ts

@@ -112,4 +112,19 @@ describe('shell module', () => {
       expect(await fs.pathExists(tempPath)).to.be.false();
     });
   });
+
+  describe('shell.trashItem()', () => {
+    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');
+      await shell.trashItem(filename);
+      expect(fs.existsSync(filename)).to.be.false();
+    });
+
+    it('throws when called with a nonexistent path', async () => {
+      const filename = path.join(app.getPath('temp'), 'does-not-exist');
+      await expect(shell.trashItem(filename)).to.eventually.be.rejected();
+    });
+  });
 });