STR: 1. Start browsing website pages and in some time enjoy crash in [@ mozilla::dom::ToJSValue ] Crashlog reports: https://crash-stats.mozilla.org/report/index/a9ae0eba-74e3-47c2-91d6-dbf8d0190103 https://crash-stats.mozilla.org/signature/?product=Firefox&signature=mozilla%3A%3Adom%3A%3AToJSValue

Regression probably caused by: bug #1353867

My follow-up didn't cause the crash, but you might want to ask Boris.

Hmm, the docshell shouldn't be null but looks like its outer window might be. This is annoying, because of course we used to just get the right window even if the docshell had lost its reference.

Seems like nika tried to make that reference not go away in bug 1401379, but had to back it out.

I don't think there's any way to make sure that these objects have the same lifetime - nulling out the references between the two objects has some pretty serious impacts here unfortunately. A solution here might be to either: a) Make BrowsingContext hold a strong reference to the outer window which isn't nulled out when the window is closed. There's a chance this could cause leaks, but I'm not sure. b) Create a dummy closed WindowProxy object which just acts like a closed window if we can't grab the inner window (as the window will have been closed at that point). I don't know if we have the dummy window functionality yet, so I'm not sure how easy it would be to use that strategy. I can look into doing a quick test for the extra pointer strategy if you'd be interested. ni? bz for any insight as to whether the dummy window functionality exists already.

721 crashes/393 installs in 2 days. Can we back something out to get back to a good working state?

The stacks linked in bug 1517518 sure look like bc->GetDOMWindow() is returning null. GetDOMWindow does: return mDocShell ? mDocShell->GetWindow() : nullptr; and we've already tested that mDocShell is not null, so mDocShell->GetWindow() is returning null. The old code for nsGenericHTMLFrameElement::GetContentWindow() looked like this: nsCOMPtr<nsIDocShell> doc_shell = mFrameLoader->GetDocShell(IgnoreErrors()); if (!doc_shell) { return nullptr; } nsCOMPtr<nsPIDOMWindowOuter> win = doc_shell->GetWindow(); if (!win) { return nullptr; } so it sure looks to me like we used to get null here in the situation when the docshell returns null from GetWindow. Patch coming up to just go back to the old behavior of returning null in that case.

Oh, and as far as comment 5 goes, I don't think we have any dummy window stuff...

The old code did this check in GetContentWindow, basically. We _could_ just put the null-check there, but this seems safer.

I guess comment 3 might be correct for other binding things that returned WindowProxy, though. We might want to audit them and see whether any of them might have switched from non-null to null. I'll leave Peter to do that. But the specific .contentWindow consumer used to return null in this situation already.

Pushed by bzbarsky@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2abc6090d9f4 We might be trying to JS-wrap a BrowsingContext with a torn-down-enough docshell that we have no window. r=nika

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 66.0a1 (2019-01-04), so I'm marking this bug as VERIFIED. Thank you very much! \o/