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

Hooking windows messages #3231

Merged
merged 2 commits into from
Oct 27, 2015
Merged

Hooking windows messages #3231

merged 2 commits into from
Oct 27, 2015

Conversation

omrilitov
Copy link
Contributor

As discussed here #3217
Hooked into the windows messages and emits them through the window instance with the event 'message'

@anaisbetts
Copy link
Contributor

Hooking every window message seems like it'll cause performance and reentrancy problems, maybe allow people to register for specific ones?

@zcbenz
Copy link
Member

zcbenz commented Oct 27, 2015

The API proposal in #1121 looks like a good start.

@omrilitov
Copy link
Contributor Author

Well, because it's the window's wndproc it already receives all the messages.
The difference registering for a specific message would make is that it won't emit to the javascript callback which most of the time won't do anything anyway.
The question for me is if emitting to the javascript is a big performance hit.
If not the registration can also be done in pure javascript

@zcbenz
Copy link
Member

zcbenz commented Oct 27, 2015

Yeah emitting an event in JavaScript is quite expensive, it involves obtaining the lock, creating several JavaScript objects, and calling into JavaScript context, though all the details have been hidden in the Emit method, it is enough to have negative affect on performance.

@omrilitov
Copy link
Contributor Author

The API is now similar to global shortcut, you hook with a specific message

@zcbenz
Copy link
Member

zcbenz commented Oct 27, 2015

Awesome, thanks!

zcbenz added a commit that referenced this pull request Oct 27, 2015
@zcbenz zcbenz merged commit 142f380 into electron:master Oct 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants