Bugzilla

I think I know why this is happening now. When we call mController->GetTopLevelPresShell() in APZTaskRunnable.cpp that seems like it'd just be returning a pointer, however it is not. ContentProcessController::GetTopLevelPresShell calls BrowserChild::GetTopLevelPresShell which calls BrowserChild::GetTopLevelDocument which what seems like a pure getter on nsIWebNavigation, however it will actually create a document if one does not exist

https://searchfox.org/mozilla-central/rev/4365e6ab34ca829c930b60252ff19a5f42856da0/docshell/base/nsIWebNavigation.idl#342

And I've run into that before where the creation of a doc from what seemed like a getter was causing one of my patches to fail. Anyways, that can get to nsDocShell::CreateAboutBlankContentViewer which can apparently kill the current docshell

https://searchfox.org/mozilla-central/rev/4365e6ab34ca829c930b60252ff19a5f42856da0/docshell/base/nsDocShell.cpp#6661

I audited all the C++ callers of nsIWebNavigation::GetDocument (there aren't that many), nsWebBrowser::SaveDocument we can just hold a refptr to make it safe (I don't know much about that function so I don't want to change it). The other caller is BrowserChild::GetTopLevelDocument. I checked all users of BrowserChild::GetTopLevelDocument and they all seem better to not create a document. So not too hard to fix up the nsIWebNavigation::GetDocument callers.

However the root of the problem is the PermitUnload call in nsDocShell::CreateAboutBlankContentViewer which could destroy the current docshell in that funciton except for the strong pointer we hold in that function, but then the docshell will probably just die when we return from the function. Looking at all callers of PermitUnload it seems there are several places that could be unsafe (there's quite a lot of auditing to do it properly).

I think I know why this is happening now. When we call mController->GetTopLevelPresShell() in APZTaskRunnable.cpp that seems like it'd just be returning a pointer, however it is not. ContentProcessController::GetTopLevelPresShell calls BrowserChild::GetTopLevelPresShell which calls BrowserChild::GetTopLevelDocument which what seems like a pure getter on nsIWebNavigation, however it will actually create a document if one does not exist

https://searchfox.org/mozilla-central/rev/4365e6ab34ca829c930b60252ff19a5f42856da0/docshell/base/nsIWebNavigation.idl#342

And I've run into that before where the creation of a doc from what seemed like a getter was causing one of my patches to fail. Anyways, that can get to nsDocShell::CreateAboutBlankContentViewer which can apparently kill the current docshell

https://searchfox.org/mozilla-central/rev/4365e6ab34ca829c930b60252ff19a5f42856da0/docshell/base/nsDocShell.cpp#6661

I audited all the C++ callers of nsIWebNavigation::GetDocument (there aren't that many), nsWebBrowser::SaveDocument we can just hold a refptr to make it safe (I don't know much about that function so I don't want to change it functionally). The other caller is BrowserChild::GetTopLevelDocument. I checked all users of BrowserChild::GetTopLevelDocument and they all seem better to not create a document. So not too hard to fix up the nsIWebNavigation::GetDocument callers.

However the root of the problem is the PermitUnload call in nsDocShell::CreateAboutBlankContentViewer which could destroy the current docshell in that funciton except for the strong pointer we hold in that function, but then the docshell will probably just die when we return from the function. Looking at all callers of PermitUnload it seems there are several places that could be unsafe (there's quite a lot of auditing to do it properly).