X Tutup
Skip to content

Commit 6d96f30

Browse files
codebyterecodebytere
authored
refactor: make shell.OpenExternal async (electron#17135)
1 parent 0755857 commit 6d96f30

File tree

16 files changed

+69
-139
lines changed

16 files changed

+69
-139
lines changed

‎atom/browser/atom_browser_client.cc‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -794,14 +794,10 @@ void AtomBrowserClient::RenderProcessExited(
794794
}
795795

796796
void OnOpenExternal(const GURL& escaped_url, bool allowed) {
797-
if (allowed)
797+
if (allowed) {
798798
platform_util::OpenExternal(
799-
#if defined(OS_WIN)
800-
base::UTF8ToUTF16(escaped_url.spec()),
801-
#else
802-
escaped_url,
803-
#endif
804-
platform_util::OpenExternalOptions());
799+
escaped_url, platform_util::OpenExternalOptions(), base::DoNothing());
800+
}
805801
}
806802

807803
void HandleExternalProtocolInUI(

‎atom/common/api/atom_api_shell.cc‎

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -52,33 +52,9 @@ void OnOpenExternalFinished(atom::util::Promise promise,
5252
promise.RejectWithErrorMessage(error.c_str());
5353
}
5454

55-
bool OpenExternalSync(
56-
#if defined(OS_WIN)
57-
const base::string16& url,
58-
#else
59-
const GURL& url,
60-
#endif
61-
mate::Arguments* args) {
62-
platform_util::OpenExternalOptions options;
63-
if (args->Length() >= 2) {
64-
mate::Dictionary obj;
65-
if (args->GetNext(&obj)) {
66-
obj.Get("activate", &options.activate);
67-
obj.Get("workingDirectory", &options.working_dir);
68-
}
69-
}
70-
71-
return platform_util::OpenExternal(url, options);
72-
}
73-
74-
v8::Local<v8::Promise> OpenExternal(
75-
#if defined(OS_WIN)
76-
const base::string16& url,
77-
#else
78-
const GURL& url,
79-
#endif
80-
mate::Arguments* args) {
55+
v8::Local<v8::Promise> OpenExternal(const GURL& url, mate::Arguments* args) {
8156
atom::util::Promise promise(args->isolate());
57+
v8::Local<v8::Promise> handle = promise.GetHandle();
8258

8359
platform_util::OpenExternalOptions options;
8460
if (args->Length() >= 2) {
@@ -89,7 +65,6 @@ v8::Local<v8::Promise> OpenExternal(
8965
}
9066
}
9167

92-
v8::Local<v8::Promise> handle = promise.GetHandle();
9368
platform_util::OpenExternal(
9469
url, options,
9570
base::BindOnce(&OnOpenExternalFinished, std::move(promise)));
@@ -158,7 +133,6 @@ void Initialize(v8::Local<v8::Object> exports,
158133
mate::Dictionary dict(context->GetIsolate(), exports);
159134
dict.SetMethod("showItemInFolder", &platform_util::ShowItemInFolder);
160135
dict.SetMethod("openItem", &platform_util::OpenItem);
161-
dict.SetMethod("openExternalSync", &OpenExternalSync);
162136
dict.SetMethod("openExternal", &OpenExternal);
163137
dict.SetMethod("moveItemToTrash", &platform_util::MoveItemToTrash);
164138
dict.SetMethod("beep", &platform_util::Beep);

‎atom/common/platform_util.h‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,9 @@ struct OpenExternalOptions {
3636

3737
// Open the given external protocol URL in the desktop's default manner.
3838
// (For example, mailto: URLs in the default mail user agent.)
39-
bool OpenExternal(
40-
#if defined(OS_WIN)
41-
const base::string16& url,
42-
#else
43-
const GURL& url,
44-
#endif
45-
const OpenExternalOptions& options);
46-
47-
// The asynchronous version of OpenExternal.
48-
void OpenExternal(
49-
#if defined(OS_WIN)
50-
const base::string16& url,
51-
#else
52-
const GURL& url,
53-
#endif
54-
const OpenExternalOptions& options,
55-
OpenExternalCallback callback);
39+
void OpenExternal(const GURL& url,
40+
const OpenExternalOptions& options,
41+
OpenExternalCallback callback);
5642

5743
// Move a file to trash.
5844
bool MoveItemToTrash(const base::FilePath& full_path);

‎atom/common/platform_util_linux.cc‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,16 @@ bool OpenItem(const base::FilePath& full_path) {
8181
return XDGOpen(full_path.value(), false);
8282
}
8383

84-
bool OpenExternal(const GURL& url, const OpenExternalOptions& options) {
84+
void OpenExternal(const GURL& url,
85+
const OpenExternalOptions& options,
86+
OpenExternalCallback callback) {
8587
// Don't wait for exit, since we don't want to wait for the browser/email
8688
// client window to close before returning
8789
if (url.SchemeIs("mailto"))
88-
return XDGEmail(url.spec(), false);
90+
std::move(callback).Run(XDGEmail(url.spec(), false) ? ""
91+
: "Failed to open");
8992
else
90-
return XDGOpen(url.spec(), false);
91-
}
92-
93-
void OpenExternal(const GURL& url,
94-
const OpenExternalOptions& options,
95-
OpenExternalCallback callback) {
96-
// TODO(gabriel): Implement async open if callback is specified
97-
std::move(callback).Run(OpenExternal(url, options) ? "" : "Failed to open");
93+
std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open");
9894
}
9995

10096
bool MoveItemToTrash(const base::FilePath& full_path) {

‎atom/common/platform_util_mac.mm‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,10 @@ bool OpenItem(const base::FilePath& full_path) {
9494
launchIdentifiers:NULL];
9595
}
9696

97-
bool OpenExternal(const GURL& url, const OpenExternalOptions& options) {
98-
DCHECK([NSThread isMainThread]);
99-
NSURL* ns_url = net::NSURLWithGURL(url);
100-
if (ns_url)
101-
return OpenURL(ns_url, options.activate).empty();
102-
return false;
103-
}
104-
10597
void OpenExternal(const GURL& url,
10698
const OpenExternalOptions& options,
10799
OpenExternalCallback callback) {
100+
DCHECK([NSThread isMainThread]);
108101
NSURL* ns_url = net::NSURLWithGURL(url);
109102
if (!ns_url) {
110103
std::move(callback).Run("Invalid URL");

‎atom/common/platform_util_win.cc‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,29 @@ HRESULT DeleteFileProgressSink::ResumeTimer() {
230230
return S_OK;
231231
}
232232

233+
std::string OpenExternalOnWorkerThread(
234+
const GURL& url,
235+
const platform_util::OpenExternalOptions& options) {
236+
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
237+
base::BlockingType::MAY_BLOCK);
238+
// Quote the input scheme to be sure that the command does not have
239+
// parameters unexpected by the external program. This url should already
240+
// have been escaped.
241+
std::string escaped_url = url.spec();
242+
escaped_url.insert(0, "\"");
243+
escaped_url += "\"";
244+
245+
std::string working_dir = options.working_dir.AsUTF8Unsafe();
246+
247+
if (reinterpret_cast<ULONG_PTR>(
248+
ShellExecuteA(nullptr, "open", escaped_url.c_str(), nullptr,
249+
working_dir.empty() ? nullptr : working_dir.c_str(),
250+
SW_SHOWNORMAL)) <= 32) {
251+
return "Failed to open";
252+
}
253+
return "";
254+
}
255+
233256
void ShowItemInFolderOnWorkerThread(const base::FilePath& full_path) {
234257
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
235258
base::BlockingType::MAY_BLOCK);
@@ -301,31 +324,15 @@ bool OpenItem(const base::FilePath& full_path) {
301324
return ui::win::OpenFileViaShell(full_path);
302325
}
303326

304-
bool OpenExternal(const base::string16& url,
305-
const OpenExternalOptions& options) {
306-
// Quote the input scheme to be sure that the command does not have
307-
// parameters unexpected by the external program. This url should already
308-
// have been escaped.
309-
base::string16 escaped_url = L"\"" + url + L"\"";
310-
auto working_dir = options.working_dir.value();
311-
312-
if (reinterpret_cast<ULONG_PTR>(
313-
ShellExecuteW(nullptr, L"open", escaped_url.c_str(), nullptr,
314-
working_dir.empty() ? nullptr : working_dir.c_str(),
315-
SW_SHOWNORMAL)) <= 32) {
316-
// We fail to execute the call. We could display a message to the user.
317-
// TODO(nsylvain): we should also add a dialog to warn on errors. See
318-
// bug 1136923.
319-
return false;
320-
}
321-
return true;
322-
}
323-
324-
void OpenExternal(const base::string16& url,
327+
void OpenExternal(const GURL& url,
325328
const OpenExternalOptions& options,
326329
OpenExternalCallback callback) {
327-
// TODO(gabriel): Implement async open if callback is specified
328-
std::move(callback).Run(OpenExternal(url, options) ? "" : "Failed to open");
330+
base::PostTaskAndReplyWithResult(
331+
base::CreateCOMSTATaskRunnerWithTraits(
332+
{base::MayBlock(), base::TaskPriority::USER_BLOCKING})
333+
.get(),
334+
FROM_HERE, base::BindOnce(&OpenExternalOnWorkerThread, url, options),
335+
std::move(callback));
329336
}
330337

331338
bool MoveItemToTrash(const base::FilePath& path) {

‎docs/api/menu.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,10 @@ const template = [
240240
submenu: [
241241
{
242242
label: 'Learn More',
243-
click () { require('electron').shell.openExternalSync('https://electronjs.org') }
243+
click: async () => {
244+
const { shell } = require('electron')
245+
await shell.openExternal('https://electronjs.org')
246+
}
244247
}
245248
]
246249
}

‎docs/api/shell.md‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,6 @@ Returns `Boolean` - Whether the item was successfully opened.
3232

3333
Open the given file in the desktop's default manner.
3434

35-
### `shell.openExternalSync(url[, options])`
36-
37-
* `url` String - Max 2081 characters on Windows, or the function returns false.
38-
* `options` Object (optional)
39-
* `activate` Boolean (optional) - `true` to bring the opened application to the
40-
foreground. The default is `true`. _macOS_
41-
* `workingDirectory` String (optional) - The working directory. _Windows_
42-
43-
Returns `Boolean` - Whether an application was available to open the URL.
44-
45-
Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent).
46-
4735
### `shell.openExternal(url[, options])`
4836

4937
* `url` String - Max 2081 characters on windows.

‎docs/api/webview-tag.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -774,10 +774,10 @@ The following example code opens the new url in system's default browser.
774774
const { shell } = require('electron')
775775
const webview = document.querySelector('webview')
776776

777-
webview.addEventListener('new-window', (e) => {
777+
webview.addEventListener('new-window', async (e) => {
778778
const protocol = require('url').parse(e.url).protocol
779779
if (protocol === 'http:' || protocol === 'https:') {
780-
shell.openExternalSync(e.url)
780+
await shell.openExternal(e.url)
781781
}
782782
})
783783
```

‎docs/tutorial/electron-timelines.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,4 @@ Take a look at 5.0.0 Timeline [blog post](https://electronjs.org/blog/electron-5
4747
| Tue, 2019-Jul-30 | 6.0.0 | ✨ stable ✨ |
4848

4949
## 7.0.0 Release Schedule
50-
TBD
50+
TBD

0 commit comments

Comments
 (0)
X Tutup