634936 - Crash [@ nsFocusManager::Init ]

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Josh Matthews [:jdm]]

There are consumers of nsContentUtils::GetPrefBranch which don't null-check the result.  This is bad, as nsContentUtils::Init explicitly states that a null pref service is allowed.  484de288-23dc-4490-ad28-8eff22110217 is an example of a crash because of this.

Consumers requiring fixing:
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp#1904
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#197

Comment 1

[Josh Matthews [:jdm]]

https://crash-stats.mozilla.com/report/index/484de288-23dc-4490-ad28-8eff22110217

Comment 2

[Ed Morley [:emorley]]

Should the nsFocusManager instance return NS_ERROR_FAILURE or just if(prefBranch) ?

Comment 3

[Josh Matthews [:jdm]]

I think a straight |if (prefBranch)| will be fine.

Comment 4

[Ed Morley [:emorley]]

Attachment: Patch v1

Thanks.

Ideas for who to review?

Comment 5

[Josh Matthews [:jdm]]

Comment on attachment 524869 [details] [diff] [review]
Patch v1

I think jst's a good candidate here.

Comment 6

[Josh Matthews [:jdm]]

Thanks, Johnny.

Comment 7

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

And thanks, Ed!

http://hg.mozilla.org/mozilla-central/rev/1c62fb90d4dd

Comment 8

[Ed Morley [:emorley]]

No worries, thanks for the checkin :-)

Comment 9

[Ed Morley [:emorley]]

All instances of:
http://mxr.mozilla.org/mozilla-central/search?string=nsContentUtils::GetPrefBranch%28%29
...null check prefBranch.

-> Verified.