fix: properly pass openExternal activate option (electron#18722)

jeremyspiegel · codebytere · commit 9ef83cd8a049 · 2019-06-11T10:23:48.000-07:00
A reference to an OpenExternalOptions structure was being captured by an Objective-C block that
outlived the object that was being referenced.
diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm
@@ -108,14 +108,15 @@ void OpenExternal(const GURL& url,
     return;
   }
 
+  bool activate = options.activate;
   __block OpenExternalCallback c = std::move(callback);
-  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(), ^{
-          std::move(c).Run(error);
-        });
-      });
+  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(), ^{
+                     std::move(c).Run(error);
+                   });
+                 });
 }
 
 bool MoveItemToTrash(const base::FilePath& full_path) {
diff --git a/spec/api-shell-spec.js b/spec/api-shell-spec.js
@@ -2,7 +2,10 @@ const assert = require('assert')
 const fs = require('fs')
 const path = require('path')
 const os = require('os')
-const { shell } = require('electron')
+const { shell, remote } = require('electron')
+const { BrowserWindow } = remote
+
+const { closeWindow } = require('./window-helpers')
 
 describe('shell module', () => {
   const fixtures = path.resolve(__dirname, 'fixtures')
@@ -18,6 +21,7 @@ describe('shell module', () => {
 
   describe('shell.openExternal()', () => {
     let envVars = {}
+    let w
 
     beforeEach(function () {
       envVars = {
@@ -27,7 +31,9 @@ describe('shell module', () => {
       }
     })
 
-    afterEach(() => {
+    afterEach(async () => {
+      await closeWindow(w)
+      w = null
       // reset env vars to prevent side effects
       if (process.platform === 'linux') {
         process.env.DE = envVars.de
@@ -44,7 +50,24 @@ describe('shell module', () => {
         process.env.DISPLAY = ''
       }
 
-      shell.openExternal(url).then(() => done())
+      // Ensure an external window is activated via a new window's blur event
+      w = new BrowserWindow()
+      let promiseResolved = false
+      let blurEventEmitted = false
+
+      w.on('blur', () => {
+        blurEventEmitted = true
+        if (promiseResolved) {
+          done()
+        }
+      })
+
+      shell.openExternal(url).then(() => {
+        promiseResolved = true
+        if (blurEventEmitted || process.platform === 'linux') {
+          done()
+        }
+      })
     })
 
     it('opens an external link synchronously', () => {
