1273739 - Crash in nsDeviceContext::GetDepth

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect, P3)

Description

[Nicholas Nethercote [inactive]]

This bug was filed from the Socorro interface and is 
report bp-0691269b-c81a-45f7-8037-e444a2160508.
=============================================================

This is a low-volume crash, with 21 occurrences this year across versions 45--49, mostly on Windows but with a couple on Mac.

> 0 	xul.dll 	nsDeviceContext::GetDepth(unsigned int&) 	gfx/src/nsDeviceContext.cpp:452
> 1 	xul.dll 	nsScreen::GetPixelDepth(mozilla::ErrorResult&) 	dom/base/nsScreen.cpp:80
> 2 	xul.dll 	mozilla::dom::ScreenBinding::get_pixelDepth 	obj-firefox/dom/bindings/ScreenBinding.cpp:150
> 3 	xul.dll 	js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:432

The crash address is always 0x0 -- |primaryScreen| is null. This means that GetPrimaryScreen() must have not set |primaryScreen|, or set it to null. nsScreenManagerProxy::GetPrimaryScreen() is the only GetPrimaryScreen() implementation I can see that can fail, though it's conceivable that one of the other GetPrimary() implementations may have assigned null.

Looking around at GetPrimaryScreen() call sites, most of them are null-checked or rv-checked, but a few (including this one) are not.

mattwoodrow, any thoughts on how to handle this one? Add null checks to all GetPrimaryScreen() call sites that lack them?

Comment 1

[Matt Woodrow (:mattwoodrow)]

I don't know this code particularly well, but adding null checks sounds reasonable to me.

Comment 2

[Nicholas Nethercote [inactive]]

Any ideas who knows this code well?

Comment 3

[BugBot [:suhaib / :marco]]

Crash volume for signature 'nsDeviceContext::GetDepth':
 - nightly (version 50): 5 crashes from 2016-06-06.
 - aurora  (version 49): 42 crashes from 2016-06-07.
 - beta    (version 48): 23 crashes from 2016-06-06.
 - release (version 47): 1 crash from 2016-05-31.
 - esr     (version 45): 0 crashes from 2016-04-07.

Crash volume on the last weeks:
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly       0       0       0       1       2       1       0
 - aurora        3      28       2       3       3       2       1
 - beta          0       1       0       0       0      21       1
 - release       0       0       1       0       0       0       0
 - esr           0       0       0       0       0       0       0

Affected platform: Windows

Comment 4

[BugBot [:suhaib / :marco]]

Closing because no crashes reported for 12 weeks.