Repository: electron/electron (language: cpp)

Issue:
[Bug]: After resize the window manually, clicking the menu does not show submenus
### Preflight Checklist

- [x] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

20.1.4

### What operating system are you using?

Other Linux

### Operating System Version

Manjaro Linux 22.0.0

### What arch are you using?

x64

### Last Known Working Electron version

`v19.0.0-alpha.3`

### Expected Behavior

After resize the window manually, clicking the menu should show submenus

### Actual Behavior

After resize the window manually, clicking the menu does not show submenus, both gnome and kde, manjaro and ubuntu

### Testcase Gist URL

_No response_

### Additional Information

_No response_


Discussion / hints:
Thanks for reporting this and helping to make Electron better!

Please use [Electron Fiddle](https://github.com/electron/fiddle) to create a standalone (no external dependencies) testcase and publish it as a [gist](https://gist.github.com), then link to it here.

- Even if you have a code snippet or a git repository that show the testcase, please put it in a gist. This lets maintainers triage issues more effectively. It also ensures that we're testing the exact same issue that you're seeing!

- Please avoid external dependencies in the test. The Electron team is small and generally doesn't have enough bandwidth to triage third-party code. Also, standalone tests are easier to adapt into regression tests.

- If the bug you've found can be tested with a pass / fail test, please make the testcase [exit 0 on success or nonzero on failure](https://github.com/electron/bugbot#by-bug-reporters). This lets [Electron's issue bot](https://github.com/electron/bugbot#readme) see what releases are affected by the bug.

I'm adding the `blocked/need-repro` label for this reason. After you make a test case, please link to the gist in a followup comment.

Thanks in advance! Your help is appreciated.

> 

This seems to be the case with all electron programs, including vscode, and my own simplest demos
@ckerr 
gist: https://gist.github.com/veekxt/00cce2d9051dc6fffee5c4227e2a7d5f

![image](https://i.imgur.com/vjOd2UQ.gif)
I found this problem with version v19.0.0-alpha.4 or newer, but not with v19.0.0-alpha.3, so maybe we should see what changes have been made to v19.0.0-alpha.4
The most likely suspect in https://github.com/electron/electron/compare/v19.0.0-alpha.3...v19.0.0-alpha.4 is https://github.com/electron/electron/commit/cfd9825bc2d2b95cea1c5f2d17bd4cecc8d41c16
More details from duplicate issue #35960: This seems to also happen on manually un-maximizing a window (but not when maximizing it). There is also another test case repro and screencast video in that issue.
I have tested with provided [gist](https://gist.github.com/veekxt/00cce2d9051dc6fffee5c4227e2a7d5f) from @veekxt with electron v19.0.13 and v20.0.3 . I am an Archlinux user with KDE Plasma Desktop. I found two things when testing:
1. After starting Electron, the first resizing of the window did not cause the problem, but the second resizing did. In each resizing, the window size kept changing without stopping. 
2. After the Compositor of the desktop environment was suspended, this problem disappeared.

I hope there can help!
This issue has been automatically marked as stale. **If this issue is still affecting you, please leave any comment** (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the [latest version of Electron](https://www.electronjs.org/releases/stable) or in the [beta](https://www.electronjs.org/releases/beta)—please include it with your comment!
This bug still exists in electron 20.3.12. Tested with the same [gist](https://gist.github.com/veekxt/00cce2d9051dc6fffee5c4227e2a7d5f).
I want to point out that this also applies to context-menus, which makes it much worse. I heavily rely on context-menus.

In the Fiddle you can open a context menu. After resizing the window it won't open again.

https://gist.github.com/a1e2d62b6d41e24deb061672fb4f73c8

(yeah, that screen recording is really great without a cursor :smile:  )

[Screencast from 2023-02-28 21-41-46.webm](https://user-images.githubusercontent.com/679144/221974764-f95e9a6f-0dbc-4c49-bae1-5c6e5329cb5a.webm)

This bug also suitable for the context menu. Menu receive event 'menu-will-close' immediately after calling` .popup()`
Still present in Electron 25.0.0-nightly.20230327. @clavin is correct that maximizing -> unmaximizing is enough to trigger this in the Hello World fiddle.
> I found this problem with version v19.0.0-alpha.4 or newer, but not with v19.0.0-alpha.3, so maybe we should see what changes have been made to v19.0.0-alpha.4

Confirmed, this works in 19.0.0-alpha.3 but not alpha.4.
...and just to finish nodding my head in agreement for a third comment, agree with @codebytere that cfd9825bc2d2b95cea1c5f2d17bd4cecc8d41c16 looks like the most likely suspect.

(TBH I'm not seeing the problem in cfd9825bc2d2b95cea1c5f2d17bd4cecc8d41c16 yet, but there are only a handful of commits between alpha.3 and alpha.4 and the others look even less likely, so :man_shrugging:)
This bug (for context-menu) also occurs when I switch between full-screen mode (only on Ubuntu; Windows and Mac work correctly). It is worth mentioning that it is behaving normally after I switch focus from my app to another app's window and go back to my app (until I resize/fullscreen again)
I've tracked this down to an upstream Chromium issue. Will be opening an Electron PR shortly to fix it by reverting the offending Chromium commit, and then will open a crbug so they can fix it properly and we'll float the patch until then.

For the interested reader, I confirmed this was an upstream issue by checking the `views_examples` target on Chromium `main`, where it reproduced. Then I bisected the Chromium commits in the suspected range (`102.0.4971.0..102.0.4989.0`) until I found the offending commit (required applying [`9274e9d`](https://chromium.googlesource.com/chromium/src.git/+/9274e9d8ae89bc616019ca945f85d1be2f661365) to fix `views_examples` on those older revisions). That led to [the offending CL](https://chromium-review.googlesource.com/c/chromium/src/+/3565227) which looks to just be an attempted performance improvement, so it's easy enough to revert without any larger impact.

Fix patch:
diff --git a/patches/chromium/.patches b/patches/chromium/.patches
index 97553efb6b586..d0792f0ca5b54 100644
--- a/patches/chromium/.patches
+++ b/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
diff --git a/patches/chromium/revert_x11_keep_windowcache_alive_for_a_time_interval.patch b/patches/chromium/revert_x11_keep_windowcache_alive_for_a_time_interval.patch
new file mode 100644
index 0000000000000..95e23e6fd7322
--- /dev/null
+++ b/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();
+   }
+ 
diff --git a/spec/api-menu-spec.ts b/spec/api-menu-spec.ts
index adf10663d69bd..6207e0fb1ccb6 100644
--- a/spec/api-menu-spec.ts
+++ b/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';
@@ -887,6 +887,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', () => {


Test patch: