From b266533dfc8f4ae0f2c00771ef1b2f03ee99cda3 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Thu, 13 Oct 2016 13:28:11 -0700 Subject: [PATCH 01/21] Shell openExternal can take optional callback (macOS) --- atom/common/api/atom_api_shell.cc | 13 +++++++++- atom/common/platform_util.h | 12 ++++++++++ atom/common/platform_util_linux.cc | 13 +++++++++- atom/common/platform_util_mac.mm | 38 +++++++++++++++++++++++++----- atom/common/platform_util_win.cc | 13 +++++++++- docs/api/shell.md | 6 ++++- 6 files changed, 85 insertions(+), 10 deletions(-) diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index 03e8ed32e2..05214531e1 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -4,6 +4,7 @@ #include +#include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/gurl_converter.h" #include "atom/common/native_mate_converters/string16_converter.h" @@ -49,12 +50,22 @@ bool OpenExternal( #endif mate::Arguments* args) { bool activate = true; - if (args->Length() == 2) { + if (args->Length() >= 2) { mate::Dictionary options; if (args->GetNext(&options)) { options.Get("activate", &activate); } } + + if (args->Length() >= 3) { + v8::Local peek = args->PeekNext(); + platform_util::OpenExternalCallback callback; + if (mate::Converter::FromV8(args->isolate(), peek, &callback)) { + return platform_util::OpenExternal(url, activate, callback); + } + return false; + } + return platform_util::OpenExternal(url, activate); } diff --git a/atom/common/platform_util.h b/atom/common/platform_util.h index 520faa48f4..8f2c5b6e52 100644 --- a/atom/common/platform_util.h +++ b/atom/common/platform_util.h @@ -6,6 +6,7 @@ #define ATOM_COMMON_PLATFORM_UTIL_H_ #include "build/build_config.h" +#include "base/callback_forward.h" #if defined(OS_WIN) #include "base/strings/string16.h" @@ -27,6 +28,8 @@ bool ShowItemInFolder(const base::FilePath& full_path); // Must be called from the UI thread. bool OpenItem(const base::FilePath& full_path); +typedef base::Callback OpenExternalCallback; + // Open the given external protocol URL in the desktop's default manner. // (For example, mailto: URLs in the default mail user agent.) bool OpenExternal( @@ -37,6 +40,15 @@ bool OpenExternal( #endif bool activate); +bool OpenExternal( +#if defined(OS_WIN) + const base::string16& url, +#else + const GURL& url, +#endif + bool activate, + const OpenExternalCallback& callback); + // Move a file to trash. bool MoveItemToTrash(const base::FilePath& full_path); diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 82265901f1..8cfa516038 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -79,7 +79,7 @@ bool OpenItem(const base::FilePath& full_path) { return XDGOpen(full_path.value(), true); } -bool OpenExternal(const GURL& url, bool activate) { +bool openExternal(const GURL& url, bool activate) { // 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")) @@ -88,6 +88,17 @@ bool OpenExternal(const GURL& url, bool activate) { return XDGOpen(url.spec(), false); } +bool OpenExternal(const GURL& url, bool activate) { + return openExternal(url, activate); +} + +bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { + // TODO: Implement async open if callback is specified + bool opened = openExternal(url, activate); + callback(opened); + return opened; +} + bool MoveItemToTrash(const base::FilePath& full_path) { std::string trash; if (getenv(ELECTRON_TRASH) != NULL) { diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index 6d253c99c8..3f6c3470f7 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -7,6 +7,7 @@ #include #import +#include "base/cancelable_callback.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/logging.h" @@ -128,7 +129,17 @@ bool OpenItem(const base::FilePath& full_path) { return status == noErr; } -bool OpenExternal(const GURL& url, bool activate) { +bool openURLInWorkspace(NSURL* ns_url, NSUInteger launchOptions) { + return [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] + withAppBundleIdentifier: nil + options: launchOptions + additionalEventParamDescriptor: NULL + launchIdentifiers: NULL]; +} + +typedef bool(^OpenExternalBlock)(NSURL* ns_url, NSUInteger launchOptions); + +bool openExternal(const GURL& url, bool activate, OpenExternalBlock open) { DCHECK([NSThread isMainThread]); NSURL* ns_url = net::NSURLWithGURL(url); if (!ns_url) { @@ -149,11 +160,26 @@ bool OpenExternal(const GURL& url, bool activate) { if (!activate) launchOptions |= NSWorkspaceLaunchWithoutActivation; - return [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] - withAppBundleIdentifier: nil - options: launchOptions - additionalEventParamDescriptor: NULL - launchIdentifiers: NULL]; + return open(ns_url, launchOptions); +} + +bool OpenExternal(const GURL& url, bool activate) { + return openExternal(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { + return openURLInWorkspace(ns_url, launchOptions); + }); +} + +bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { + __block OpenExternalCallback callback = c; + return openExternal(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + bool opened = openURLInWorkspace(ns_url, launchOptions); + dispatch_async(dispatch_get_main_queue(), ^{ + callback.Run(opened); + }); + }); + return YES; + }); } bool MoveItemToTrash(const base::FilePath& full_path) { diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index 2c98115989..5483c783f1 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -299,7 +299,7 @@ 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, bool activate) { // 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. @@ -316,6 +316,17 @@ bool OpenExternal(const base::string16& url, bool activate) { return true; } +bool OpenExternal(const base::string16& url, bool activate) { + return openExternal(url, activate); +} + +bool OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { + // TODO: Implement async open if callback is specified + bool opened = openExternal(url, activate) + callback(opened); + return opened; +} + bool MoveItemToTrash(const base::FilePath& path) { base::win::ScopedCOMInitializer com_initializer; if (!com_initializer.succeeded()) diff --git a/docs/api/shell.md b/docs/api/shell.md index 89ae954ad6..d96df89153 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -34,18 +34,22 @@ Returns `Boolean` - Whether the item was successfully opened. Open the given file in the desktop's default manner. -### `shell.openExternal(url[, options])` +### `shell.openExternal(url[, options, callback])` * `url` String * `options` Object (optional) _macOS_ * `activate` Boolean - `true` to bring the opened application to the foreground. The default is `true`. +* `callback` Function (optional) _macOS_ Returns `Boolean` - Whether an application was available to open the URL. Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). +If a `callback` is passed, the API call will be asynchronous and the result +will be passed via `callback(opened)`. + ### `shell.moveItemToTrash(fullPath)` * `fullPath` String From 3eb5f8d52147e7a589dabbbf151743995dca7b09 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Thu, 13 Oct 2016 19:10:12 -0700 Subject: [PATCH 02/21] Fix typo --- atom/common/platform_util_win.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index 5483c783f1..7eb3c470f7 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -322,7 +322,7 @@ bool OpenExternal(const base::string16& url, bool activate) { bool OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { // TODO: Implement async open if callback is specified - bool opened = openExternal(url, activate) + bool opened = openExternal(url, activate); callback(opened); return opened; } From 128feb17cb1751adbd452bb73b46cb6488d8a783 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Thu, 13 Oct 2016 19:16:47 -0700 Subject: [PATCH 03/21] Fix method call --- atom/common/platform_util_linux.cc | 2 +- atom/common/platform_util_win.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 8cfa516038..bf3c3b272b 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -95,7 +95,7 @@ bool OpenExternal(const GURL& url, bool activate) { bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { // TODO: Implement async open if callback is specified bool opened = openExternal(url, activate); - callback(opened); + callback.Run(opened); return opened; } diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index 7eb3c470f7..dd0121b8f7 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -323,7 +323,7 @@ bool OpenExternal(const base::string16& url, bool activate) { bool OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { // TODO: Implement async open if callback is specified bool opened = openExternal(url, activate); - callback(opened); + callback.Run(opened); return opened; } From 9ca684d87ffedc3c77c31e567d3d5a194e46d106 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Fri, 14 Oct 2016 12:34:02 -0700 Subject: [PATCH 04/21] Remove redundant method --- atom/common/platform_util_linux.cc | 8 ++------ atom/common/platform_util_win.cc | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index bf3c3b272b..3aabe375ad 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -79,7 +79,7 @@ bool OpenItem(const base::FilePath& full_path) { return XDGOpen(full_path.value(), true); } -bool openExternal(const GURL& url, bool activate) { +bool OpenExternal(const GURL& url, bool activate) { // 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")) @@ -88,13 +88,9 @@ bool openExternal(const GURL& url, bool activate) { return XDGOpen(url.spec(), false); } -bool OpenExternal(const GURL& url, bool activate) { - return openExternal(url, activate); -} - bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { // TODO: Implement async open if callback is specified - bool opened = openExternal(url, activate); + bool opened = OpenExternal(url, activate); callback.Run(opened); return opened; } diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index dd0121b8f7..f859193de2 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -299,7 +299,7 @@ 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, bool activate) { // 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. @@ -316,13 +316,9 @@ bool openExternal(const base::string16& url, bool activate) { return true; } -bool OpenExternal(const base::string16& url, bool activate) { - return openExternal(url, activate); -} - bool OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { // TODO: Implement async open if callback is specified - bool opened = openExternal(url, activate); + bool opened = OpenExternal(url, activate); callback.Run(opened); return opened; } From c78567aba6fad051452c4477cdea4f2c4a030437 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Fri, 14 Oct 2016 12:34:22 -0700 Subject: [PATCH 05/21] Update shell openExternal docs (from review) --- docs/api/shell.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/api/shell.md b/docs/api/shell.md index d96df89153..bfe2ae03cd 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -40,9 +40,11 @@ Open the given file in the desktop's default manner. * `options` Object (optional) _macOS_ * `activate` Boolean - `true` to bring the opened application to the foreground. The default is `true`. -* `callback` Function (optional) _macOS_ +* `callback` Function (optional) - If specified will perform the open asynchronously. _macOS_ + * `opened` Boolean Returns `Boolean` - Whether an application was available to open the URL. +If callback is specified, it will return whether it was able to initiate the open call. Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). From cfd2a029add3eabf5d59a0b7c83d931e55169421 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Fri, 14 Oct 2016 12:38:55 -0700 Subject: [PATCH 06/21] Fix linting --- atom/common/api/atom_api_shell.cc | 3 ++- atom/common/platform_util.h | 2 +- atom/common/platform_util_linux.cc | 5 +++-- atom/common/platform_util_win.cc | 5 +++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index 05214531e1..894c85eeea 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -60,7 +60,8 @@ bool OpenExternal( if (args->Length() >= 3) { v8::Local peek = args->PeekNext(); platform_util::OpenExternalCallback callback; - if (mate::Converter::FromV8(args->isolate(), peek, &callback)) { + if (mate::Converter::FromV8( + args->isolate(), peek, &callback)) { return platform_util::OpenExternal(url, activate, callback); } return false; diff --git a/atom/common/platform_util.h b/atom/common/platform_util.h index 8f2c5b6e52..d7bb920648 100644 --- a/atom/common/platform_util.h +++ b/atom/common/platform_util.h @@ -5,8 +5,8 @@ #ifndef ATOM_COMMON_PLATFORM_UTIL_H_ #define ATOM_COMMON_PLATFORM_UTIL_H_ -#include "build/build_config.h" #include "base/callback_forward.h" +#include "build/build_config.h" #if defined(OS_WIN) #include "base/strings/string16.h" diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 3aabe375ad..fd83217959 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -88,8 +88,9 @@ bool OpenExternal(const GURL& url, bool activate) { return XDGOpen(url.spec(), false); } -bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { - // TODO: Implement async open if callback is specified +bool OpenExternal(const GURL& url, bool activate, + const OpenExternalCallback& callback) { + // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); callback.Run(opened); return opened; diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index f859193de2..69778fefe8 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -316,8 +316,9 @@ bool OpenExternal(const base::string16& url, bool activate) { return true; } -bool OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { - // TODO: Implement async open if callback is specified +bool OpenExternal(const base::string16& url, bool activate, + const OpenExternalCallback& callback) { + // // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); callback.Run(opened); return opened; From 21df08d5368344e13e8f939482209dacac3d40e8 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Fri, 14 Oct 2016 12:41:12 -0700 Subject: [PATCH 07/21] Remove redundant comment --- docs/api/shell.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/api/shell.md b/docs/api/shell.md index bfe2ae03cd..75eccb518e 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -49,9 +49,6 @@ If callback is specified, it will return whether it was able to initiate the ope Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). -If a `callback` is passed, the API call will be asynchronous and the result -will be passed via `callback(opened)`. - ### `shell.moveItemToTrash(fullPath)` * `fullPath` String From 6524a33ffc883ee325c663ac2cccb28fa7505832 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Thu, 20 Oct 2016 13:01:14 -0700 Subject: [PATCH 08/21] Include callback header --- atom/common/platform_util_linux.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index fd83217959..2a8fb9e7a7 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -6,6 +6,7 @@ #include +#include "base/callback_forward.h" #include "base/files/file_util.h" #include "base/process/kill.h" #include "base/process/launch.h" From d186fd78e3fbfc6c64df2d5a161b6be928aa46d8 Mon Sep 17 00:00:00 2001 From: "Gabriel Handford (linux)" Date: Thu, 20 Oct 2016 17:08:05 -0700 Subject: [PATCH 09/21] Fix header --- atom/common/platform_util_linux.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 2a8fb9e7a7..1b998ecaef 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -6,7 +6,7 @@ #include -#include "base/callback_forward.h" +#include "base/cancelable_callback.h" #include "base/files/file_util.h" #include "base/process/kill.h" #include "base/process/launch.h" From 08a9af3a85f205a26656630205bcccab75ee61e4 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Mon, 24 Oct 2016 10:09:48 -0700 Subject: [PATCH 10/21] Linting --- atom/common/platform_util_linux.cc | 2 +- atom/common/platform_util_win.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 1b998ecaef..ee08d2832b 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -90,7 +90,7 @@ bool OpenExternal(const GURL& url, bool activate) { } bool OpenExternal(const GURL& url, bool activate, - const OpenExternalCallback& callback) { + const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); callback.Run(opened); diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index 69778fefe8..d2f2b1ca11 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -317,8 +317,8 @@ bool OpenExternal(const base::string16& url, bool activate) { } bool OpenExternal(const base::string16& url, bool activate, - const OpenExternalCallback& callback) { - // // TODO(gabriel): Implement async open if callback is specified + const OpenExternalCallback& callback) { + // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); callback.Run(opened); return opened; From 5e8059e0fae4138a823773a8a032c27cae772102 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Tue, 25 Oct 2016 15:19:34 -0700 Subject: [PATCH 11/21] Fix method names, move to anon namespace --- atom/common/platform_util_mac.mm | 80 +++++++++++++++++--------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index 3f6c3470f7..b88bd539ce 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -17,6 +17,44 @@ #include "net/base/mac/url_conversions.h" #include "url/gurl.h" +namespace { + +bool OpenURLInWorkspace(NSURL* ns_url, NSUInteger launchOptions) { + return [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] + withAppBundleIdentifier: nil + options: launchOptions + additionalEventParamDescriptor: NULL + launchIdentifiers: NULL]; +} + +typedef bool(^OpenExternalBlock)(NSURL* ns_url, NSUInteger launchOptions); + +bool OpenExternalWithBlock(const GURL& url, bool activate, OpenExternalBlock open) { + DCHECK([NSThread isMainThread]); + NSURL* ns_url = net::NSURLWithGURL(url); + if (!ns_url) { + return false; + } + + CFURLRef openingApp = NULL; + OSStatus status = LSGetApplicationForURL((CFURLRef)ns_url, + kLSRolesAll, + NULL, + &openingApp); + if (status != noErr) { + return false; + } + CFRelease(openingApp); // NOT A BUG; LSGetApplicationForURL retains for us + + NSUInteger launchOptions = NSWorkspaceLaunchDefault; + if (!activate) + launchOptions |= NSWorkspaceLaunchWithoutActivation; + + return open(ns_url, launchOptions); +} + +} // namespace + namespace platform_util { bool ShowItemInFolder(const base::FilePath& path) { @@ -129,51 +167,17 @@ bool OpenItem(const base::FilePath& full_path) { return status == noErr; } -bool openURLInWorkspace(NSURL* ns_url, NSUInteger launchOptions) { - return [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] - withAppBundleIdentifier: nil - options: launchOptions - additionalEventParamDescriptor: NULL - launchIdentifiers: NULL]; -} - -typedef bool(^OpenExternalBlock)(NSURL* ns_url, NSUInteger launchOptions); - -bool openExternal(const GURL& url, bool activate, OpenExternalBlock open) { - DCHECK([NSThread isMainThread]); - NSURL* ns_url = net::NSURLWithGURL(url); - if (!ns_url) { - return false; - } - - CFURLRef openingApp = NULL; - OSStatus status = LSGetApplicationForURL((CFURLRef)ns_url, - kLSRolesAll, - NULL, - &openingApp); - if (status != noErr) { - return false; - } - CFRelease(openingApp); // NOT A BUG; LSGetApplicationForURL retains for us - - NSUInteger launchOptions = NSWorkspaceLaunchDefault; - if (!activate) - launchOptions |= NSWorkspaceLaunchWithoutActivation; - - return open(ns_url, launchOptions); -} - bool OpenExternal(const GURL& url, bool activate) { - return openExternal(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { - return openURLInWorkspace(ns_url, launchOptions); + return OpenExternalWithBlock(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { + return OpenURLInWorkspace(ns_url, launchOptions); }); } bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { __block OpenExternalCallback callback = c; - return openExternal(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { + return OpenExternalWithBlock(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - bool opened = openURLInWorkspace(ns_url, launchOptions); + bool opened = OpenURLInWorkspace(ns_url, launchOptions); dispatch_async(dispatch_get_main_queue(), ^{ callback.Run(opened); }); From 99a5258999cabb2c8e65306d05c3355b43b9f644 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:20:26 -0700 Subject: [PATCH 12/21] Callback uses (platform specific) error, with message --- atom/common/api/atom_api_shell.cc | 3 +- atom/common/platform_util.h | 5 +- atom/common/platform_util_linux.cc | 10 +++- atom/common/platform_util_mac.mm | 90 +++++++++++++++++++++--------- atom/common/platform_util_win.cc | 10 +++- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index 894c85eeea..eb91e53ff2 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -62,7 +62,8 @@ bool OpenExternal( platform_util::OpenExternalCallback callback; if (mate::Converter::FromV8( args->isolate(), peek, &callback)) { - return platform_util::OpenExternal(url, activate, callback); + platform_util::OpenExternal(url, activate, callback); + return true; } return false; } diff --git a/atom/common/platform_util.h b/atom/common/platform_util.h index d7bb920648..e3153cb082 100644 --- a/atom/common/platform_util.h +++ b/atom/common/platform_util.h @@ -7,6 +7,7 @@ #include "base/callback_forward.h" #include "build/build_config.h" +#include "v8/include/v8.h" #if defined(OS_WIN) #include "base/strings/string16.h" @@ -28,7 +29,7 @@ bool ShowItemInFolder(const base::FilePath& full_path); // Must be called from the UI thread. bool OpenItem(const base::FilePath& full_path); -typedef base::Callback OpenExternalCallback; +typedef base::Callback error)> OpenExternalCallback; // Open the given external protocol URL in the desktop's default manner. // (For example, mailto: URLs in the default mail user agent.) @@ -40,7 +41,7 @@ bool OpenExternal( #endif bool activate); -bool OpenExternal( +void OpenExternal( #if defined(OS_WIN) const base::string16& url, #else diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index ee08d2832b..039e26e263 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -89,12 +89,16 @@ bool OpenExternal(const GURL& url, bool activate) { return XDGOpen(url.spec(), false); } -bool OpenExternal(const GURL& url, bool activate, +void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); - callback.Run(opened); - return opened; + if (!opened) { + callback.Run(v8::Exception::Error( + v8::String::NewFromUtf8(isolate, @"Failed to open"))); + } else { + callback.Run(v8::Null(isolate);) + } } bool MoveItemToTrash(const base::FilePath& full_path) { diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index b88bd539ce..0bd902af57 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -19,30 +19,43 @@ namespace { -bool OpenURLInWorkspace(NSURL* ns_url, NSUInteger launchOptions) { - return [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] - withAppBundleIdentifier: nil - options: launchOptions - additionalEventParamDescriptor: NULL - launchIdentifiers: NULL]; +NSError *PlatformError(NSString* message) { + return [NSError errorWithDomain:@"Electron" + code:0 + userInfo:@{NSLocalizedDescriptionKey: message}]; } -typedef bool(^OpenExternalBlock)(NSURL* ns_url, NSUInteger launchOptions); - -bool OpenExternalWithBlock(const GURL& url, bool activate, OpenExternalBlock open) { - DCHECK([NSThread isMainThread]); - NSURL* ns_url = net::NSURLWithGURL(url); - if (!ns_url) { - return false; +NSString *MessageForOSStatus(OSStatus status) { + switch (status) { + case kLSAppInTrashErr: return @"The application cannot be run because it is inside a Trash folder."; + case kLSUnknownErr: return @"An unknown error has occurred."; + case kLSNotAnApplicationErr: return @"The item to be registered is not an application."; + case kLSNotInitializedErr: return @"Formerly returned by LSInit on initialization failure; no longer used."; + case kLSDataUnavailableErr: return @"Data of the desired type is not available (for example, there is no kind string)."; + case kLSApplicationNotFoundErr: return @"No application in the Launch Services database matches the input criteria."; + case kLSDataErr: return @"Data is structured improperly (for example, an item’s information property list is malformed). Not used in macOS 10.4."; + case kLSLaunchInProgressErr: return @"A launch of the application is already in progress."; + case kLSServerCommunicationErr: return @"There is a problem communicating with the server process that maintains the Launch Services database."; + case kLSCannotSetInfoErr: return @"The filename extension to be hidden cannot be hidden."; + case kLSIncompatibleSystemVersionErr: return @"The application to be launched cannot run on the current Mac OS version."; + case kLSNoLaunchPermissionErr: return @"The user does not have permission to launch the application (on a managed network)."; + case kLSNoExecutableErr: return @"The executable file is missing or has an unusable format."; + case kLSNoClassicEnvironmentErr: return @"The Classic emulation environment was required but is not available."; + case kLSMultipleSessionsNotSupportedErr: return @"The application to be launched cannot run simultaneously in two different user sessions."; + default: return [NSString stringWithFormat:@"Failed to open (%@)", @(status)]; } +} +// This may be called from a global dispatch queue, the methods used here are thread safe, +// including LSGetApplicationForURL (> 10.2) and NSWorkspace#openURLs. +NSError* OpenURL(NSURL* ns_url, bool activate) { CFURLRef openingApp = NULL; OSStatus status = LSGetApplicationForURL((CFURLRef)ns_url, kLSRolesAll, NULL, &openingApp); if (status != noErr) { - return false; + return PlatformError(MessageForOSStatus(status)); } CFRelease(openingApp); // NOT A BUG; LSGetApplicationForURL retains for us @@ -50,7 +63,23 @@ bool OpenExternalWithBlock(const GURL& url, bool activate, OpenExternalBlock ope if (!activate) launchOptions |= NSWorkspaceLaunchWithoutActivation; - return open(ns_url, launchOptions); + bool opened = [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] + withAppBundleIdentifier: nil + options: launchOptions + additionalEventParamDescriptor: nil + launchIdentifiers: nil]; + if (!opened) return PlatformError(@"Failed to open URL"); + return nil; +} + +v8::Local ConvertNSError(v8::Isolate* isolate, NSError* platformError) { + if (!platformError) { + return v8::Null(isolate); + } else { + v8::Local error_message = + v8::String::NewFromUtf8(isolate, platformError.localizedDescription.UTF8String); + return v8::Exception::Error(error_message); + } } } // namespace @@ -168,21 +197,30 @@ bool OpenItem(const base::FilePath& full_path) { } bool OpenExternal(const GURL& url, bool activate) { - return OpenExternalWithBlock(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { - return OpenURLInWorkspace(ns_url, launchOptions); - }); + DCHECK([NSThread isMainThread]); + NSURL* ns_url = net::NSURLWithGURL(url); + if (!ns_url) { + return false; + } + NSError *error = OpenURL(ns_url, activate); + return error ? false : true; } -bool OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { +void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { + NSURL* ns_url = net::NSURLWithGURL(url); + if (!ns_url) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + c.Run(ConvertNSError(isolate, PlatformError(@"Invalid URL"))); + return; + } + __block OpenExternalCallback callback = c; - return OpenExternalWithBlock(url, activate, ^bool(NSURL* ns_url, NSUInteger launchOptions) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - bool opened = OpenURLInWorkspace(ns_url, launchOptions); - dispatch_async(dispatch_get_main_queue(), ^{ - callback.Run(opened); - }); + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + NSError *openError = OpenURL(ns_url, activate); + dispatch_async(dispatch_get_main_queue(), ^{ + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + callback.Run(ConvertNSError(isolate, openError)); }); - return YES; }); } diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index d2f2b1ca11..acf663b1dc 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -316,12 +316,16 @@ bool OpenExternal(const base::string16& url, bool activate) { return true; } -bool OpenExternal(const base::string16& url, bool activate, +void OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); - callback.Run(opened); - return opened; + if (!opened) { + callback.Run(v8::Exception::Error( + v8::String::NewFromUtf8(isolate, @"Failed to open"))); + } else { + callback.Run(v8::Null(isolate);) + } } bool MoveItemToTrash(const base::FilePath& path) { From 31123f908da20405c45dacf708e89fa6a669b9fb Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:22:20 -0700 Subject: [PATCH 13/21] Fix ternary --- atom/common/platform_util_mac.mm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index 0bd902af57..e630b264d2 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -202,8 +202,7 @@ bool OpenExternal(const GURL& url, bool activate) { if (!ns_url) { return false; } - NSError *error = OpenURL(ns_url, activate); - return error ? false : true; + return !OpenURL(ns_url, activate); } void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { From 2931c2739586612dcaf67b8b16e72c52a3be2617 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:23:37 -0700 Subject: [PATCH 14/21] More readable --- atom/common/platform_util_mac.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index e630b264d2..2310a245e8 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -202,7 +202,8 @@ bool OpenExternal(const GURL& url, bool activate) { if (!ns_url) { return false; } - return !OpenURL(ns_url, activate); + NSError *error = OpenURL(ns_url, activate); + return !error; } void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { From 54222bdf284db3884ad2213b67fd58fe095bbd96 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:26:30 -0700 Subject: [PATCH 15/21] More general string for OSStatus --- atom/common/platform_util_mac.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index 2310a245e8..5e2d948ad4 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -25,7 +25,7 @@ NSError *PlatformError(NSString* message) { userInfo:@{NSLocalizedDescriptionKey: message}]; } -NSString *MessageForOSStatus(OSStatus status) { +NSString *MessageForOSStatus(OSStatus status, NSString* defaultMessage) { switch (status) { case kLSAppInTrashErr: return @"The application cannot be run because it is inside a Trash folder."; case kLSUnknownErr: return @"An unknown error has occurred."; @@ -42,7 +42,7 @@ NSString *MessageForOSStatus(OSStatus status) { case kLSNoExecutableErr: return @"The executable file is missing or has an unusable format."; case kLSNoClassicEnvironmentErr: return @"The Classic emulation environment was required but is not available."; case kLSMultipleSessionsNotSupportedErr: return @"The application to be launched cannot run simultaneously in two different user sessions."; - default: return [NSString stringWithFormat:@"Failed to open (%@)", @(status)]; + default: return [NSString stringWithFormat:@"%@ (%@)", defaultMessage, @(status)]; } } @@ -55,7 +55,7 @@ NSError* OpenURL(NSURL* ns_url, bool activate) { NULL, &openingApp); if (status != noErr) { - return PlatformError(MessageForOSStatus(status)); + return PlatformError(MessageForOSStatus(status, @"Failed to open")); } CFRelease(openingApp); // NOT A BUG; LSGetApplicationForURL retains for us From 9382d48708f1b7a3d09f38fe1d9bd16d7d922b99 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:29:25 -0700 Subject: [PATCH 16/21] Update docs --- docs/api/shell.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/shell.md b/docs/api/shell.md index 75eccb518e..411ef9cce5 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -41,10 +41,10 @@ Open the given file in the desktop's default manner. * `activate` Boolean - `true` to bring the opened application to the foreground. The default is `true`. * `callback` Function (optional) - If specified will perform the open asynchronously. _macOS_ - * `opened` Boolean + * `error` Error Returns `Boolean` - Whether an application was available to open the URL. -If callback is specified, it will return whether it was able to initiate the open call. +If callback is specified, always returns true. Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). From f0ca9dff810db29bffedb7536b8ab7584401912a Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:34:56 -0700 Subject: [PATCH 17/21] Fix win/linux compile --- atom/common/platform_util_linux.cc | 4 ++-- atom/common/platform_util_win.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 039e26e263..a8940fd68f 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -95,9 +95,9 @@ void OpenExternal(const GURL& url, bool activate, bool opened = OpenExternal(url, activate); if (!opened) { callback.Run(v8::Exception::Error( - v8::String::NewFromUtf8(isolate, @"Failed to open"))); + v8::String::NewFromUtf8(isolate, "Failed to open"))); } else { - callback.Run(v8::Null(isolate);) + callback.Run(v8::Null(isolate)); } } diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index acf663b1dc..34cc2b1835 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -322,9 +322,9 @@ void OpenExternal(const base::string16& url, bool activate, bool opened = OpenExternal(url, activate); if (!opened) { callback.Run(v8::Exception::Error( - v8::String::NewFromUtf8(isolate, @"Failed to open"))); + v8::String::NewFromUtf8(isolate, "Failed to open"))); } else { - callback.Run(v8::Null(isolate);) + callback.Run(v8::Null(isolate)); } } From 5b260dbee3dce3b07d00a3fcf7bb262c5a6ab963 Mon Sep 17 00:00:00 2001 From: Gabriel Handford Date: Wed, 2 Nov 2016 16:36:01 -0700 Subject: [PATCH 18/21] Fix win/linux compile (again) --- atom/common/platform_util_linux.cc | 1 + atom/common/platform_util_win.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index a8940fd68f..63488fe7df 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -93,6 +93,7 @@ void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); if (!opened) { callback.Run(v8::Exception::Error( v8::String::NewFromUtf8(isolate, "Failed to open"))); diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index 34cc2b1835..6c59699a83 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -320,6 +320,7 @@ void OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified bool opened = OpenExternal(url, activate); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); if (!opened) { callback.Run(v8::Exception::Error( v8::String::NewFromUtf8(isolate, "Failed to open"))); From 622bad1cff50412619aa7f92ff31ecd84e2e429e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 17 Nov 2016 10:36:16 +0900 Subject: [PATCH 19/21] Simplify getting callback --- atom/common/api/atom_api_shell.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index eb91e53ff2..6b39191596 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -58,14 +58,11 @@ bool OpenExternal( } if (args->Length() >= 3) { - v8::Local peek = args->PeekNext(); platform_util::OpenExternalCallback callback; - if (mate::Converter::FromV8( - args->isolate(), peek, &callback)) { + if (args->GetNext(&callback)) { platform_util::OpenExternal(url, activate, callback); return true; } - return false; } return platform_util::OpenExternal(url, activate); From 090a5d9a619b047d165bdc388d67c05e049bfc3d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 17 Nov 2016 11:22:09 +0900 Subject: [PATCH 20/21] platform_util code should not involve V8 code --- atom/common/api/atom_api_shell.cc | 16 +++- atom/common/platform_util.h | 8 +- atom/common/platform_util_mac.mm | 130 ++++++++++++++++-------------- 3 files changed, 90 insertions(+), 64 deletions(-) diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index 6b39191596..39e6aea7ba 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -42,6 +42,16 @@ struct Converter { namespace { +void OnOpenExternalFinished( + v8::Isolate* isolate, + const base::Callback)>& callback, + const std::string& error) { + if (error.empty()) + callback.Run(v8::Null(isolate)); + else + callback.Run(v8::String::NewFromUtf8(isolate, error.c_str())); +} + bool OpenExternal( #if defined(OS_WIN) const base::string16& url, @@ -58,9 +68,11 @@ bool OpenExternal( } if (args->Length() >= 3) { - platform_util::OpenExternalCallback callback; + base::Callback)> callback; if (args->GetNext(&callback)) { - platform_util::OpenExternal(url, activate, callback); + platform_util::OpenExternal( + url, activate, + base::Bind(&OnOpenExternalFinished, args->isolate(), callback)); return true; } } diff --git a/atom/common/platform_util.h b/atom/common/platform_util.h index e3153cb082..dc4b472358 100644 --- a/atom/common/platform_util.h +++ b/atom/common/platform_util.h @@ -5,9 +5,10 @@ #ifndef ATOM_COMMON_PLATFORM_UTIL_H_ #define ATOM_COMMON_PLATFORM_UTIL_H_ +#include + #include "base/callback_forward.h" #include "build/build_config.h" -#include "v8/include/v8.h" #if defined(OS_WIN) #include "base/strings/string16.h" @@ -21,6 +22,8 @@ class FilePath; namespace platform_util { +typedef base::Callback OpenExternalCallback; + // Show the given file in a file manager. If possible, select the file. // Must be called from the UI thread. bool ShowItemInFolder(const base::FilePath& full_path); @@ -29,8 +32,6 @@ bool ShowItemInFolder(const base::FilePath& full_path); // Must be called from the UI thread. bool OpenItem(const base::FilePath& full_path); -typedef base::Callback error)> OpenExternalCallback; - // Open the given external protocol URL in the desktop's default manner. // (For example, mailto: URLs in the default mail user agent.) bool OpenExternal( @@ -41,6 +42,7 @@ bool OpenExternal( #endif bool activate); +// The asynchronous version of OpenExternal. void OpenExternal( #if defined(OS_WIN) const base::string16& url, diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index 5e2d948ad4..aa64678caf 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -4,82 +4,97 @@ #include "atom/common/platform_util.h" -#include +#import #import -#include "base/cancelable_callback.h" +#include "base/callback.h" #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/logging.h" #include "base/mac/mac_logging.h" #include "base/mac/scoped_aedesc.h" +#include "base/strings/stringprintf.h" #include "base/strings/sys_string_conversions.h" #include "net/base/mac/url_conversions.h" #include "url/gurl.h" namespace { -NSError *PlatformError(NSString* message) { - return [NSError errorWithDomain:@"Electron" - code:0 - userInfo:@{NSLocalizedDescriptionKey: message}]; -} - -NSString *MessageForOSStatus(OSStatus status, NSString* defaultMessage) { +std::string MessageForOSStatus(OSStatus status, const char* default_message) { switch (status) { - case kLSAppInTrashErr: return @"The application cannot be run because it is inside a Trash folder."; - case kLSUnknownErr: return @"An unknown error has occurred."; - case kLSNotAnApplicationErr: return @"The item to be registered is not an application."; - case kLSNotInitializedErr: return @"Formerly returned by LSInit on initialization failure; no longer used."; - case kLSDataUnavailableErr: return @"Data of the desired type is not available (for example, there is no kind string)."; - case kLSApplicationNotFoundErr: return @"No application in the Launch Services database matches the input criteria."; - case kLSDataErr: return @"Data is structured improperly (for example, an item’s information property list is malformed). Not used in macOS 10.4."; - case kLSLaunchInProgressErr: return @"A launch of the application is already in progress."; - case kLSServerCommunicationErr: return @"There is a problem communicating with the server process that maintains the Launch Services database."; - case kLSCannotSetInfoErr: return @"The filename extension to be hidden cannot be hidden."; - case kLSIncompatibleSystemVersionErr: return @"The application to be launched cannot run on the current Mac OS version."; - case kLSNoLaunchPermissionErr: return @"The user does not have permission to launch the application (on a managed network)."; - case kLSNoExecutableErr: return @"The executable file is missing or has an unusable format."; - case kLSNoClassicEnvironmentErr: return @"The Classic emulation environment was required but is not available."; - case kLSMultipleSessionsNotSupportedErr: return @"The application to be launched cannot run simultaneously in two different user sessions."; - default: return [NSString stringWithFormat:@"%@ (%@)", defaultMessage, @(status)]; + case kLSAppInTrashErr: + return "The application cannot be run because it is inside a Trash " + "folder."; + case kLSUnknownErr: + return "An unknown error has occurred."; + case kLSNotAnApplicationErr: + return "The item to be registered is not an application."; + case kLSNotInitializedErr: + return "Formerly returned by LSInit on initialization failure; " + "no longer used."; + case kLSDataUnavailableErr: + return "Data of the desired type is not available (for example, there is " + "no kind string)."; + case kLSApplicationNotFoundErr: + return "No application in the Launch Services database matches the input " + "criteria."; + case kLSDataErr: + return "Data is structured improperly (for example, an item’s " + "information property list is malformed). Not used in macOS 10.4."; + case kLSLaunchInProgressErr: + return "A launch of the application is already in progress."; + case kLSServerCommunicationErr: + return "There is a problem communicating with the server process that " + "maintains the Launch Services database."; + case kLSCannotSetInfoErr: + return "The filename extension to be hidden cannot be hidden."; + case kLSIncompatibleSystemVersionErr: + return "The application to be launched cannot run on the current Mac OS " + "version."; + case kLSNoLaunchPermissionErr: + return "The user does not have permission to launch the application (on a" + "managed network)."; + case kLSNoExecutableErr: + return "The executable file is missing or has an unusable format."; + case kLSNoClassicEnvironmentErr: + return "The Classic emulation environment was required but is not " + "available."; + case kLSMultipleSessionsNotSupportedErr: + return "The application to be launched cannot run simultaneously in two " + "different user sessions."; + default: + return base::StringPrintf("%s (%d)", default_message, status); } } -// This may be called from a global dispatch queue, the methods used here are thread safe, -// including LSGetApplicationForURL (> 10.2) and NSWorkspace#openURLs. -NSError* OpenURL(NSURL* ns_url, bool activate) { +// This may be called from a global dispatch queue, the methods used here are +// thread safe, including LSGetApplicationForURL (> 10.2) and +// NSWorkspace#openURLs. +std::string OpenURL(NSURL* ns_url, bool activate) { CFURLRef openingApp = NULL; OSStatus status = LSGetApplicationForURL((CFURLRef)ns_url, kLSRolesAll, NULL, &openingApp); - if (status != noErr) { - return PlatformError(MessageForOSStatus(status, @"Failed to open")); - } + if (status != noErr) + return MessageForOSStatus(status, "Failed to open"); + CFRelease(openingApp); // NOT A BUG; LSGetApplicationForURL retains for us NSUInteger launchOptions = NSWorkspaceLaunchDefault; if (!activate) launchOptions |= NSWorkspaceLaunchWithoutActivation; - bool opened = [[NSWorkspace sharedWorkspace] openURLs: @[ns_url] - withAppBundleIdentifier: nil - options: launchOptions - additionalEventParamDescriptor: nil - launchIdentifiers: nil]; - if (!opened) return PlatformError(@"Failed to open URL"); - return nil; -} + bool opened = [[NSWorkspace sharedWorkspace] + openURLs:@[ns_url] + withAppBundleIdentifier:nil + options:launchOptions + additionalEventParamDescriptor:nil + launchIdentifiers:nil]; + if (!opened) + return "Failed to open URL"; -v8::Local ConvertNSError(v8::Isolate* isolate, NSError* platformError) { - if (!platformError) { - return v8::Null(isolate); - } else { - v8::Local error_message = - v8::String::NewFromUtf8(isolate, platformError.localizedDescription.UTF8String); - return v8::Exception::Error(error_message); - } + return ""; } } // namespace @@ -199,27 +214,24 @@ bool OpenItem(const base::FilePath& full_path) { bool OpenExternal(const GURL& url, bool activate) { DCHECK([NSThread isMainThread]); NSURL* ns_url = net::NSURLWithGURL(url); - if (!ns_url) { - return false; - } - NSError *error = OpenURL(ns_url, activate); - return !error; + if (ns_url) + return OpenURL(ns_url, activate).empty(); + return false; } -void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& c) { +void OpenExternal(const GURL& url, bool activate, + const OpenExternalCallback& callback) { NSURL* ns_url = net::NSURLWithGURL(url); if (!ns_url) { - v8::Isolate* isolate = v8::Isolate::GetCurrent(); - c.Run(ConvertNSError(isolate, PlatformError(@"Invalid URL"))); + callback.Run("Invalid URL"); return; } - __block OpenExternalCallback callback = c; + __block OpenExternalCallback c = callback; dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSError *openError = OpenURL(ns_url, activate); + __block std::string error = OpenURL(ns_url, activate); dispatch_async(dispatch_get_main_queue(), ^{ - v8::Isolate* isolate = v8::Isolate::GetCurrent(); - callback.Run(ConvertNSError(isolate, openError)); + c.Run(error); }); }); } From 5639faf0697a8a55464eb6cbfcc6d605c09f01a2 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 17 Nov 2016 12:13:07 +0900 Subject: [PATCH 21/21] Also fix the Windows and Linux side of async openExternal --- atom/common/platform_util_linux.cc | 9 +-------- atom/common/platform_util_win.cc | 9 +-------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 63488fe7df..923adbd882 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -92,14 +92,7 @@ bool OpenExternal(const GURL& url, bool activate) { void OpenExternal(const GURL& url, bool activate, const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified - bool opened = OpenExternal(url, activate); - v8::Isolate* isolate = v8::Isolate::GetCurrent(); - if (!opened) { - callback.Run(v8::Exception::Error( - v8::String::NewFromUtf8(isolate, "Failed to open"))); - } else { - callback.Run(v8::Null(isolate)); - } + callback.Run(OpenExternal(url, activate) ? "" : "Failed to open"); } bool MoveItemToTrash(const base::FilePath& full_path) { diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index 6c59699a83..348868dc58 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -319,14 +319,7 @@ bool OpenExternal(const base::string16& url, bool activate) { void OpenExternal(const base::string16& url, bool activate, const OpenExternalCallback& callback) { // TODO(gabriel): Implement async open if callback is specified - bool opened = OpenExternal(url, activate); - v8::Isolate* isolate = v8::Isolate::GetCurrent(); - if (!opened) { - callback.Run(v8::Exception::Error( - v8::String::NewFromUtf8(isolate, "Failed to open"))); - } else { - callback.Run(v8::Null(isolate)); - } + callback.Run(OpenExternal(url, activate) ? "" : "Failed to open"); } bool MoveItemToTrash(const base::FilePath& path) {