refactor: make shell.OpenExternal async (#17135)

atom/browser/atom_browser_client.cc

@@ -794,14 +794,10 @@ void AtomBrowserClient::RenderProcessExited(
}

void OnOpenExternal(const GURL& escaped_url, bool allowed) {
  if (allowed)
  if (allowed) {
    platform_util::OpenExternal(
#if defined(OS_WIN)
        base::UTF8ToUTF16(escaped_url.spec()),
#else
        escaped_url,
#endif
        platform_util::OpenExternalOptions());
        escaped_url, platform_util::OpenExternalOptions(), base::DoNothing());
  }
}

void HandleExternalProtocolInUI(

atom/common/api/atom_api_shell.cc

@@ -52,33 +52,9 @@ void OnOpenExternalFinished(atom::util::Promise promise,
    promise.RejectWithErrorMessage(error.c_str());
}

bool OpenExternalSync(
#if defined(OS_WIN)
    const base::string16& url,
#else
    const GURL& url,
#endif
    mate::Arguments* args) {
  platform_util::OpenExternalOptions options;
  if (args->Length() >= 2) {
    mate::Dictionary obj;
    if (args->GetNext(&obj)) {
      obj.Get("activate", &options.activate);
      obj.Get("workingDirectory", &options.working_dir);
    }
  }

  return platform_util::OpenExternal(url, options);
}

v8::Local<v8::Promise> OpenExternal(
#if defined(OS_WIN)
    const base::string16& url,
#else
    const GURL& url,
#endif
    mate::Arguments* args) {
v8::Local<v8::Promise> OpenExternal(const GURL& url, mate::Arguments* args) {
  atom::util::Promise promise(args->isolate());
  v8::Local<v8::Promise> handle = promise.GetHandle();

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

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

atom/common/platform_util.h

@@ -36,23 +36,9 @@ struct OpenExternalOptions {

// Open the given external protocol URL in the desktop's default manner.
// (For example, mailto: URLs in the default mail user agent.)
bool OpenExternal(
#if defined(OS_WIN)
    const base::string16& url,
#else
    const GURL& url,
#endif
    const OpenExternalOptions& options);

// The asynchronous version of OpenExternal.
void OpenExternal(
#if defined(OS_WIN)
    const base::string16& url,
#else
    const GURL& url,
#endif
    const OpenExternalOptions& options,
    OpenExternalCallback callback);
void OpenExternal(const GURL& url,
                  const OpenExternalOptions& options,
                  OpenExternalCallback callback);

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

atom/common/platform_util_linux.cc

@@ -81,20 +81,16 @@ bool OpenItem(const base::FilePath& full_path) {
  return XDGOpen(full_path.value(), false);
}

bool OpenExternal(const GURL& url, const OpenExternalOptions& options) {
void OpenExternal(const GURL& url,
                  const OpenExternalOptions& options,
                  OpenExternalCallback 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"))
    return XDGEmail(url.spec(), false);
    std::move(callback).Run(XDGEmail(url.spec(), false) ? ""
                                                        : "Failed to open");
  else
    return XDGOpen(url.spec(), false);
}

void OpenExternal(const GURL& url,
                  const OpenExternalOptions& options,
                  OpenExternalCallback callback) {
  // TODO(gabriel): Implement async open if callback is specified
  std::move(callback).Run(OpenExternal(url, options) ? "" : "Failed to open");
    std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open");
}

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

atom/common/platform_util_mac.mm

@@ -94,17 +94,10 @@ bool OpenItem(const base::FilePath& full_path) {
                               launchIdentifiers:NULL];
}

bool OpenExternal(const GURL& url, const OpenExternalOptions& options) {
  DCHECK([NSThread isMainThread]);
  NSURL* ns_url = net::NSURLWithGURL(url);
  if (ns_url)
    return OpenURL(ns_url, options.activate).empty();
  return false;
}

void OpenExternal(const GURL& url,
                  const OpenExternalOptions& options,
                  OpenExternalCallback callback) {
  DCHECK([NSThread isMainThread]);
  NSURL* ns_url = net::NSURLWithGURL(url);
  if (!ns_url) {
    std::move(callback).Run("Invalid URL");

atom/common/platform_util_win.cc

@@ -230,6 +230,29 @@ HRESULT DeleteFileProgressSink::ResumeTimer() {
  return S_OK;
}

std::string OpenExternalOnWorkerThread(
    const GURL& url,
    const platform_util::OpenExternalOptions& options) {
  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                base::BlockingType::MAY_BLOCK);
  // 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.
  std::string escaped_url = url.spec();
  escaped_url.insert(0, "\"");
  escaped_url += "\"";

  std::string working_dir = options.working_dir.AsUTF8Unsafe();

  if (reinterpret_cast<ULONG_PTR>(
          ShellExecuteA(nullptr, "open", escaped_url.c_str(), nullptr,
                        working_dir.empty() ? nullptr : working_dir.c_str(),
                        SW_SHOWNORMAL)) <= 32) {
    return "Failed to open";
  }
  return "";
}

void ShowItemInFolderOnWorkerThread(const base::FilePath& full_path) {
  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                base::BlockingType::MAY_BLOCK);
@@ -301,31 +324,15 @@ bool OpenItem(const base::FilePath& full_path) {
    return ui::win::OpenFileViaShell(full_path);
}

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(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.
    return false;
  }
  return true;
}

void OpenExternal(const base::string16& url,
void OpenExternal(const GURL& url,
                  const OpenExternalOptions& options,
                  OpenExternalCallback callback) {
  // TODO(gabriel): Implement async open if callback is specified
  std::move(callback).Run(OpenExternal(url, options) ? "" : "Failed to open");
  base::PostTaskAndReplyWithResult(
      base::CreateCOMSTATaskRunnerWithTraits(
          {base::MayBlock(), base::TaskPriority::USER_BLOCKING})
          .get(),
      FROM_HERE, base::BindOnce(&OpenExternalOnWorkerThread, url, options),
      std::move(callback));
}

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

docs/api/menu.md

@@ -240,7 +240,10 @@ const template = [
    submenu: [
      {
        label: 'Learn More',
        click () { require('electron').shell.openExternalSync('https://electronjs.org') }
        click: async () => {
          const { shell } = require('electron')
          await shell.openExternal('https://electronjs.org')
        }
      }
    ]
  }

docs/api/shell.md

@@ -32,18 +32,6 @@ Returns `Boolean` - Whether the item was successfully opened.

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

### `shell.openExternalSync(url[, options])`

* `url` String - Max 2081 characters on Windows, or the function returns false.
* `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_

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).

### `shell.openExternal(url[, options])`

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

docs/api/webview-tag.md

@@ -774,10 +774,10 @@ The following example code opens the new url in system's default browser.
const { shell } = require('electron')
const webview = document.querySelector('webview')

webview.addEventListener('new-window', (e) => {
webview.addEventListener('new-window', async (e) => {
  const protocol = require('url').parse(e.url).protocol
  if (protocol === 'http:' || protocol === 'https:') {
    shell.openExternalSync(e.url)
    await shell.openExternal(e.url)
  }
})
```

docs/tutorial/electron-timelines.md

@@ -47,4 +47,4 @@ Take a look at 5.0.0 Timeline [blog post](https://electronjs.org/blog/electron-5
| Tue, 2019-Jul-30 | 6.0.0 | ✨ stable ✨ |

## 7.0.0 Release Schedule
TBD
TBD

docs/tutorial/security.md

@@ -681,12 +681,12 @@ windows, limiting it to only what you need.
const { shell } = require('electron')

app.on('web-contents-created', (event, contents) => {
  contents.on('new-window', (event, navigationUrl) => {
  contents.on('new-window', async (event, navigationUrl) => {
    // In this example, we'll ask the operating system
    // to open this event's url in the default browser.
    event.preventDefault()

    shell.openExternalSync(navigationUrl)
    await shell.openExternal(navigationUrl)
  })
})
```

lib/browser/default-menu.ts

@@ -12,28 +12,27 @@ export const setDefaultApplicationMenu = () => {
    submenu: [
      {
        label: 'Learn More',
        click () {
          shell.openExternalSync('https://electronjs.org')
        click: async () => {
          await shell.openExternal('https://electronjs.org')
        }
      },
      {
        label: 'Documentation',
        click () {
          shell.openExternalSync(
            `https://github.com/electron/electron/tree/v${process.versions.electron}/docs#readme`
          )
        click: async () => {
          const version = process.versions.electron
          await shell.openExternal(`https://github.com/electron/electron/tree/v${version}/docs#readme`)
        }
      },
      {
        label: 'Community Discussions',
        click () {
          shell.openExternalSync('https://discuss.atom.io/c/electron')
        click: async () => {
          await shell.openExternal('https://discuss.atom.io/c/electron')
        }
      },
      {
        label: 'Search Issues',
        click () {
          shell.openExternalSync('https://github.com/electron/electron/issues')
        click: async () => {
          await shell.openExternal('https://github.com/electron/electron/issues')
        }
      }
    ]

spec/api-remote-spec.js

@@ -198,7 +198,6 @@ describe('remote module', () => {
      assert.strictEqual(typeof remote.app.getPath, 'function')
      assert.strictEqual(typeof remote.webContents.getFocusedWebContents, 'function')
      assert.strictEqual(typeof remote.clipboard.readText, 'function')
      assert.strictEqual(typeof remote.shell.openExternalSync, 'function')
    })

    it('returns toString() of original function via toString()', () => {

spec/api-shell-spec.js

@@ -36,7 +36,7 @@ describe('shell module', () => {
      }
    })

    it('opens an external link asynchronously', done => {
    it('opens an external link', done => {
      const url = 'http://www.example.com'
      if (process.platform === 'linux') {
        process.env.BROWSER = '/bin/true'
@@ -46,18 +46,6 @@ describe('shell module', () => {

      shell.openExternal(url).then(() => done())
    })

    it('opens an external link synchronously', () => {
      const url = 'http://www.example.com'
      if (process.platform === 'linux') {
        process.env.DE = 'generic'
        process.env.DE = '/bin/true'
        process.env.DISPLAY = ''
      }

      const success = shell.openExternalSync(url)
      assert.strictEqual(true, success)
    })
  })

  describe('shell.readShortcutLink(shortcutPath)', () => {

spec/ts-smoke/electron/main.ts

@@ -997,7 +997,7 @@ shell.moveItemToTrash('/home/user/Desktop/test.txt')

shell.openExternal('https://github.com', {
  activate: false
})
}).then(() => {})

shell.beep()

spec/ts-smoke/electron/renderer.ts

@@ -218,7 +218,7 @@ app.on('ready', () => {
// shell
// https://github.com/atom/electron/blob/master/docs/api/shell.md

shell.openExternal('https://github.com')
shell.openExternal('https://github.com').then(() => {})

// <webview>
// https://github.com/atom/electron/blob/master/docs/api/web-view-tag.md
@@ -238,8 +238,9 @@ webview.addEventListener('found-in-page', function (e) {

const requestId = webview.findInPage('test')

webview.addEventListener('new-window', function (e) {
  require('electron').shell.openExternal(e.url)
webview.addEventListener('new-window', async e => {
  const { shell } = require('electron')
  await shell.openExternal(e.url)
})

webview.addEventListener('close', function () {