feat: convert shell.openItem to async shell.openPath (#20682)

docs/api/shell.md

@@ -24,11 +24,14 @@ The `shell` module has the following methods:
 
 Show the given file in a file manager. If possible, select the file.
 
-### `shell.openItem(fullPath)`
+### `shell.openPath(path)`
 
-* `fullPath` String
+* `path` String
 
-Returns `Boolean` - Whether the item was successfully opened.
+Returns `Promise<Object>` - Resolve with an object containing the following:
+
+* `success` Boolean - whether or not the path was successfully opened in the desktop's default manner.
+* `errorMessage` String (optional) - The error message corresponding to the failure if a failure occurred, otherwise empty string.
 
 Open the given file in the desktop's default manner.
 

shell/browser/ui/inspectable_web_contents_impl.cc

@@ -559,7 +559,9 @@ void InspectableWebContentsImpl::ShowItemInFolder(
     return;
 
   base::FilePath path = base::FilePath::FromUTF8Unsafe(file_system_path);
-  platform_util::OpenItem(path);
+
+  // Pass empty callback here; we can ignore errors
+  platform_util::OpenPath(path, platform_util::OpenCallback());
 }
 
 void InspectableWebContentsImpl::SaveToFile(const std::string& url,

shell/common/api/atom_api_shell.cc

@@ -44,8 +44,8 @@ struct Converter<base::win::ShortcutOperation> {
 
 namespace {
 
-void OnOpenExternalFinished(gin_helper::Promise<void> promise,
+void OnOpenFinished(gin_helper::Promise<void> promise,
-                            const std::string& error) {
+                    const std::string& error) {
   if (error.empty())
     promise.Resolve();
   else
@@ -66,8 +66,21 @@ v8::Local<v8::Promise> OpenExternal(const GURL& url, gin::Arguments* args) {
   }
 
   platform_util::OpenExternal(
-      url, options,
+      url, options, base::BindOnce(&OnOpenFinished, std::move(promise)));
-      base::BindOnce(&OnOpenExternalFinished, std::move(promise)));
+  return handle;
+}
+
+v8::Local<v8::Promise> OpenPath(v8::Isolate* isolate,
+                                const base::FilePath& full_path) {
+  gin_helper::Promise<const std::string&> promise(isolate);
+  v8::Local<v8::Promise> handle = promise.GetHandle();
+
+  platform_util::OpenPath(
+      full_path,
+      base::BindOnce(
+          [](gin_helper::Promise<const std::string&> promise,
+             const std::string& err_msg) { promise.Resolve(err_msg); },
+          std::move(promise)));
   return handle;
 }
 
@@ -142,7 +155,7 @@ void Initialize(v8::Local<v8::Object> exports,
                 void* priv) {
   gin_helper::Dictionary dict(context->GetIsolate(), exports);
   dict.SetMethod("showItemInFolder", &platform_util::ShowItemInFolder);
-  dict.SetMethod("openItem", &platform_util::OpenItem);
+  dict.SetMethod("openPath", &OpenPath);
   dict.SetMethod("openExternal", &OpenExternal);
   dict.SetMethod("moveItemToTrash", &MoveItemToTrash);
   dict.SetMethod("beep", &platform_util::Beep);

shell/common/platform_util.h

@@ -19,7 +19,7 @@ class GURL;
 
 namespace platform_util {
 
-typedef base::OnceCallback<void(const std::string&)> OpenExternalCallback;
+typedef base::OnceCallback<void(const std::string&)> OpenCallback;
 
 // Show the given file in a file manager. If possible, select the file.
 // Must be called from the UI thread.
@@ -27,7 +27,7 @@ void ShowItemInFolder(const base::FilePath& full_path);
 
 // Open the given file in the desktop's default manner.
 // Must be called from the UI thread.
-bool OpenItem(const base::FilePath& full_path);
+void OpenPath(const base::FilePath& full_path, OpenCallback callback);
 
 struct OpenExternalOptions {
   bool activate = true;
@@ -38,7 +38,7 @@ struct OpenExternalOptions {
 // (For example, mailto: URLs in the default mail user agent.)
 void OpenExternal(const GURL& url,
                   const OpenExternalOptions& options,
-                  OpenExternalCallback callback);
+                  OpenCallback callback);
 
 // Move a file to trash.
 bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail);

shell/common/platform_util_linux.cc

@@ -19,7 +19,25 @@
 
 namespace {
 
-bool XDGUtil(const std::vector<std::string>& argv, const bool wait_for_exit) {
+// Descriptions pulled from https://linux.die.net/man/1/xdg-open
+std::string GetErrorDescription(int error_code) {
+  switch (error_code) {
+    case 1:
+      return "Error in command line syntax";
+    case 2:
+      return "The item does not exist";
+    case 3:
+      return "A required tool could not be found";
+    case 4:
+      return "The action failed";
+    default:
+      return "";
+  }
+}
+
+bool XDGUtil(const std::vector<std::string>& argv,
+             const bool wait_for_exit,
+             platform_util::OpenCallback callback) {
   base::LaunchOptions options;
   options.allow_new_privs = true;
   // xdg-open can fall back on mailcap which eventually might plumb through
@@ -34,52 +52,56 @@ bool XDGUtil(const std::vector<std::string>& argv, const bool wait_for_exit) {
 
   if (wait_for_exit) {
     int exit_code = -1;
-    if (!process.WaitForExit(&exit_code))
+    bool success = process.WaitForExit(&exit_code);
-      return false;
+    if (!callback.is_null())
-    return (exit_code == 0);
+      std::move(callback).Run(GetErrorDescription(exit_code));
+    return success ? (exit_code == 0) : false;
   }
 
   base::EnsureProcessGetsReaped(std::move(process));
   return true;
 }
 
-bool XDGOpen(const std::string& path, const bool wait_for_exit) {
+bool XDGOpen(const std::string& path,
-  return XDGUtil({"xdg-open", path}, wait_for_exit);
+             const bool wait_for_exit,
+             platform_util::OpenCallback callback) {
+  return XDGUtil({"xdg-open", path}, wait_for_exit, std::move(callback));
 }
 
 bool XDGEmail(const std::string& email, const bool wait_for_exit) {
-  return XDGUtil({"xdg-email", email}, wait_for_exit);
+  return XDGUtil({"xdg-email", email}, wait_for_exit,
+                 platform_util::OpenCallback());
 }
 
 }  // namespace
 
 namespace platform_util {
 
-// TODO(estade): It would be nice to be able to select the file in the file
-// manager, but that probably requires extending xdg-open. For now just
-// show the folder.
 void ShowItemInFolder(const base::FilePath& full_path) {
   base::FilePath dir = full_path.DirName();
   if (!base::DirectoryExists(dir))
     return;
 
-  XDGOpen(dir.value(), false);
+  XDGOpen(dir.value(), false, platform_util::OpenCallback());
 }
 
-bool OpenItem(const base::FilePath& full_path) {
+void OpenPath(const base::FilePath& full_path, OpenCallback callback) {
-  return XDGOpen(full_path.value(), false);
+  // This is async, so we don't care about the return value.
+  XDGOpen(full_path.value(), true, std::move(callback));
 }
 
 void OpenExternal(const GURL& url,
                   const OpenExternalOptions& options,
-                  OpenExternalCallback callback) {
+                  OpenCallback callback) {
   // Don't wait for exit, since we don't want to wait for the browser/email
   // client window to close before returning
-  if (url.SchemeIs("mailto"))
+  if (url.SchemeIs("mailto")) {
-    std::move(callback).Run(XDGEmail(url.spec(), false) ? ""
+    bool success = XDGEmail(url.spec(), false);
-                                                        : "Failed to open");
+    std::move(callback).Run(success ? "" : "Failed to open path");
-  else
+  } else {
-    std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open");
+    bool success = XDGOpen(url.spec(), false, platform_util::OpenCallback());
+    std::move(callback).Run(success ? "" : "Failed to open path");
+  }
 }
 
 bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
@@ -111,7 +133,7 @@ bool MoveItemToTrash(const base::FilePath& full_path, bool delete_on_fail) {
     argv = {"gio", "trash", filename};
   }
 
-  return XDGUtil(argv, true);
+  return XDGUtil(argv, true, platform_util::OpenCallback());
 }
 
 void Beep() {

shell/common/platform_util_mac.mm

@@ -58,6 +58,23 @@ NSString* GetLoginHelperBundleIdentifier() {
       stringByAppendingString:@".loginhelper"];
 }
 
+std::string OpenPathOnThread(const base::FilePath& full_path) {
+  NSString* path_string = base::SysUTF8ToNSString(full_path.value());
+  NSURL* url = [NSURL fileURLWithPath:path_string];
+  if (!url)
+    return "Invalid path";
+
+  const NSWorkspaceLaunchOptions launch_options =
+      NSWorkspaceLaunchAsync | NSWorkspaceLaunchWithErrorPresentation;
+  BOOL success = [[NSWorkspace sharedWorkspace] openURLs:@[ url ]
+                                 withAppBundleIdentifier:nil
+                                                 options:launch_options
+                          additionalEventParamDescriptor:nil
+                                       launchIdentifiers:NULL];
+
+  return success ? "" : "Failed to open path";
+}
+
 }  // namespace
 
 namespace platform_util {
@@ -75,28 +92,13 @@ void ShowItemInFolder(const base::FilePath& path) {
   }
 }
 
-bool OpenItem(const base::FilePath& full_path) {
+void OpenPath(const base::FilePath& full_path, OpenCallback callback) {
-  DCHECK([NSThread isMainThread]);
+  std::move(callback).Run(OpenPathOnThread(full_path));
-  NSString* path_string = base::SysUTF8ToNSString(full_path.value());
-  if (!path_string)
-    return false;
-
-  NSURL* url = [NSURL fileURLWithPath:path_string];
-  if (!url)
-    return false;
-
-  const NSWorkspaceLaunchOptions launch_options =
-      NSWorkspaceLaunchAsync | NSWorkspaceLaunchWithErrorPresentation;
-  return [[NSWorkspace sharedWorkspace] openURLs:@[ url ]
-                         withAppBundleIdentifier:nil
-                                         options:launch_options
-                  additionalEventParamDescriptor:nil
-                               launchIdentifiers:NULL];
 }
 
 void OpenExternal(const GURL& url,
                   const OpenExternalOptions& options,
-                  OpenExternalCallback callback) {
+                  OpenCallback callback) {
   DCHECK([NSThread isMainThread]);
   NSURL* ns_url = net::NSURLWithGURL(url);
   if (!ns_url) {
@@ -105,7 +107,7 @@ void OpenExternal(const GURL& url,
   }
 
   bool activate = options.activate;
-  __block OpenExternalCallback c = std::move(callback);
+  __block OpenCallback c = std::move(callback);
   dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
                  ^{
                    __block std::string error = OpenURL(ns_url, activate);

shell/common/platform_util_win.cc

@@ -30,6 +30,7 @@
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/windows_version.h"
 #include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "ui/base/win/shell.h"
 #include "url/gurl.h"
 
@@ -303,6 +304,19 @@ void ShowItemInFolderOnWorkerThread(const base::FilePath& full_path) {
   }
 }
 
+std::string OpenPathOnThread(const base::FilePath& full_path) {
+  // May result in an interactive dialog.
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
+  bool success;
+  if (base::DirectoryExists(full_path))
+    success = ui::win::OpenFolderViaShell(full_path);
+  else
+    success = ui::win::OpenFileViaShell(full_path);
+
+  return success ? "" : "Failed to open path";
+}
+
 }  // namespace
 
 namespace platform_util {
@@ -314,16 +328,20 @@ void ShowItemInFolder(const base::FilePath& full_path) {
                  base::BindOnce(&ShowItemInFolderOnWorkerThread, full_path));
 }
 
-bool OpenItem(const base::FilePath& full_path) {
+void OpenPath(const base::FilePath& full_path, OpenCallback callback) {
-  if (base::DirectoryExists(full_path))
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-    return ui::win::OpenFolderViaShell(full_path);
+
-  else
+  base::PostTaskAndReplyWithResult(
-    return ui::win::OpenFileViaShell(full_path);
+      base::CreateCOMSTATaskRunner({base::ThreadPool(), base::MayBlock(),
+                                    base::TaskPriority::USER_BLOCKING})
+          .get(),
+      FROM_HERE, base::BindOnce(&OpenPathOnThread, full_path),
+      std::move(callback));
 }
 
 void OpenExternal(const GURL& url,
                   const OpenExternalOptions& options,
-                  OpenExternalCallback callback) {
+                  OpenCallback callback) {
   base::PostTaskAndReplyWithResult(
       base::CreateCOMSTATaskRunner({base::ThreadPool(), base::MayBlock(),
                                     base::TaskPriority::USER_BLOCKING})

spec/ts-smoke/electron/main.ts

@@ -1056,9 +1056,12 @@ app.on('ready', () => {
 // https://github.com/atom/electron/blob/master/docs/api/shell.md
 
 shell.showItemInFolder('/home/user/Desktop/test.txt')
-shell.openItem('/home/user/Desktop/test.txt')
 shell.moveItemToTrash('/home/user/Desktop/test.txt')
 
+shell.openPath('/home/user/Desktop/test.txt').then(err => {
+  if (err) console.log(err)
+})
+
 shell.openExternal('https://github.com', {
   activate: false
 }).then(() => {})