Closed Bug 1517415 Opened 4 years ago Closed 4 years ago

Mozilla Firefox Nightly 66.0a1 (2019-01-02) crashes in [@ mozilla::dom::ToJSValue ] after landing patches from bug #1353867


(Core :: DOM: Core & HTML, defect)

66 Branch
Windows 7
Not set



Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 + verified


(Reporter: Virtual, Assigned: bzbarsky)



(Keywords: crash, nightly-community, regression)

Crash Data


(1 file)

Regression probably caused by:
bug #1353867
Blocks: 1353867
Has Regression Range: --- → yes
My follow-up didn't cause the crash, but you might want to ask Boris.
Flags: needinfo?(jorgk) → needinfo?(bzbarsky)
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.
Flags: needinfo?(peterv)
Flags: needinfo?(bzbarsky)
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.
Flags: needinfo?(bzbarsky)
721 crashes/393 installs in 2 days. Can we back something out to get back to a good working state?
Crash Signature: [@ mozilla::dom::ToJSValue ] → [@ mozilla::dom::ToJSValue ] [@ nsPIDOMWindowOuter::GetDoc]
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...
Flags: needinfo?(bzbarsky)
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.
Assignee: nobody → bzbarsky
Pushed by
We might be trying to JS-wrap a BrowsingContext with a torn-down-enough docshell that we have no window.  r=nika
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
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/
Summary: Mozilla Firefox Nightly 66.0a1 (2019-01-02) crashes in [@ mozilla::dom::ToJSValue ] → Mozilla Firefox Nightly 66.0a1 (2019-01-02) crashes in [@ mozilla::dom::ToJSValue ] after landing patches from bug #1353867
Component: DOM → DOM: Core & HTML
See Also: → 1725107
You need to log in before you can comment on or make changes to this bug.