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

VERIFIED FIXED in Firefox 66

Status

()

defect
--
critical
VERIFIED FIXED
8 months ago
3 months ago

People

(Reporter: Virtual, Assigned: bzbarsky)

Tracking

({crash, nightly-community, regression})

66 Branch
mozilla66
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65 unaffected, firefox66+ verified)

Details

(crash signature)

Attachments

(1 attachment)

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?
Duplicate of this bug: 1517518
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 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
https://hg.mozilla.org/mozilla-central/rev/2abc6090d9f4
Status: NEW → RESOLVED
Closed: 8 months 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/
Status: RESOLVED → VERIFIED
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
You need to log in before you can comment on or make changes to this bug.