Bug 1732818 - Ensure DestroyComplete is run even if the BrowserParent is already destroyed, r=smaug

I was unable to determine why the BrowserParent is already destroyed,
but the change of checking `CanSend()` in `nsFrameLoaderDestroyRunnable`
seems to have fixed the issue in my test run.

My original theory was that somehow the BrowserParent is being destroyed
before it is associated with the nsFrameLoader, but the assertions to
that effect which I added didn't seem to fire. I'm guessing this may
have something instead to do with BFCache, but I'm not completely
certain.

Differential Revision: https://phabricator.services.mozilla.com/D130103
This commit is contained in:
Nika Layzell
2021-11-03 18:11:58 +00:00
parent 849bb4e21f
commit 3f131b48c2
6 changed files with 28 additions and 7 deletions

View File

@@ -1988,10 +1988,12 @@ nsresult nsFrameLoaderDestroyRunnable::Run() {
// In the out-of-process case, BrowserParent will eventually call
// DestroyComplete once it receives a __delete__ message from the child.
// In the in-process case, we dispatch a series of runnables to ensure
// that DestroyComplete gets called at the right time. The frame loader is
// kept alive by mFrameLoader during this time.
if (mFrameLoader->mChildMessageManager) {
// In the in-process case, or if the BrowserParent was already destroyed,
// we dispatch a series of runnables to ensure that DestroyComplete gets
// called at the right time. The frame loader is kept alive by
// mFrameLoader during this time.
if (!mFrameLoader->GetRemoteBrowser() ||
!mFrameLoader->GetRemoteBrowser()->CanRecv()) {
// When the docshell is destroyed, NotifyWindowIDDestroyed is called to
// asynchronously notify {outer,inner}-window-destroyed via a runnable.
// We don't want DestroyComplete to run until after those runnables have
@@ -2611,6 +2613,16 @@ bool nsFrameLoader::TryRemoteBrowserInternal() {
return false;
}
if (RefPtr<nsFrameLoaderOwner> flo = do_QueryObject(mOwnerContent)) {
RefPtr<nsFrameLoader> fl = flo->GetFrameLoader();
if (fl != this) {
MOZ_ASSERT_UNREACHABLE(
"Got TryRemoteBrowserInternal but mOwnerContent already has a "
"different frameloader?");
return false;
}
}
if (!doc->IsActive()) {
// Don't allow subframe loads in non-active documents.
// (See bug 610571 comment 5.)
@@ -2735,6 +2747,8 @@ bool nsFrameLoader::TryRemoteBrowserInternal() {
// Grab the reference to the actor
RefPtr<BrowserParent> browserParent = GetBrowserParent();
MOZ_ASSERT(browserParent->CanSend(), "BrowserParent cannot send?");
// We no longer need the remoteType attribute on the frame element.
// The remoteType can be queried by asking the message manager instead.
ownerElement->UnsetAttr(kNameSpaceID_None, nsGkAtoms::RemoteType, false);