fix: exceptions during function/promise result conversions live in calling world (#37922)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Samuel Attard <marshallofsound@electronjs.org>

docs/breaking-changes.md

@@ -61,6 +61,44 @@ protocol.handle('some-protocol', () => {
 })
 ```
 
+### Deprecated: `BrowserWindow.setTrafficLightPosition(position)`
+
+`BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the
+`BrowserWindow.setWindowButtonPosition(position)` API should be used instead
+which accepts `null` instead of `{ x: 0, y: 0 }` to reset the position to
+system default.
+
+```js
+// Deprecated in Electron 25
+win.setTrafficLightPosition({ x: 10, y: 10 })
+win.setTrafficLightPosition({ x: 0, y: 0 })
+
+// Replace with
+win.setWindowButtonPosition({ x: 10, y: 10 })
+win.setWindowButtonPosition(null)
+```
+
+### Deprecated: `BrowserWindow.getTrafficLightPosition()`
+
+`BrowserWindow.getTrafficLightPosition()` has been deprecated, the
+`BrowserWindow.getWindowButtonPosition()` API should be used instead
+which returns `null` instead of `{ x: 0, y: 0 }` when there is no custom
+position.
+
+```js
+// Deprecated in Electron 25
+const pos = win.getTrafficLightPosition()
+if (pos.x === 0 && pos.y === 0) {
+  // No custom position.
+}
+
+// Replace with
+const ret = win.getWindowButtonPosition()
+if (ret === null) {
+  // No custom position.
+}
+```
+
 ## Planned Breaking API Changes (24.0)
 
 ### API Changed: `nativeImage.createThumbnailFromPath(path, size)`
@@ -98,44 +136,6 @@ nativeImage.createThumbnailFromPath(imagePath, size).then(result => {
 })
 ```
 
-### Deprecated: `BrowserWindow.setTrafficLightPosition(position)`
-
-`BrowserWindow.setTrafficLightPosition(position)` has been deprecated, the
-`BrowserWindow.setWindowButtonPosition(position)` API should be used instead
-which accepts `null` instead of `{ x: 0, y: 0 }` to reset the position to
-system default.
-
-```js
-// Removed in Electron 24
-win.setTrafficLightPosition({ x: 10, y: 10 })
-win.setTrafficLightPosition({ x: 0, y: 0 })
-
-// Replace with
-win.setWindowButtonPosition({ x: 10, y: 10 })
-win.setWindowButtonPosition(null)
-```
-
-### Deprecated: `BrowserWindow.getTrafficLightPosition()`
-
-`BrowserWindow.getTrafficLightPosition()` has been deprecated, the
-`BrowserWindow.getWindowButtonPosition()` API should be used instead
-which returns `null` instead of `{ x: 0, y: 0 }` when there is no custom
-position.
-
-```js
-// Removed in Electron 24
-const pos = win.getTrafficLightPosition()
-if (pos.x === 0 && pos.y === 0) {
-  // No custom position.
-}
-
-// Replace with
-const ret = win.getWindowButtonPosition()
-if (ret === null) {
-  // No custom position.
-}
-```
-
 ## Planned Breaking API Changes (23.0)
 
 ### Behavior Changed: Draggable Regions on macOS

docs/tutorial/electron-timelines.md

@@ -9,11 +9,11 @@ check out our [Electron Versioning](./electron-versioning.md) doc.
 
 | Electron | Alpha | Beta | Stable | EOL | Chrome | Node | Supported |
 | ------- | ----- | ------- | ------ | ------ | ---- | ---- | ---- |
-| 25.0.0 | 2023-Apr-10 | 2023-May-02 | 2023-May-30 | TBD | M114 | TBD | TBD |
-| 24.0.0 | 2022-Feb-09 | 2023-Mar-07 | 2023-Apr-08 | TBD | M112 | TBD | ✅ |
-| 23.0.0 | 2022-Dec-01 | 2023-Jan-10 | 2023-Feb-07 | TBD | M110 | TBD | ✅ |
-| 22.0.0 | 2022-Sep-29 | 2022-Oct-25 | 2022-Nov-29 | TBD | M108 | v16.17 | ✅ |
-| 21.0.0 | 2022-Aug-04 | 2022-Aug-30 | 2022-Sep-27 | TBD | M106 | v16.16 | ✅ |
+| 25.0.0 | 2023-Apr-10 | 2023-May-02 | 2023-May-30 | 2023-Dec-05 | M114 | TBD | ✅ |
+| 24.0.0 | 2022-Feb-09 | 2023-Mar-07 | 2023-Apr-04 | 2023-Oct-03 | M112 | v18.14 | ✅ |
+| 23.0.0 | 2022-Dec-01 | 2023-Jan-10 | 2023-Feb-07 | 2023-Aug-08 | M110 | v18.12 | ✅ |
+| 22.0.0 | 2022-Sep-29 | 2022-Oct-25 | 2022-Nov-29 | 2023-May-30 | M108 | v16.17 | ✅ |
+| 21.0.0 | 2022-Aug-04 | 2022-Aug-30 | 2022-Sep-27 | 2023-Apr-04 | M106 | v16.16 | 🚫 |
 | 20.0.0 | 2022-May-26 | 2022-Jun-21 | 2022-Aug-02 | 2023-Feb-07 | M104 | v16.15 | 🚫 |
 | 19.0.0 | 2022-Mar-31 | 2022-Apr-26 | 2022-May-24 | 2022-Nov-29 | M102 | v16.14 | 🚫 |
 | 18.0.0 | 2022-Feb-03 | 2022-Mar-03 | 2022-Mar-29 | 2022-Sep-27 | M100 | v16.13 | 🚫 |

lib/browser/api/menu-item-roles.ts

@@ -26,7 +26,7 @@ export const roleList: Record<RoleId, Role> = {
     get label () {
       return isLinux ? 'About' : `About ${app.name}`;
     },
-    ...(isWindows && { appMethod: () => app.showAboutPanel() })
+    ...((isWindows || isLinux) && { appMethod: () => app.showAboutPanel() })
   },
   close: {
     label: isMac ? 'Close Window' : 'Close',

patches/chromium/.patches

@@ -127,3 +127,4 @@ chore_patch_out_profile_methods_in_profile_selections_cc.patch
 add_gin_converter_support_for_arraybufferview.patch
 chore_defer_usb_service_getdevices_request_until_usb_service_is.patch
 revert_roll_clang_rust_llvmorg-16-init-17653-g39da55e8-3.patch
+revert_x11_keep_windowcache_alive_for_a_time_interval.patch

patches/chromium/revert_x11_keep_windowcache_alive_for_a_time_interval.patch

@@ -0,0 +1,240 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: David Sanders <dsanders11@ucsbalum.com>
+Date: Sun, 2 Apr 2023 05:52:30 -0700
+Subject: Revert "[X11] Keep WindowCache alive for a time interval"
+
+This reverts commit bbc20a9c1b91ac6d6035408748091369cc96d4d7.
+
+While intended as a performance improvement, the commit breaks
+Views menus on X11 after certain window events such as resizing,
+or maximizing and unmaximizing.
+
+The patch can be removed once the upstream issue is fixed. That
+was reported in https://crbug.com/1429935.
+
+diff --git a/ui/base/x/x11_whole_screen_move_loop.cc b/ui/base/x/x11_whole_screen_move_loop.cc
+index 08e1c09749c5c39d99deec75c1a914c43936a6a5..ccd4785415a743a9519e750d5f0e334632058654 100644
+--- a/ui/base/x/x11_whole_screen_move_loop.cc
++++ b/ui/base/x/x11_whole_screen_move_loop.cc
+@@ -25,6 +25,7 @@
+ #include "ui/events/x/x11_event_translation.h"
+ #include "ui/gfx/x/connection.h"
+ #include "ui/gfx/x/keysyms/keysyms.h"
++#include "ui/gfx/x/window_cache.h"
+ #include "ui/gfx/x/x11_window_event_manager.h"
+ #include "ui/gfx/x/xproto.h"
+ 
+@@ -150,6 +151,10 @@ bool X11WholeScreenMoveLoop::RunMoveLoop(
+   auto* connection = x11::Connection::Get();
+   CreateDragInputWindow(connection);
+ 
++  // Keep a window cache alive for the duration of the drag so that the drop
++  // target under the drag window can be quickly determined.
++  x11::WindowCache cache(connection, connection->default_root(), true);
++
+   // Only grab mouse capture of |grab_input_window_| if |can_grab_pointer| is
+   // true aka the source that initiated the move loop doesn't have explicit
+   // grab.
+diff --git a/ui/gfx/x/window_cache.cc b/ui/gfx/x/window_cache.cc
+index 9c603366a657954b4be44d0a30fcf428265f95e7..03885b55354c37e04bc016b509ac2ae8bd6191c2 100644
+--- a/ui/gfx/x/window_cache.cc
++++ b/ui/gfx/x/window_cache.cc
+@@ -11,8 +11,6 @@
+ #include "base/notreached.h"
+ #include "base/ranges/algorithm.h"
+ #include "base/run_loop.h"
+-#include "base/task/single_thread_task_runner.h"
+-#include "base/time/time.h"
+ #include "ui/gfx/geometry/insets.h"
+ #include "ui/gfx/geometry/rect.h"
+ #include "ui/gfx/geometry/vector2d.h"
+@@ -24,23 +22,19 @@
+ 
+ namespace x11 {
+ 
+-const base::TimeDelta kDestroyTimerInterval = base::Seconds(3);
+-
+ Window GetWindowAtPoint(const gfx::Point& point_px,
+                         const base::flat_set<Window>* ignore) {
+   auto* connection = Connection::Get();
+   Window root = connection->default_root();
+ 
+-  if (!WindowCache::instance()) {
+-    auto instance =
+-        std::make_unique<WindowCache>(connection, connection->default_root());
+-    auto* cache = instance.get();
+-    cache->BeginDestroyTimer(std::move(instance));
++  if (auto* instance = WindowCache::instance()) {
++    instance->WaitUntilReady();
++    return instance->GetWindowAtPoint(point_px, root, ignore);
+   }
+ 
+-  auto* instance = WindowCache::instance();
+-  instance->WaitUntilReady();
+-  return instance->GetWindowAtPoint(point_px, root, ignore);
++  WindowCache cache(connection, connection->default_root(), false);
++  cache.WaitUntilReady();
++  return cache.GetWindowAtPoint(point_px, root, ignore);
+ }
+ 
+ ScopedShapeEventSelector::ScopedShapeEventSelector(Connection* connection,
+@@ -62,21 +56,24 @@ WindowCache::WindowInfo::~WindowInfo() = default;
+ // static
+ WindowCache* WindowCache::instance_ = nullptr;
+ 
+-WindowCache::WindowCache(Connection* connection, Window root)
++WindowCache::WindowCache(Connection* connection, Window root, bool track_events)
+     : connection_(connection),
+       root_(root),
++      track_events_(track_events),
+       gtk_frame_extents_(GetAtom("_GTK_FRAME_EXTENTS")) {
+   DCHECK(!instance_) << "Only one WindowCache should be active at a time";
+   instance_ = this;
+ 
+   connection_->AddEventObserver(this);
+ 
+-  // We select for SubstructureNotify events on all windows (to receive
+-  // CreateNotify events), which will cause events to be sent for all child
+-  // windows.  This means we need to additionally select for StructureNotify
+-  // changes for the root window.
+-  root_events_ =
+-      std::make_unique<XScopedEventSelector>(root_, EventMask::StructureNotify);
++  if (track_events_) {
++    // We select for SubstructureNotify events on all windows (to receive
++    // CreateNotify events), which will cause events to be sent for all child
++    // windows.  This means we need to additionally select for StructureNotify
++    // changes for the root window.
++    root_events_ = std::make_unique<XScopedEventSelector>(
++        root_, EventMask::StructureNotify);
++  }
+   AddWindow(root_, Window::None);
+ }
+ 
+@@ -106,16 +103,6 @@ void WindowCache::WaitUntilReady() {
+     last_processed_event_ = events[event - 1].sequence();
+ }
+ 
+-void WindowCache::BeginDestroyTimer(std::unique_ptr<WindowCache> self) {
+-  DCHECK_EQ(this, self.get());
+-  delete_when_destroy_timer_fires_ = false;
+-  base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask(
+-      FROM_HERE,
+-      base::BindOnce(&WindowCache::OnDestroyTimerExpired,
+-                     base::Unretained(this), std::move(self)),
+-      kDestroyTimerInterval);
+-}
+-
+ void WindowCache::SyncForTest() {
+   do {
+     // Perform a blocking sync to prevent spinning while waiting for replies.
+@@ -127,7 +114,6 @@ void WindowCache::SyncForTest() {
+ Window WindowCache::GetWindowAtPoint(gfx::Point point_px,
+                                      Window window,
+                                      const base::flat_set<Window>* ignore) {
+-  delete_when_destroy_timer_fires_ = true;
+   if (ignore && ignore->contains(window))
+     return Window::None;
+   auto* info = GetInfo(window);
+@@ -265,10 +251,12 @@ void WindowCache::AddWindow(Window window, Window parent) {
+     return;
+   WindowInfo& info = windows_[window];
+   info.parent = parent;
+-  // Events must be selected before getting the initial window info to
+-  // prevent race conditions.
+-  info.events = std::make_unique<XScopedEventSelector>(
+-      window, EventMask::SubstructureNotify | EventMask::PropertyChange);
++  if (track_events_) {
++    // Events must be selected before getting the initial window info to
++    // prevent race conditions.
++    info.events = std::make_unique<XScopedEventSelector>(
++        window, EventMask::SubstructureNotify | EventMask::PropertyChange);
++  }
+ 
+   AddRequest(connection_->GetWindowAttributes(window),
+              &WindowCache::OnGetWindowAttributesResponse, window);
+@@ -282,8 +270,10 @@ void WindowCache::AddWindow(Window window, Window parent) {
+ 
+   auto& shape = connection_->shape();
+   if (shape.present()) {
+-    info.shape_events =
+-        std::make_unique<ScopedShapeEventSelector>(connection_, window);
++    if (track_events_) {
++      info.shape_events =
++          std::make_unique<ScopedShapeEventSelector>(connection_, window);
++    }
+ 
+     for (auto kind : {Shape::Sk::Bounding, Shape::Sk::Input}) {
+       AddRequest(shape.GetRectangles(window, kind),
+@@ -391,11 +381,4 @@ void WindowCache::OnGetRectanglesResponse(
+   }
+ }
+ 
+-void WindowCache::OnDestroyTimerExpired(std::unique_ptr<WindowCache> self) {
+-  if (!delete_when_destroy_timer_fires_)
+-    return;  // destroy `this`
+-
+-  BeginDestroyTimer(std::move(self));
+-}
+-
+ }  // namespace x11
+diff --git a/ui/gfx/x/window_cache.h b/ui/gfx/x/window_cache.h
+index f241d6c23855fad478813ff3029fa6a17d084d34..ebc05d311ed3719be98180086baae8230ec9c58e 100644
+--- a/ui/gfx/x/window_cache.h
++++ b/ui/gfx/x/window_cache.h
+@@ -78,7 +78,7 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
+   // If `track_events` is true, the WindowCache will keep the cache state synced
+   // with the server's state over time. It may be set to false if the cache is
+   // short-lived, if only a single GetWindowAtPoint call is made.
+-  WindowCache(Connection* connection, Window root);
++  WindowCache(Connection* connection, Window root, bool track_events);
+   WindowCache(const WindowCache&) = delete;
+   WindowCache& operator=(const WindowCache&) = delete;
+   ~WindowCache() override;
+@@ -92,10 +92,6 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
+   // Blocks until all outstanding requests are processed.
+   void WaitUntilReady();
+ 
+-  // Destroys |self| if no calls to GetWindowAtPoint() are made within
+-  // a time window.
+-  void BeginDestroyTimer(std::unique_ptr<WindowCache> self);
+-
+   void SyncForTest();
+ 
+   const std::unordered_map<Window, WindowInfo>& windows() const {
+@@ -147,12 +143,11 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
+                                Shape::Sk kind,
+                                Shape::GetRectanglesResponse response);
+ 
+-  void OnDestroyTimerExpired(std::unique_ptr<WindowCache> self);
+-
+   static WindowCache* instance_;
+ 
+   const raw_ptr<Connection> connection_;
+   const Window root_;
++  const bool track_events_;
+   const Atom gtk_frame_extents_;
+   std::unique_ptr<XScopedEventSelector> root_events_;
+ 
+@@ -164,9 +159,6 @@ class COMPONENT_EXPORT(X11) WindowCache : public EventObserver {
+   // processed in order.
+   absl::optional<uint32_t> last_processed_event_;
+ 
+-  // True iff GetWindowAtPoint() was called since the last timer interval.
+-  bool delete_when_destroy_timer_fires_ = false;
+-
+   // Although only one instance of WindowCache may be created at a time, the
+   // instance will be created and destroyed as needed, so WeakPtrs are still
+   // necessary.
+diff --git a/ui/gfx/x/window_cache_unittest.cc b/ui/gfx/x/window_cache_unittest.cc
+index 2199ddac2577a33ff7a42f3d3752613cef00dd32..af0a2d3737c132b596096514b5ca4f572d6c9d64 100644
+--- a/ui/gfx/x/window_cache_unittest.cc
++++ b/ui/gfx/x/window_cache_unittest.cc
+@@ -21,7 +21,7 @@ class WindowCacheTest : public testing::Test {
+  protected:
+   void ResetCache() {
+     cache_.reset();
+-    cache_ = std::make_unique<WindowCache>(connection_, root_);
++    cache_ = std::make_unique<WindowCache>(connection_, root_, true);
+     cache_->SyncForTest();
+   }
+ 

script/spec-runner.js

@@ -64,10 +64,24 @@ if (args.runners !== undefined) {
 async function main () {
   if (args.electronVersion) {
     const versions = await ElectronVersions.create();
-    if (!versions.isVersion(args.electronVersion)) {
+    if (args.electronVersion === 'latest') {
+      args.electronVersion = versions.latest.version;
+    } else if (args.electronVersion.startsWith('latest@')) {
+      const majorVersion = parseInt(args.electronVersion.slice('latest@'.length));
+      const ver = versions.inMajor(majorVersion).slice(-1)[0];
+      if (ver) {
+        args.electronVersion = ver.version;
+      } else {
+        console.log(`${fail} '${majorVersion}' is not a recognized Electron major version`);
+        process.exit(1);
+      }
+    } else if (!versions.isVersion(args.electronVersion)) {
       console.log(`${fail} '${args.electronVersion}' is not a recognized Electron version`);
       process.exit(1);
     }
+
+    const versionString = `v${args.electronVersion}`;
+    console.log(`Running against Electron ${versionString.green}`);
   }
 
   const [lastSpecHash, lastSpecInstallHash] = loadLastSpecHash();

shell/browser/api/electron_api_push_notifications.cc

@@ -10,9 +10,7 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
 
-namespace electron {
-
-namespace api {
+namespace electron::api {
 
 PushNotifications* g_push_notifications = nullptr;
 
@@ -55,9 +53,7 @@ const char* PushNotifications::GetTypeName() {
   return "PushNotifications";
 }
 
-}  // namespace api
-
-}  // namespace electron
+}  // namespace electron::api
 
 namespace {
 

shell/browser/api/electron_api_push_notifications.h

@@ -15,9 +15,7 @@
 #include "shell/browser/event_emitter_mixin.h"
 #include "shell/common/gin_helper/promise.h"
 
-namespace electron {
-
-namespace api {
+namespace electron::api {
 
 class PushNotifications
     : public ElectronBrowserClient::Delegate,
@@ -57,8 +55,6 @@ class PushNotifications
 #endif
 };
 
-}  // namespace api
-
-}  // namespace electron
+}  // namespace electron::api
 
 #endif  // ELECTRON_SHELL_BROWSER_API_ELECTRON_API_PUSH_NOTIFICATIONS_H_

shell/browser/api/electron_api_push_notifications_mac.mm

@@ -12,9 +12,7 @@
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/promise.h"
 
-namespace electron {
-
-namespace api {
+namespace electron::api {
 
 v8::Local<v8::Promise> PushNotifications::RegisterForAPNSNotifications(
     v8::Isolate* isolate) {
@@ -57,6 +55,4 @@ void PushNotifications::OnDidReceiveAPNSNotification(
   Emit("received-apns-notification", user_info);
 }
 
-}  // namespace api
-
-}  // namespace electron
+}  // namespace electron::api

shell/browser/api/electron_api_service_worker_context.cc

@@ -115,10 +115,10 @@ v8::Local<v8::Value> ServiceWorkerContext::GetAllRunningWorkerInfo(
   gin::DataObjectBuilder builder(isolate);
   const base::flat_map<int64_t, content::ServiceWorkerRunningInfo>& info_map =
       service_worker_context_->GetRunningServiceWorkerInfos();
-  for (auto iter = info_map.begin(); iter != info_map.end(); ++iter) {
+  for (const auto& iter : info_map) {
     builder.Set(
-        std::to_string(iter->first),
-        ServiceWorkerRunningInfoToDict(isolate, std::move(iter->second)));
+        std::to_string(iter.first),
+        ServiceWorkerRunningInfoToDict(isolate, std::move(iter.second)));
   }
   return builder.Build();
 }

shell/browser/api/electron_api_utility_process.cc

@@ -106,10 +106,10 @@ UtilityProcessWrapper::UtilityProcessWrapper(
         return;
       }
       if (io_handle == IOHandle::STDOUT) {
-        fds_to_remap.push_back(std::make_pair(pipe_fd[1], STDOUT_FILENO));
+        fds_to_remap.emplace_back(pipe_fd[1], STDOUT_FILENO);
         stdout_read_fd_ = pipe_fd[0];
       } else if (io_handle == IOHandle::STDERR) {
-        fds_to_remap.push_back(std::make_pair(pipe_fd[1], STDERR_FILENO));
+        fds_to_remap.emplace_back(pipe_fd[1], STDERR_FILENO);
         stderr_read_fd_ = pipe_fd[0];
       }
 #endif
@@ -135,9 +135,9 @@ UtilityProcessWrapper::UtilityProcessWrapper(
         return;
       }
       if (io_handle == IOHandle::STDOUT) {
-        fds_to_remap.push_back(std::make_pair(devnull, STDOUT_FILENO));
+        fds_to_remap.emplace_back(devnull, STDOUT_FILENO);
       } else if (io_handle == IOHandle::STDERR) {
-        fds_to_remap.push_back(std::make_pair(devnull, STDERR_FILENO));
+        fds_to_remap.emplace_back(devnull, STDERR_FILENO);
       }
 #endif
     }

shell/browser/api/electron_api_utility_process.h

@@ -33,9 +33,7 @@ namespace base {
 class Process;
 }  // namespace base
 
-namespace electron {
-
-namespace api {
+namespace electron::api {
 
 class UtilityProcessWrapper
     : public gin::Wrappable<UtilityProcessWrapper>,
@@ -93,8 +91,6 @@ class UtilityProcessWrapper
   base::WeakPtrFactory<UtilityProcessWrapper> weak_factory_{this};
 };
 
-}  // namespace api
-
-}  // namespace electron
+}  // namespace electron::api
 
 #endif  // ELECTRON_SHELL_BROWSER_API_ELECTRON_API_UTILITY_PROCESS_H_

shell/browser/api/message_port.cc

@@ -240,8 +240,8 @@ std::vector<blink::MessagePortChannel> MessagePort::DisentanglePorts(
   // Passed-in ports passed validity checks, so we can disentangle them.
   std::vector<blink::MessagePortChannel> channels;
   channels.reserve(ports.size());
-  for (unsigned i = 0; i < ports.size(); ++i)
-    channels.push_back(ports[i]->Disentangle());
+  for (auto port : ports)
+    channels.push_back(port->Disentangle());
   return channels;
 }
 

shell/browser/extensions/api/resources_private/resources_private_api.cc

@@ -9,6 +9,7 @@
 
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/pdf/pdf_extension_util.h"
 #include "chrome/common/extensions/api/resources_private.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
@@ -30,45 +31,6 @@
 
 namespace extensions {
 
-namespace {
-
-void AddStringsForPdf(base::Value::Dict* dict) {
-#if BUILDFLAG(ENABLE_PDF)
-  static constexpr webui::LocalizedString kPdfResources[] = {
-      {"passwordDialogTitle", IDS_PDF_PASSWORD_DIALOG_TITLE},
-      {"passwordPrompt", IDS_PDF_NEED_PASSWORD},
-      {"passwordSubmit", IDS_PDF_PASSWORD_SUBMIT},
-      {"thumbnailPageAriaLabel", IDS_PDF_THUMBNAIL_PAGE_ARIA_LABEL},
-      {"passwordInvalid", IDS_PDF_PASSWORD_INVALID},
-      {"pageLoading", IDS_PDF_PAGE_LOADING},
-      {"pageLoadFailed", IDS_PDF_PAGE_LOAD_FAILED},
-      {"errorDialogTitle", IDS_PDF_ERROR_DIALOG_TITLE},
-      {"pageReload", IDS_PDF_PAGE_RELOAD_BUTTON},
-      {"bookmarks", IDS_PDF_BOOKMARKS},
-      {"labelPageNumber", IDS_PDF_LABEL_PAGE_NUMBER},
-      {"tooltipDownload", IDS_PDF_TOOLTIP_DOWNLOAD},
-      {"tooltipPrint", IDS_PDF_TOOLTIP_PRINT},
-      {"tooltipFitToPage", IDS_PDF_TOOLTIP_FIT_PAGE},
-      {"tooltipFitToWidth", IDS_PDF_TOOLTIP_FIT_WIDTH},
-      {"tooltipZoomIn", IDS_PDF_TOOLTIP_ZOOM_IN},
-      {"tooltipZoomOut", IDS_PDF_TOOLTIP_ZOOM_OUT},
-  };
-  for (const auto& resource : kPdfResources)
-    dict->Set(resource.name, l10n_util::GetStringUTF16(resource.id));
-
-  dict->Set("presetZoomFactors", zoom::GetPresetZoomFactorsAsJSON());
-#endif  // BUILDFLAG(ENABLE_PDF)
-}
-
-void AddAdditionalDataForPdf(base::Value::Dict* dict) {
-#if BUILDFLAG(ENABLE_PDF)
-  dict->Set("pdfAnnotationsEnabled", base::Value(false));
-  dict->Set("printingEnabled", base::Value(true));
-#endif  // BUILDFLAG(ENABLE_PDF)
-}
-
-}  // namespace
-
 namespace get_strings = api::resources_private::GetStrings;
 
 ResourcesPrivateGetStringsFunction::ResourcesPrivateGetStringsFunction() =
@@ -86,8 +48,11 @@ ExtensionFunction::ResponseAction ResourcesPrivateGetStringsFunction::Run() {
 
   switch (component) {
     case api::resources_private::COMPONENT_PDF:
-      AddStringsForPdf(&dict);
-      AddAdditionalDataForPdf(&dict);
+#if BUILDFLAG(ENABLE_PDF)
+      pdf_extension_util::AddStrings(
+          pdf_extension_util::PdfViewerContext::kPdfViewer, &dict);
+      pdf_extension_util::AddAdditionalData(true, false, &dict);
+#endif
       break;
     case api::resources_private::COMPONENT_IDENTITY:
       NOTREACHED();

shell/browser/relauncher.cc

@@ -99,8 +99,8 @@ bool RelaunchAppWithHelper(const base::FilePath& helper,
 
   base::LaunchOptions options;
 #if BUILDFLAG(IS_POSIX)
-  options.fds_to_remap.push_back(
-      std::make_pair(pipe_write_fd.get(), internal::kRelauncherSyncFD));
+  options.fds_to_remap.emplace_back(pipe_write_fd.get(),
+                                    internal::kRelauncherSyncFD);
   base::Process process = base::LaunchProcess(relaunch_argv, options);
 #elif BUILDFLAG(IS_WIN)
   base::Process process = base::LaunchProcess(

shell/browser/relauncher_mac.cc

@@ -82,8 +82,8 @@ int LaunchProgram(const StringVector& relauncher_args,
 
   base::LaunchOptions options;
   options.new_process_group = true;  // detach
-  options.fds_to_remap.push_back(std::make_pair(devnull.get(), STDERR_FILENO));
-  options.fds_to_remap.push_back(std::make_pair(devnull.get(), STDOUT_FILENO));
+  options.fds_to_remap.emplace_back(devnull.get(), STDERR_FILENO);
+  options.fds_to_remap.emplace_back(devnull.get(), STDOUT_FILENO);
 
   base::Process process = base::LaunchProcess(argv, options);
   return process.IsValid() ? 0 : 1;

shell/browser/ui/gtk/menu_gtk.cc

@@ -9,9 +9,7 @@
 #include "shell/browser/ui/gtk/menu_util.h"
 #include "ui/base/models/menu_model.h"
 
-namespace electron {
-
-namespace gtkui {
+namespace electron::gtkui {
 
 MenuGtk::MenuGtk(ui::MenuModel* model)
     : menu_model_(model), gtk_menu_(TakeGObject(gtk_menu_new())) {
@@ -65,6 +63,4 @@ void MenuGtk::OnMenuItemActivated(GtkWidget* menu_item) {
     ExecuteCommand(model, id);
 }
 
-}  // namespace gtkui
-
-}  // namespace electron
+}  // namespace electron::gtkui

shell/browser/ui/gtk/menu_gtk.h

@@ -17,9 +17,7 @@ namespace ui {
 class MenuModel;
 }
 
-namespace electron {
-
-namespace gtkui {
+namespace electron::gtkui {
 
 class MenuGtk {
  public:
@@ -41,8 +39,6 @@ class MenuGtk {
   bool block_activation_ = false;
 };
 
-}  // namespace gtkui
-
-}  // namespace electron
+}  // namespace electron::gtkui
 
 #endif  // ELECTRON_SHELL_BROWSER_UI_GTK_MENU_GTK_H_

shell/common/gin_converters/net_converter.cc

@@ -578,8 +578,7 @@ bool Converter<scoped_refptr<network::ResourceRequestBody>>::FromV8(
     return false;
   base::Value::List& list = list_value.GetList();
   *out = base::MakeRefCounted<network::ResourceRequestBody>();
-  for (size_t i = 0; i < list.size(); ++i) {
-    base::Value& dict_value = list[i];
+  for (base::Value& dict_value : list) {
     if (!dict_value.is_dict())
       return false;
     base::Value::Dict& dict = dict_value.GetDict();

shell/renderer/api/context_bridge/object_cache.cc

@@ -19,7 +19,7 @@ void ObjectCache::CacheProxiedObject(v8::Local<v8::Value> from,
     auto obj = from.As<v8::Object>();
     int hash = obj->GetIdentityHash();
 
-    proxy_map_[hash].push_front(std::make_pair(from, proxy_value));
+    proxy_map_[hash].emplace_front(from, proxy_value);
   }
 }
 

shell/renderer/api/electron_api_context_bridge.cc

@@ -241,10 +241,29 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
               global_destination_context.IsEmpty())
             return;
           context_bridge::ObjectCache object_cache;
-          auto val =
-              PassValueToOtherContext(global_source_context.Get(isolate),
-                                      global_destination_context.Get(isolate),
-                                      result, &object_cache, false, 0);
+          v8::MaybeLocal<v8::Value> val;
+          {
+            v8::TryCatch try_catch(isolate);
+            val = PassValueToOtherContext(
+                global_source_context.Get(isolate),
+                global_destination_context.Get(isolate), result, &object_cache,
+                false, 0, BridgeErrorTarget::kDestination);
+            if (try_catch.HasCaught()) {
+              if (try_catch.Message().IsEmpty()) {
+                proxied_promise->RejectWithErrorMessage(
+                    "An error was thrown while sending a promise result over "
+                    "the context bridge but it was not actually an Error "
+                    "object. This normally means that a promise was resolved "
+                    "with a value that is not supported by the Context "
+                    "Bridge.");
+              } else {
+                proxied_promise->Reject(
+                    v8::Exception::Error(try_catch.Message()->Get()));
+              }
+              return;
+            }
+          }
+          DCHECK(!val.IsEmpty());
           if (!val.IsEmpty())
             proxied_promise->Resolve(val.ToLocalChecked());
         },
@@ -268,10 +287,28 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
               global_destination_context.IsEmpty())
             return;
           context_bridge::ObjectCache object_cache;
-          auto val =
-              PassValueToOtherContext(global_source_context.Get(isolate),
-                                      global_destination_context.Get(isolate),
-                                      result, &object_cache, false, 0);
+          v8::MaybeLocal<v8::Value> val;
+          {
+            v8::TryCatch try_catch(isolate);
+            val = PassValueToOtherContext(
+                global_source_context.Get(isolate),
+                global_destination_context.Get(isolate), result, &object_cache,
+                false, 0, BridgeErrorTarget::kDestination);
+            if (try_catch.HasCaught()) {
+              if (try_catch.Message().IsEmpty()) {
+                proxied_promise->RejectWithErrorMessage(
+                    "An error was thrown while sending a promise rejection "
+                    "over the context bridge but it was not actually an Error "
+                    "object. This normally means that a promise was rejected "
+                    "with a value that is not supported by the Context "
+                    "Bridge.");
+              } else {
+                proxied_promise->Reject(
+                    v8::Exception::Error(try_catch.Message()->Get()));
+              }
+              return;
+            }
+          }
           if (!val.IsEmpty())
             proxied_promise->Reject(val.ToLocalChecked());
         },
@@ -324,7 +361,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
       auto value_for_array = PassValueToOtherContext(
           source_context, destination_context,
           arr->Get(source_context, i).ToLocalChecked(), object_cache,
-          support_dynamic_properties, recursion_depth + 1);
+          support_dynamic_properties, recursion_depth + 1, error_target);
       if (value_for_array.IsEmpty())
         return v8::MaybeLocal<v8::Value>();
 
@@ -358,7 +395,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     auto object_value = value.As<v8::Object>();
     auto passed_value = CreateProxyForAPI(
         object_value, source_context, destination_context, object_cache,
-        support_dynamic_properties, recursion_depth + 1);
+        support_dynamic_properties, recursion_depth + 1, error_target);
     if (passed_value.IsEmpty())
       return v8::MaybeLocal<v8::Value>();
     return v8::MaybeLocal<v8::Value>(passed_value.ToLocalChecked());
@@ -372,8 +409,9 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
                                                    : destination_context;
     v8::Context::Scope error_scope(error_context);
     // V8 serializer will throw an error if required
-    if (!gin::ConvertFromV8(error_context->GetIsolate(), value, &ret))
+    if (!gin::ConvertFromV8(error_context->GetIsolate(), value, &ret)) {
       return v8::MaybeLocal<v8::Value>();
+    }
   }
 
   {
@@ -420,9 +458,9 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
     args.GetRemaining(&original_args);
 
     for (auto value : original_args) {
-      auto arg =
-          PassValueToOtherContext(calling_context, func_owning_context, value,
-                                  &object_cache, support_dynamic_properties, 0);
+      auto arg = PassValueToOtherContext(
+          calling_context, func_owning_context, value, &object_cache,
+          support_dynamic_properties, 0, BridgeErrorTarget::kSource);
       if (arg.IsEmpty())
         return;
       proxied_args.push_back(arg.ToLocalChecked());
@@ -469,10 +507,50 @@ void ProxyFunctionWrapper(const v8::FunctionCallbackInfo<v8::Value>& info) {
     if (maybe_return_value.IsEmpty())
       return;
 
-    auto ret = PassValueToOtherContext(
-        func_owning_context, calling_context,
-        maybe_return_value.ToLocalChecked(), &object_cache,
-        support_dynamic_properties, 0, BridgeErrorTarget::kDestination);
+    // In the case where we encounted an exception converting the return value
+    // of the function we need to ensure that the exception / thrown value is
+    // safely transferred from the function_owning_context (where it was thrown)
+    // into the calling_context (where it needs to be thrown) To do this we pull
+    // the message off the exception and later re-throw it in the right context.
+    // In some cases the caught thing is not an exception i.e. it's technically
+    // valid to `throw 123`.  In these cases to avoid infinite
+    // PassValueToOtherContext recursion we bail early as being unable to send
+    // the value from one context to the other.
+    // TODO(MarshallOfSound): In this case and other cases where the error can't
+    // be sent _across_ worlds we should probably log it globally in some way to
+    // allow easier debugging.  This is not trivial though so is left to a
+    // future change.
+    bool did_error_converting_result = false;
+    v8::MaybeLocal<v8::Value> ret;
+    v8::Local<v8::String> exception;
+    {
+      v8::TryCatch try_catch(args.isolate());
+      ret = PassValueToOtherContext(func_owning_context, calling_context,
+                                    maybe_return_value.ToLocalChecked(),
+                                    &object_cache, support_dynamic_properties,
+                                    0, BridgeErrorTarget::kDestination);
+      if (try_catch.HasCaught()) {
+        did_error_converting_result = true;
+        if (!try_catch.Message().IsEmpty()) {
+          exception = try_catch.Message()->Get();
+        }
+      }
+    }
+    if (did_error_converting_result) {
+      v8::Context::Scope calling_context_scope(calling_context);
+      if (exception.IsEmpty()) {
+        const char err_msg[] =
+            "An unknown exception occurred while sending a function return "
+            "value over the context bridge, an error "
+            "occurred but a valid exception was not thrown.";
+        args.isolate()->ThrowException(v8::Exception::Error(
+            gin::StringToV8(args.isolate(), err_msg).As<v8::String>()));
+      } else {
+        args.isolate()->ThrowException(v8::Exception::Error(exception));
+      }
+      return;
+    }
+    DCHECK(!ret.IsEmpty());
     if (ret.IsEmpty())
       return;
     info.GetReturnValue().Set(ret.ToLocalChecked());
@@ -485,7 +563,8 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Context>& destination_context,
     context_bridge::ObjectCache* object_cache,
     bool support_dynamic_properties,
-    int recursion_depth) {
+    int recursion_depth,
+    BridgeErrorTarget error_target) {
   gin_helper::Dictionary api(source_context->GetIsolate(), api_object);
 
   {
@@ -526,14 +605,16 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
             if (!getter.IsEmpty()) {
               if (!PassValueToOtherContext(source_context, destination_context,
                                            getter, object_cache,
-                                           support_dynamic_properties, 1)
+                                           support_dynamic_properties, 1,
+                                           error_target)
                        .ToLocal(&getter_proxy))
                 continue;
             }
             if (!setter.IsEmpty()) {
               if (!PassValueToOtherContext(source_context, destination_context,
                                            setter, object_cache,
-                                           support_dynamic_properties, 1)
+                                           support_dynamic_properties, 1,
+                                           error_target)
                        .ToLocal(&setter_proxy))
                 continue;
             }
@@ -551,7 +632,7 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
 
       auto passed_value = PassValueToOtherContext(
           source_context, destination_context, value, object_cache,
-          support_dynamic_properties, recursion_depth + 1);
+          support_dynamic_properties, recursion_depth + 1, error_target);
       if (passed_value.IsEmpty())
         return v8::MaybeLocal<v8::Object>();
       proxy.Set(key, passed_value.ToLocalChecked());
@@ -597,9 +678,9 @@ void ExposeAPIInWorld(v8::Isolate* isolate,
     context_bridge::ObjectCache object_cache;
     v8::Context::Scope target_context_scope(target_context);
 
-    v8::MaybeLocal<v8::Value> maybe_proxy =
-        PassValueToOtherContext(electron_isolated_context, target_context, api,
-                                &object_cache, false, 0);
+    v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
+        electron_isolated_context, target_context, api, &object_cache, false, 0,
+        BridgeErrorTarget::kSource);
     if (maybe_proxy.IsEmpty())
       return;
     auto proxy = maybe_proxy.ToLocalChecked();
@@ -649,7 +730,7 @@ void OverrideGlobalValueFromIsolatedWorld(
     context_bridge::ObjectCache object_cache;
     v8::MaybeLocal<v8::Value> maybe_proxy = PassValueToOtherContext(
         value->GetCreationContextChecked(), main_context, value, &object_cache,
-        support_dynamic_properties, 1);
+        support_dynamic_properties, 1, BridgeErrorTarget::kSource);
     DCHECK(!maybe_proxy.IsEmpty());
     auto proxy = maybe_proxy.ToLocalChecked();
 
@@ -685,14 +766,14 @@ bool OverrideGlobalPropertyFromIsolatedWorld(
     if (!getter->IsNullOrUndefined()) {
       v8::MaybeLocal<v8::Value> maybe_getter_proxy = PassValueToOtherContext(
           getter->GetCreationContextChecked(), main_context, getter,
-          &object_cache, false, 1);
+          &object_cache, false, 1, BridgeErrorTarget::kSource);
       DCHECK(!maybe_getter_proxy.IsEmpty());
       getter_proxy = maybe_getter_proxy.ToLocalChecked();
     }
     if (!setter->IsNullOrUndefined() && setter->IsObject()) {
       v8::MaybeLocal<v8::Value> maybe_setter_proxy = PassValueToOtherContext(
           getter->GetCreationContextChecked(), main_context, setter,
-          &object_cache, false, 1);
+          &object_cache, false, 1, BridgeErrorTarget::kSource);
       DCHECK(!maybe_setter_proxy.IsEmpty());
       setter_proxy = maybe_setter_proxy.ToLocalChecked();
     }

shell/renderer/api/electron_api_context_bridge.h

@@ -39,7 +39,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     context_bridge::ObjectCache* object_cache,
     bool support_dynamic_properties,
     int recursion_depth,
-    BridgeErrorTarget error_target = BridgeErrorTarget::kSource);
+    BridgeErrorTarget error_target);
 
 v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Object>& api_object,
@@ -47,7 +47,8 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Context>& destination_context,
     context_bridge::ObjectCache* object_cache,
     bool support_dynamic_properties,
-    int recursion_depth);
+    int recursion_depth,
+    BridgeErrorTarget error_target);
 
 }  // namespace electron::api
 

shell/renderer/api/electron_api_web_frame.cc

@@ -142,7 +142,7 @@ class ScriptExecutionCallback {
       context_bridge::ObjectCache object_cache;
       maybe_result = PassValueToOtherContext(
           result->GetCreationContextChecked(), promise_.GetContext(), result,
-          &object_cache, false, 0);
+          &object_cache, false, 0, BridgeErrorTarget::kSource);
       if (maybe_result.IsEmpty() || try_catch.HasCaught()) {
         success = false;
       }

shell/renderer/renderer_client_base.cc

@@ -608,7 +608,8 @@ void RendererClientBase::SetupMainWorldOverrides(
   if (global.GetHidden("guestViewInternal", &guest_view_internal)) {
     api::context_bridge::ObjectCache object_cache;
     auto result = api::PassValueToOtherContext(
-        source_context, context, guest_view_internal, &object_cache, false, 0);
+        source_context, context, guest_view_internal, &object_cache, false, 0,
+        api::BridgeErrorTarget::kSource);
     if (!result.IsEmpty()) {
       isolated_api.Set("guestViewInternal", result.ToLocalChecked());
     }

spec/api-context-bridge-spec.ts

@@ -806,10 +806,29 @@ describe('contextBridge', () => {
             throwNotClonable: () => {
               return Object(Symbol('foo'));
             },
-            argumentConvert: () => {}
+            throwNotClonableNestedArray: () => {
+              return [Object(Symbol('foo'))];
+            },
+            throwNotClonableNestedObject: () => {
+              return {
+                bad: Object(Symbol('foo'))
+              };
+            },
+            throwDynamic: () => {
+              return {
+                get bad () {
+                  throw new Error('damm');
+                }
+              };
+            },
+            argumentConvert: () => {},
+            rejectNotClonable: async () => {
+              throw Object(Symbol('foo'));
+            },
+            resolveNotClonable: async () => Object(Symbol('foo'))
           });
         });
-        const result = await callWithBindings((root: any) => {
+        const result = await callWithBindings(async (root: any) => {
           const getError = (fn: Function) => {
             try {
               fn();
@@ -818,13 +837,26 @@ describe('contextBridge', () => {
             }
             return null;
           };
+          const getAsyncError = async (fn: Function) => {
+            try {
+              await fn();
+            } catch (e) {
+              return e;
+            }
+            return null;
+          };
           const normalIsError = Object.getPrototypeOf(getError(root.example.throwNormal)) === Error.prototype;
           const weirdIsError = Object.getPrototypeOf(getError(root.example.throwWeird)) === Error.prototype;
           const notClonableIsError = Object.getPrototypeOf(getError(root.example.throwNotClonable)) === Error.prototype;
+          const notClonableNestedArrayIsError = Object.getPrototypeOf(getError(root.example.throwNotClonableNestedArray)) === Error.prototype;
+          const notClonableNestedObjectIsError = Object.getPrototypeOf(getError(root.example.throwNotClonableNestedObject)) === Error.prototype;
+          const dynamicIsError = Object.getPrototypeOf(getError(root.example.throwDynamic)) === Error.prototype;
           const argumentConvertIsError = Object.getPrototypeOf(getError(() => root.example.argumentConvert(Object(Symbol('test'))))) === Error.prototype;
-          return [normalIsError, weirdIsError, notClonableIsError, argumentConvertIsError];
+          const rejectNotClonableIsError = Object.getPrototypeOf(await getAsyncError(root.example.rejectNotClonable)) === Error.prototype;
+          const resolveNotClonableIsError = Object.getPrototypeOf(await getAsyncError(root.example.resolveNotClonable)) === Error.prototype;
+          return [normalIsError, weirdIsError, notClonableIsError, notClonableNestedArrayIsError, notClonableNestedObjectIsError, dynamicIsError, argumentConvertIsError, rejectNotClonableIsError, resolveNotClonableIsError];
         });
-        expect(result).to.deep.equal([true, true, true, true], 'should all be errors in the current context');
+        expect(result).to.deep.equal([true, true, true, true, true, true, true, true, true], 'should all be errors in the current context');
       });
 
       it('should not leak prototypes', async () => {

spec/api-menu-spec.ts

@@ -1,6 +1,6 @@
 import * as cp from 'child_process';
 import * as path from 'path';
-import { expect } from 'chai';
+import { assert, expect } from 'chai';
 import { BrowserWindow, Menu, MenuItem } from 'electron/main';
 import { sortMenuItems } from '../lib/browser/api/menu-utils';
 import { ifit } from './lib/spec-helpers';
@@ -878,6 +878,46 @@ describe('Menu module', function () {
         throw new Error('Menu is garbage-collected while popuping');
       }
     });
+
+    // https://github.com/electron/electron/issues/35724
+    // Maximizing window is enough to trigger the bug
+    // FIXME(dsanders11): Test always passes on CI, even pre-fix
+    ifit(process.platform === 'linux' && !process.env.CI)('does not trigger issue #35724', (done) => {
+      const showAndCloseMenu = async () => {
+        await setTimeout(1000);
+        menu.popup({ window: w, x: 50, y: 50 });
+        await setTimeout(500);
+        const closed = once(menu, 'menu-will-close');
+        menu.closePopup();
+        await closed;
+      };
+
+      const failOnEvent = () => { done(new Error('Menu closed prematurely')); };
+
+      assert(!w.isVisible());
+      w.on('show', async () => {
+        assert(!w.isMaximized());
+        // Show the menu once, then maximize window
+        await showAndCloseMenu();
+        // NOTE - 'maximize' event never fires on CI for Linux
+        const maximized = once(w, 'maximize');
+        w.maximize();
+        await maximized;
+
+        // Bug only seems to trigger programmatically after showing the menu once more
+        await showAndCloseMenu();
+
+        // Now ensure the menu stays open until we close it
+        await setTimeout(500);
+        menu.once('menu-will-close', failOnEvent);
+        menu.popup({ window: w, x: 50, y: 50 });
+        await setTimeout(1500);
+        menu.off('menu-will-close', failOnEvent);
+        menu.once('menu-will-close', () => done());
+        menu.closePopup();
+      });
+      w.show();
+    });
   });
 
   describe('Menu.setApplicationMenu', () => {