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 to issue #1109 regarding Webbrowser #1110

Closed
wants to merge 2 commits into from
Closed

Conversation

eriksunden
Copy link
Member

Fixes issue #1109

… when desctruction of WebBrowserProcessor occurs..
@CLAassistant
Copy link

CLAassistant commented Feb 9, 2021

CLA assistant check
All committers have signed the CLA.

@eriksunden eriksunden linked an issue Feb 9, 2021 that may be closed by this pull request
WebBrowserClient* webbrowser =
static_cast<WebBrowserClient*>(browser_->GetHost()->GetClient().get());
webbrowser->removeLoadHandler(this);
webbrowser->OnBeforeClose(browser_);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that this is a solution to the problem. From the docs of close_browser:
"Results in a call to cef_life_span_handler_t::do_close() if the event handler allows the close or if |force_close| is true (1)"

Copy link
Contributor

Choose a reason for hiding this comment

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

If the code above resolves the issue we might want to look into handling browser destruction in WebBrowserClient::OnBeforeClose

@@ -141,13 +141,13 @@ bool WebBrowserClient::DoClose(CefRefPtr<CefBrowser> browser) {
void WebBrowserClient::OnBeforeClose(CefRefPtr<CefBrowser> browser) {
CEF_REQUIRE_UI_THREAD();
auto bdIt = browserParents_.find(browser->GetIdentifier());
if (bdIt != browserParents_.end()) {
if (bdIt != browserParents_.end() && messageRouter_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is caused by a bug you introduced with your other fix :)

@ResearchDaniel
Copy link
Contributor

See PR #1115 instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Webbrowser crash on loading of new/same workspace
4 participants