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

fix: crash when tray popup called twice #18999

Merged
merged 1 commit into from Jun 28, 2019

Conversation

@codebytere
Copy link
Member

codebytere commented Jun 27, 2019

Description of Change

When the Tray popup functionality was made asynchronous, ScopedPumpMessagesInPrivateModes was introduced to ensure that the UI can update while the menu is fading out. However, given that we're now asynchronous, there exists a possibility that we can call popUpContextMenu more than once and this will cause a crash since we're no longer blocking the main process.

As a result, we only want to call ScopedPumpMessagesInPrivateModes if we're going to open the menu, which means that we first need to ensure that the menu is not already open. This PR thus fixes the aforementioned crash which can be seen in the following code before this PR and is fixed with the code therein.

  tray.setContextMenu(contextMenu)
  tray.popUpContextMenu()
  tray.popUpContextMenu()

cc @deermichel @miniak

Checklist

Release Notes

Notes: Fixed a crash on tray popup being called multiple times in a row.

@codebytere codebytere requested review from miniak and deermichel Jun 27, 2019
@deermichel

This comment has been minimized.

Copy link

deermichel commented Jun 27, 2019

Works for me. Can you add a test for this or is testing tray stuff difficult on CI?

@codebytere

This comment has been minimized.

Copy link
Member Author

codebytere commented Jun 27, 2019

@deermichel the tray tests don't work presently on Mac (you'll see they get skipped for all except windows) so for now no but i was planning to dig further into trying to add some/make em work on macOS once your PR to move tray tests into the main spec runner gets merged 🚀

Copy link

deermichel left a comment

aight, then go ahead 🎈

@zcbenz
zcbenz approved these changes Jun 28, 2019
@electron-cation electron-cation bot removed the new-pr 🌱 label Jun 28, 2019
@codebytere codebytere merged commit a4f6156 into master Jun 28, 2019
11 of 13 checks passed
11 of 13 checks passed
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
electron-arm-testing Build #20190627.6 succeeded
Details
electron-arm64-testing Build #20190627.6 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented Jun 28, 2019

Release Notes Persisted

Fixed a crash on tray popup being called multiple times in a row.

@codebytere codebytere deleted the double-popup-dont-crash branch Jun 28, 2019
deermichel pushed a commit that referenced this pull request Jul 2, 2019
deermichel pushed a commit that referenced this pull request Jul 2, 2019
deermichel pushed a commit that referenced this pull request Jul 2, 2019
deermichel pushed a commit that referenced this pull request Jul 2, 2019
deermichel pushed a commit that referenced this pull request Jul 10, 2019
MarshallOfSound added a commit that referenced this pull request Jul 31, 2019
* restore stash

revert

some things work others dont

tracking area for rescue

manual popup

restore drag n drop

cleanup

* fix: make tray not block main process (#18880)

* fix: make tray not block main process

* make AtomMenuModel refcounted

* add support for ansi codes in title

add remove TODOs

* chore: use ScopedPumpMessagesInPrivateModes in tray (#18977)

* chore: use ScopedPumpMessagesInPrivateModes in tray

* revert refcounting of AtomMenuModel

* Prefer WeakPtr for posting tasks to handle unexpected destruction

* cleanup .h

* cleanup .mm

* add imports

add missing include

* fix: crash when tray popup called twice (#18999)

* remove highlightMode and TODOs

* remove unnecessary copy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.