Skip to content

Repairing pywin32 example (#224) #374

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 6 commits into from
Jan 7, 2018

Conversation

jf-mgd
Copy link
Contributor

@jf-mgd jf-mgd commented Jul 4, 2017

No description provided.

Copy link
Owner

@cztomczak cztomczak left a comment

Choose a reason for hiding this comment

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

@cztomczak
Copy link
Owner

cztomczak commented Jul 5, 2017

Thanks for the PR. Code changes are overall good, there are only some minor style issues. I will test it when I switch to a Windows machine.

@jf-mgd
Copy link
Contributor Author

jf-mgd commented Jul 5, 2017

By default there is a random port generated

Yes you can remove this commit, it was for my personals tests, I should not have added that to this branch...

@cztomczak cztomczak changed the title Repairing pywin32 example Repairing pywin32 example (#224) Jul 25, 2017
@cztomczak
Copy link
Owner

Related issue: Issue #224 ("Port CEF 1 examples to CEF 3").

@cztomczak
Copy link
Owner

I have tested on Windows 7 with Python 2.7/3.4 and it works fine in default loop mode. However in multi threaded message loop it crashes on exit. Have you noticed this behavior during your testing?

@jf-mgd
Copy link
Contributor Author

jf-mgd commented Aug 3, 2017

Yes, and it was something I wanted to bring to your attention, but I forgot :/
It comes from the fact that there's a call do CefDoMessageLoopWork (https://github.com/cztomczak/cefpython/blob/master/src/cefpython.pyx#L965) in cefpython.Shutdown().
Not calling this function when using multithreaded message loop should do the trick, although it's on speculation level as I do not have a Cython compiler to test this hypothesis.

@cztomczak
Copy link
Owner

That makes sense. Created Issue #380 to fix that.

@cztomczak cztomczak merged commit 43126af into cztomczak:master Jan 7, 2018
@cztomczak
Copy link
Owner

Thanks. I will make additional changes to the example in a separate commit.

cztomczak added a commit that referenced this pull request Jan 7, 2018
Add link to README-examples.
Add usage comments.
Fix code style issues - get rid of PEP8 and PyCharm notices/warnings.
Load chromium.ico icon.
@cztomczak
Copy link
Owner

Additional changes in commit b9875f5.

@cztomczak
Copy link
Owner

@jf-mgd If you would like to add add yourself to the Authors file then please send a separate PR for that. In the authors file you can add and email or a website link next to your name.

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.

2 participants