Skip to content

Commit

Permalink
[embedlite] Workaround for browser crash during window close JB#29415
Browse files Browse the repository at this point in the history
The current shutdown procedure of EmbedLite assumes that NULLing
nsWebBrowser pointer in EmbedLiteViewThreadChild::RecvDestroy will actually
destroy it. This assumption is quite critical since the removing
nsWebBrowser also triggers removal of EmbedLitePuppetWidgets we created.
All the EmbedLitePupetWidgets hold a reference to the view object in
mEmbed variable which they use before they are destroyed. This means the
EmbedLiteViewThreadChild has to be removed last.

The proble is that EmbedLiteViewThreadchild::mWebBrowser is
reference counted. NULLing it will only remove it if the reference
count for the object happens to be exactly 1. The object however,
can be also referenced in event listeners registered for nsWindowRoot.
In theory the event listener chain should be removed by
calling mChrome->RemoveEventHandler(), but the whole process is
asynchronous. It posts tasks to the thread main loop to actuall
remove the listeners.

The proper fix for the issue should delay actual
PEmbedListViewChild::Send__delete__ call until we get some notification
from all the EmbedLitePuppetWidget children that they have been destroyed.
This would require a bigger redesign of the code, however.

This patch is a simple workaround that removes the chrome event
handler as soon as we get window close request from gecko. This gives
main loop a chance to process listener removal tasks before
EmbedLiteViewThreadChild::RecvDestroy is called.
  • Loading branch information
tworaz authored and rainemak committed Oct 30, 2015
1 parent 73cf2c6 commit b5ed4b9
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion embedding/embedlite/embedshared/EmbedLiteViewBaseChild.cpp
Expand Up @@ -1056,7 +1056,21 @@ EmbedLiteViewBaseChild::OnLoadFinished()
NS_IMETHODIMP
EmbedLiteViewBaseChild::OnWindowCloseRequested()
{
return SendOnWindowCloseRequested() ? NS_OK : NS_ERROR_FAILURE;
if (SendOnWindowCloseRequested()) {
// The event listeners can indirectly hold a reference to nsWebBrowser.
// Since the listener removal process is not synchronous we have to make
// sure that we start the process before ::RecvDestroy is called. Otherwise
// NULLing mWebBrowser in the function won't actually remove it and the current
// implementation relies on that. The nsWebBrowser object needs to be removed in
// order for all EmbedLitePuppetWidgets we created to also be released. The puppet
// widget implementation holds a pointer to EmbedLiteViewThreadParent in mEmbed
// variable and can use it during shutdown process. This means puppet widgets
// have to be removed before the view.
if (mChrome)
mChrome->RemoveEventHandler();
return NS_OK;
}
return NS_ERROR_FAILURE;
}

NS_IMETHODIMP
Expand Down

0 comments on commit b5ed4b9

Please sign in to comment.