feat: add workingDirectory option to shell.openExternal() (#15065)

Allows passing `workingDirectory` to the underlying `ShellExecuteW` API on Windows.

the motivation is that by default `ShellExecute` would use the current working directory, which would get locked on Windows and can prevent autoUpdater from working correctly. We need to be able specify a different `workingDirectory` to prevent this situation.

atom/browser/atom_browser_client.cc

@@ -611,7 +611,7 @@ void OnOpenExternal(const GURL& escaped_url, bool allowed) {
 #else
         escaped_url,
 #endif
-        true);
+        platform_util::OpenExternalOptions());
 }
 
 void HandleExternalProtocolInUI(

atom/common/api/atom_api_shell.cc

@@ -60,11 +60,12 @@ bool OpenExternal(
     const GURL& url,
 #endif
     mate::Arguments* args) {
-  bool activate = true;
+  platform_util::OpenExternalOptions options;
   if (args->Length() >= 2) {
-    mate::Dictionary options;
-    if (args->GetNext(&options)) {
-      options.Get("activate", &activate);
+    mate::Dictionary obj;
+    if (args->GetNext(&obj)) {
+      obj.Get("activate", &options.activate);
+      obj.Get("workingDirectory", &options.working_dir);
     }
   }
 
@@ -72,13 +73,13 @@ bool OpenExternal(
     base::Callback<void(v8::Local<v8::Value>)> callback;
     if (args->GetNext(&callback)) {
       platform_util::OpenExternal(
-          url, activate,
+          url, options,
           base::Bind(&OnOpenExternalFinished, args->isolate(), callback));
       return true;
     }
   }
 
-  return platform_util::OpenExternal(url, activate);
+  return platform_util::OpenExternal(url, options);
 }
 
 #if defined(OS_WIN)

atom/common/platform_util.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/callback_forward.h"
+#include "base/files/file_path.h"
 #include "build/build_config.h"
 
 #if defined(OS_WIN)
@@ -16,10 +17,6 @@
 
 class GURL;
 
-namespace base {
-class FilePath;
-}
-
 namespace platform_util {
 
 typedef base::Callback<void(const std::string&)> OpenExternalCallback;
@@ -32,6 +29,11 @@ bool ShowItemInFolder(const base::FilePath& full_path);
 // Must be called from the UI thread.
 bool OpenItem(const base::FilePath& full_path);
 
+struct OpenExternalOptions {
+  bool activate = true;
+  base::FilePath working_dir;
+};
+
 // Open the given external protocol URL in the desktop's default manner.
 // (For example, mailto: URLs in the default mail user agent.)
 bool OpenExternal(
@@ -40,7 +42,7 @@ bool OpenExternal(
 #else
     const GURL& url,
 #endif
-    bool activate);
+    const OpenExternalOptions& options);
 
 // The asynchronous version of OpenExternal.
 void OpenExternal(
@@ -49,7 +51,7 @@ void OpenExternal(
 #else
     const GURL& url,
 #endif
-    bool activate,
+    const OpenExternalOptions& options,
     const OpenExternalCallback& callback);
 
 // Move a file to trash.

atom/common/platform_util_linux.cc

@@ -80,7 +80,7 @@ bool OpenItem(const base::FilePath& full_path) {
   return XDGOpen(full_path.value(), false);
 }
 
-bool OpenExternal(const GURL& url, bool activate) {
+bool OpenExternal(const GURL& url, const OpenExternalOptions& options) {
   // 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"))
@@ -90,10 +90,10 @@ bool OpenExternal(const GURL& url, bool activate) {
 }
 
 void OpenExternal(const GURL& url,
-                  bool activate,
+                  const OpenExternalOptions& options,
                   const OpenExternalCallback& callback) {
   // TODO(gabriel): Implement async open if callback is specified
-  callback.Run(OpenExternal(url, activate) ? "" : "Failed to open");
+  callback.Run(OpenExternal(url, options) ? "" : "Failed to open");
 }
 
 bool MoveItemToTrash(const base::FilePath& full_path) {

atom/common/platform_util_mac.mm

@@ -139,16 +139,16 @@ bool OpenItem(const base::FilePath& full_path) {
                                launchIdentifiers:NULL];
 }
 
-bool OpenExternal(const GURL& url, bool activate) {
+bool OpenExternal(const GURL& url, const OpenExternalOptions& options) {
   DCHECK([NSThread isMainThread]);
   NSURL* ns_url = net::NSURLWithGURL(url);
   if (ns_url)
-    return OpenURL(ns_url, activate).empty();
+    return OpenURL(ns_url, options.activate).empty();
   return false;
 }
 
 void OpenExternal(const GURL& url,
-                  bool activate,
+                  const OpenExternalOptions& options,
                   const OpenExternalCallback& callback) {
   NSURL* ns_url = net::NSURLWithGURL(url);
   if (!ns_url) {
@@ -157,13 +157,13 @@ void OpenExternal(const GURL& url,
   }
 
   __block OpenExternalCallback c = callback;
-  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
-                 ^{
-                   __block std::string error = OpenURL(ns_url, activate);
-                   dispatch_async(dispatch_get_main_queue(), ^{
-                     c.Run(error);
-                   });
-                 });
+  dispatch_async(
+      dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        __block std::string error = OpenURL(ns_url, options.activate);
+        dispatch_async(dispatch_get_main_queue(), ^{
+          c.Run(error);
+        });
+      });
 }
 
 bool MoveItemToTrash(const base::FilePath& full_path) {

atom/common/platform_util_win.cc

@@ -294,15 +294,18 @@ bool OpenItem(const base::FilePath& full_path) {
     return ui::win::OpenFileViaShell(full_path);
 }
 
-bool OpenExternal(const base::string16& url, bool activate) {
+bool OpenExternal(const base::string16& url,
+                  const OpenExternalOptions& options) {
   // Quote the input scheme to be sure that the command does not have
   // parameters unexpected by the external program. This url should already
   // have been escaped.
   base::string16 escaped_url = L"\"" + url + L"\"";
+  auto working_dir = options.working_dir.value();
 
-  if (reinterpret_cast<ULONG_PTR>(ShellExecuteW(
-          NULL, L"open", escaped_url.c_str(), NULL, NULL, SW_SHOWNORMAL)) <=
-      32) {
+  if (reinterpret_cast<ULONG_PTR>(
+          ShellExecuteW(nullptr, L"open", escaped_url.c_str(), nullptr,
+                        working_dir.empty() ? nullptr : working_dir.c_str(),
+                        SW_SHOWNORMAL)) <= 32) {
     // We fail to execute the call. We could display a message to the user.
     // TODO(nsylvain): we should also add a dialog to warn on errors. See
     // bug 1136923.
@@ -312,10 +315,10 @@ bool OpenExternal(const base::string16& url, bool activate) {
 }
 
 void OpenExternal(const base::string16& url,
-                  bool activate,
+                  const OpenExternalOptions& options,
                   const OpenExternalCallback& callback) {
   // TODO(gabriel): Implement async open if callback is specified
-  callback.Run(OpenExternal(url, activate) ? "" : "Failed to open");
+  callback.Run(OpenExternal(url, options) ? "" : "Failed to open");
 }
 
 bool MoveItemToTrash(const base::FilePath& path) {

docs/api/shell.md

@@ -37,9 +37,10 @@ Open the given file in the desktop's default manner.
 ### `shell.openExternal(url[, options, callback])`
 
 * `url` String - Max 2081 characters on windows, or the function returns false.
-* `options` Object (optional) _macOS_
-  * `activate` Boolean - `true` to bring the opened application to the
-    foreground. The default is `true`.
+* `options` Object (optional)
+  * `activate` Boolean (optional) - `true` to bring the opened application to the
+    foreground. The default is `true`. _macOS_
+  * `workingDirectory` String (optional) - The working directory. _Windows_
 * `callback` Function (optional) _macOS_ - If specified will perform the open asynchronously.
   * `error` Error