1611767 - Crash in [@ nsGlobalWindowOuter::TabGroupOuter]

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

This bug is for crash report bp-b37c1045-835a-4985-9732-ed7d60200126.

Top 10 frames of crashing thread:

0 xul.dll nsGlobalWindowOuter::TabGroupOuter dom/base/nsGlobalWindowOuter.cpp:7536
1 xul.dll nsGlobalWindowInner::TabGroupInner dom/base/nsGlobalWindowInner.cpp:7064
2 xul.dll nsWindowMemoryReporter::CheckForGhostWindows dom/base/nsWindowMemoryReporter.cpp:850
3 xul.dll nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:562
4 xul.dll nsTimerEvent::Run xpcom/threads/TimerThread.cpp:259
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1220
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:486
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:87
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:308
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:290

This looks like an old crash (we've got reports going back to version 60) that happens when calling nsWindowMemoryReporter::CheckForGhostWindows(). The stacks are consistent across all versions.

We seem to be getting a NULL pointer dereference here.

Comment 1

[Andrew McCreight [:mccr8]]

We're trying to decide if a window is a ghost window. To do that, we get its tab group, to see if it is in the same tab group as a window we know is being displayed or whatever. However, this window doesn't have a tab group, so we look at its outer window to get the outer window's tab group.

Then we crash on this line: if (GetDocShell()->ItemType() == nsIDocShellTreeItem::typeChrome) {

Presumably GetDocShell() is null.

The basic issue here is that the ghost window detector is going to be called on windows in all sorts of weird states. Returning null from nsGlobalWindowOuter::TabGroupOuter() would presumably violate the invariant that this method never returns null, but for this particular consumer it seems okay to do. TabGroups are on the way out (bug 1561715), but maybe we could still fix this in the meanwhile. Maybe some kind of MaybeTabGroupOuter() method that does null checks would be okay.

Comment 2

[Andrew McCreight [:mccr8]]

Interesting find, Gabriele!

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

mccr8: Could you keep handling this? or could you assign somebody who can fix this soon?

Comment 4

[Andrew McCreight [:mccr8]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #3)

mccr8: Could you keep handling this? or could you assign somebody who can fix this soon?

Sure, I can try to figure something out.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1611767 - Add MaybeTabGroup methods.

The ghost window checker examines windows in all sorts of weird states,
apparently including when the docshell on the outer is null. Add some
fallible variants of the tab group methods on inner and outer windows
so it can call one of those instead.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db5a39781cc9
Add MaybeTabGroup methods. r=farre

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/db5a39781cc9