1594625 - ContentChild::ProvideWindowCommon can end up calling Resume() on a dying window

Status: NEW

(Core :: DOM: Navigation, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: log.txt

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=274747527&repo=autoland&lineNumber=9003 produces the attached stack while running dom/html/test/test_window_open_close.html.

What's going on there is that we are firing a load event in frame 43. That runs some JS that calls window.open. That spins a nested event loop under ContentChild::ProvideWindowCommon, which processes a link click event. That link click event is targeted, and also tries to open a new window, which again lands in ContentChild::ProvideWindowCommon (recursively, now). It pauses the parent of the window being opened, does the same "spin the event loop" thing, then tries to resume the parent window, but at this point that parent window is dying. The stack came from an assert that basically asserted that Resume is not called on a dying window (in the IsDying() sense).

It's not clear to me whether this "resume on a dying window" is OK, and if not where exactly we should guard on it, but it seemed a bit weird.

The actual sequence of events is presumably that we first open file_window_open_close_outer.html for one of our three tests. Its load event causes us to queue a link click to open file_window_open_close_inner.html and set a timeout to close the popup, then return.

Then we open another copy of file_window_open_close_outer.html. Trying to do that spins the event loop and processes the link click event. That starts another window opening sequence, and spinning the event loop in there fires the timer that does the close() call, so by the time the event loop spin is done and the file_window_open_close_inner.html window has been created its opener is closed and dying...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note that we already early-exit out of Resume if !IsCurrentInnerWindow, which would handle navigated-away-from, but closing a window leaves it as the current inner even as it dies.

I suspect that if we had mHasVREvents true at this point (e.g. if we managed to sneak an addEventListener call to add such an event listener after the window has started dying but before the innermost event loop spin unwinds), we'd end up with a leak due to EnableVRUpdates (called from Resume) creating a VREventObserver after FreeInnerObjects. That said, I'm not even sure we need this scenario for that: just adding the event listener after the window is torn down will call nsGlobalWindowInner::NotifyVREventListenerAdded which will call EnableVRUpdates which will do the leak thing.

For that matter, if the addEventListener call happens while the window is suspended, it will EnableVRUpdates at that point, which does not seem right either.

I think the gamepad listener bits might have the same sorts of issues too...

Comment 2

[Boris Zbarsky [:bzbarsky]]

We might need a separate bug (or bugs) on the VR/Gamepad listener interactions here.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:neha, could you have a look please?

For more information, please visit auto_nag documentation.