Skip to content

fix: delay emitting NotifyIcon events on Windows #26668

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

Merged
merged 14 commits into from
Nov 30, 2020

Conversation

erickzhao
Copy link
Member

@erickzhao erickzhao commented Nov 24, 2020

Description of Change

Following prior work done in #25836, this PR delays emitting Balloon events from the Tray module to avoid entering V8 during a WndProc callback.

Implementation Details

Since TrayIcon objects are deleted when the Tray gets garbage collected, we don't want the callback to be processed if the icon is gone. This PR uses WeakPtr references to achieve this end.

This also means that the affected events won't be emitted if the WndProc callback is entered, but the TrayIcon is destroyed. This behaviour seems in line with Electron's existing Tray API design, as a reference to the Tray needs to be maintained for its API to be used at all.

cc @nornagon

Checklist

Release Notes

Notes: Fixed a rare crash on Windows that could occur when emitting certain Tray events.

@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Nov 24, 2020
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Nov 25, 2020
@erickzhao erickzhao changed the title fix: delay emitting Tray Balloon events on Windows fix: delay emitting NotifyIcon events on Windows Nov 25, 2020
@zcbenz zcbenz merged commit 36af802 into electron:master Nov 30, 2020
@release-clerk
Copy link

release-clerk bot commented Nov 30, 2020

Release Notes Persisted

Fixed a rare crash on Windows that could occur when emitting certain Tray events.

@erickzhao erickzhao deleted the erick/delay-notifyicon-events branch March 10, 2021 19:24
erickzhao added a commit to erickzhao/ericktron that referenced this pull request Mar 10, 2021
* wip?

* attempt to use weakptr

* apply posttask change to other balloon events

* chore: add clarifying comment on weakptr

* refactor: move weakptr include to implementation

(it's not needed in the header file)

* refactor: use default initializer for weak factory

* refactor: move weakptr usage outside of loop

* fix: convert mouse events as well

* refactor: use member function for balloon events

* fix: check if wicon is truthy in callback

* refactor: bind mouse events with member function

* refactor: inline lparams

* refactor: inline getkeyboardmodifiers()

* chore: correct GetKeyboardModifiers typo
@trop
Copy link
Contributor

trop bot commented Mar 10, 2021

@erickzhao has manually backported this PR to "12-x-y", please check out #28111

@codebytere codebytere added the semver/patch backwards-compatible bug fixes label Mar 12, 2021
zcbenz pushed a commit that referenced this pull request Mar 13, 2021
* wip?

* attempt to use weakptr

* apply posttask change to other balloon events

* chore: add clarifying comment on weakptr

* refactor: move weakptr include to implementation

(it's not needed in the header file)

* refactor: use default initializer for weak factory

* refactor: move weakptr usage outside of loop

* fix: convert mouse events as well

* refactor: use member function for balloon events

* fix: check if wicon is truthy in callback

* refactor: bind mouse events with member function

* refactor: inline lparams

* refactor: inline getkeyboardmodifiers()

* chore: correct GetKeyboardModifiers typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants