Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MacOS] Remove window focus listener on unload #11075

Conversation

colin-grant-work
Copy link
Contributor

What it does

On master in Electron on MacOS, we attach a listener for window focus and never release it. As a consequence, if the user refreshes the Electron window, they will see logs indicating that a listener was called an a renderer that has been released. This PR removes that message by ensuring that we remove the listener when the window unloads.

How to test

  1. On MacOS
  2. Start the Electron application
  3. Confirm that the OSX top bar menu appears as expected and the menu items behave as they should.
  4. Switch windows (on the same monitor, if you have more than one)
  5. Observe that the OSX top bar menu is swapped out as expected and continues to work as expected.
  6. Refresh the window one or more times. (Cmd+R)
  7. Repeat steps 2-4
  8. Observe additionally that your backend logs do not show any messages about callbacks in released renderers.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added electron issues related to the electron target OS/Mac issues related to Mac OS menus issues related to the menu labels Apr 25, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I confirm that the changes work well on OSX for me 👍

The app-menu is correctly populated and appears, and also works well when switching applications.

I also confirm that the log from master does not appear:

Attempting to call a function in a renderer window that has been closed or released.
Function provided here: vendors-node_modules_electron_remote_renderer_index_js.js:242:30
Attempting to call a function in a renderer window that has been closed or released.
Function provided here: vendors-node_modules_electron_remote_renderer_index_js.js:242:30
Attempting to call a function in a renderer window that has been closed or released.
Function provided here: vendors-node_modules_electron_remote_renderer_index_js.js:242:30
Attempting to call a function in a renderer window that has been closed or released.
Function provided here: vendors-node_modules_electron_remote_renderer_index_js.js:242:30

@colin-grant-work colin-grant-work merged commit 68c0181 into eclipse-theia:master Apr 26, 2022
@colin-grant-work colin-grant-work deleted the bugfix/callback-on-released-renderer branch April 26, 2022 13:23
@colin-grant-work colin-grant-work added this to the 1.25.0 milestone Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target menus issues related to the menu OS/Mac issues related to Mac OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants