feat: macOS removal fallback when moveItemToTrash fails (electron#19700)

codebytere · web-flow · commit e1824c00a9cc · 2019-08-13T12:31:53.000-07:00
* feat: macOS removal fallback when moveItemToTrash fails

* platform_util shouldn't know about mate::Arguments

* pull full_path from args as well
diff --git a/docs/api/shell.md b/docs/api/shell.md
@@ -43,11 +43,12 @@ 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)`
+### `shell.moveItemToTrash(fullPath[, deleteOnFail])`
 
 * `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.
+Returns `Boolean` - Whether the item was successfully moved to the trash or otherwise deleted.
 
 Move the given file to trash and returns a boolean status for the operation.
 
diff --git a/shell/common/api/atom_api_shell.cc b/shell/common/api/atom_api_shell.cc
@@ -71,6 +71,16 @@ v8::Local<v8::Promise> OpenExternal(const GURL& url, mate::Arguments* args) {
   return handle;
 }
 
+bool MoveItemToTrash(mate::Arguments* args) {
+  base::FilePath full_path;
+  args->GetNext(&full_path);
+
+  bool delete_on_fail = false;
+  args->GetNext(&delete_on_fail);
+
+  return platform_util::MoveItemToTrash(full_path, delete_on_fail);
+}
+
 #if defined(OS_WIN)
 bool WriteShortcutLink(const base::FilePath& shortcut_path,
                        mate::Arguments* args) {
@@ -134,7 +144,7 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("showItemInFolder", &platform_util::ShowItemInFolder);
   dict.SetMethod("openItem", &platform_util::OpenItem);
   dict.SetMethod("openExternal", &OpenExternal);
-  dict.SetMethod("moveItemToTrash", &platform_util::MoveItemToTrash);
+  dict.SetMethod("moveItemToTrash", &MoveItemToTrash);
   dict.SetMethod("beep", &platform_util::Beep);
 #if defined(OS_WIN)
   dict.SetMethod("writeShortcutLink", &WriteShortcutLink);
diff --git a/shell/common/platform_util.h b/shell/common/platform_util.h
@@ -41,7 +41,7 @@ void OpenExternal(const GURL& url,
                   OpenExternalCallback callback);
 
 // Move a file to trash.
-bool MoveItemToTrash(const base::FilePath& full_path);
+bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail);
 
 void Beep();
 
diff --git a/shell/common/platform_util_linux.cc b/shell/common/platform_util_linux.cc
@@ -82,7 +82,7 @@ void OpenExternal(const GURL& url,
     std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open");
 }
 
-bool MoveItemToTrash(const base::FilePath& full_path) {
+bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
   std::unique_ptr<base::Environment> env(base::Environment::Create());
 
   // find the trash method
diff --git a/shell/common/platform_util_mac.mm b/shell/common/platform_util_mac.mm
@@ -115,16 +115,35 @@ void OpenExternal(const GURL& url,
                  });
 }
 
-bool MoveItemToTrash(const base::FilePath& full_path) {
+bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
   NSString* path_string = base::SysUTF8ToNSString(full_path.value());
-  BOOL status = [[NSFileManager defaultManager]
-        trashItemAtURL:[NSURL fileURLWithPath:path_string]
-      resultingItemURL:nil
-                 error:nil];
-  if (!path_string || !status)
+  if (!path_string) {
+    LOG(WARNING) << "Invalid file path " << full_path.value();
+    return false;
+  }
+
+  NSURL* url = [NSURL fileURLWithPath:path_string];
+  NSError* err = nil;
+  BOOL did_trash = [[NSFileManager defaultManager] trashItemAtURL:url
+                                                 resultingItemURL:nil
+                                                            error:&err];
+
+  if (delete_on_fail) {
+    // Some volumes may not support a Trash folder or it may be disabled
+    // so these methods will report failure by returning NO or nil and
+    // an NSError with NSFeatureUnsupportedError.
+    // Handle this by deleting the item as a fallback.
+    if (!did_trash && [err code] == NSFeatureUnsupportedError) {
+      did_trash = [[NSFileManager defaultManager] removeItemAtURL:url
+                                                            error:nil];
+    }
+  }
+
+  if (!did_trash)
     LOG(WARNING) << "NSWorkspace failed to move file " << full_path.value()
                  << " to trash";
-  return status;
+
+  return did_trash;
 }
 
 void Beep() {
diff --git a/shell/common/platform_util_win.cc b/shell/common/platform_util_win.cc
@@ -335,7 +335,7 @@ void OpenExternal(const GURL& url,
       std::move(callback));
 }
 
-bool MoveItemToTrash(const base::FilePath& path) {
+bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) {
   base::win::ScopedCOMInitializer com_initializer;
   if (!com_initializer.Succeeded())
     return false;

Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ void OpenExternal(const GURL& url,`
`82`	`82`	`std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open");`
`83`	`83`	`}`
`84`	`84`
`85`		`-bool MoveItemToTrash(const base::FilePath& full_path) {`
	`85`	`+bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {`
`86`	`86`	`std::unique_ptr<base::Environment> env(base::Environment::Create());`
`87`	`87`
`88`	`88`	`// find the trash method`
Original file line number	Diff line number	Diff line change
`@@ -335,7 +335,7 @@ void OpenExternal(const GURL& url,`
`335`	`335`	`std::move(callback));`
`336`	`336`	`}`
`337`	`337`
`338`		`-bool MoveItemToTrash(const base::FilePath& path) {`
	`338`	`+bool MoveItemToTrash(const base::FilePath& path, bool delete_on_fail) {`
`339`	`339`	`base::win::ScopedCOMInitializer com_initializer;`
`340`	`340`	`if (!com_initializer.Succeeded())`
`341`	`341`	`return false;`