Last Comment Bug 634936 - Crash [@ nsFocusManager::Init ]
: Crash [@ nsFocusManager::Init ]
Status: VERIFIED FIXED
: crash
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla6
Assigned To: Ed Morley [:emorley]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-17 09:17 PST by Josh Matthews [:jdm]
Modified: 2011-06-09 14:58 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (2.39 KB, patch)
2011-04-09 11:08 PDT, Ed Morley [:emorley]
jst: review+
Details | Diff | Review

Description Josh Matthews [:jdm] 2011-02-17 09:17:35 PST
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 2 Ed Morley [:emorley] 2011-04-09 05:12:35 PDT
Should the nsFocusManager instance return NS_ERROR_FAILURE or just if(prefBranch) ?
Comment 3 Josh Matthews [:jdm] 2011-04-09 07:33:59 PDT
I think a straight |if (prefBranch)| will be fine.
Comment 4 Ed Morley [:emorley] 2011-04-09 11:08:25 PDT
Created attachment 524869 [details] [diff] [review]
Patch v1

Thanks.

Ideas for who to review?
Comment 5 Josh Matthews [:jdm] 2011-04-09 13:30:19 PDT
Comment on attachment 524869 [details] [diff] [review]
Patch v1

I think jst's a good candidate here.
Comment 6 Josh Matthews [:jdm] 2011-04-14 14:28:49 PDT
Thanks, Johnny.
Comment 7 :Ms2ger 2011-04-16 05:44:31 PDT
And thanks, Ed!

http://hg.mozilla.org/mozilla-central/rev/1c62fb90d4dd
Comment 8 Ed Morley [:emorley] 2011-04-18 10:20:20 PDT
No worries, thanks for the checkin :-)
Comment 9 Ed Morley [:emorley] 2011-05-02 17:09:15 PDT
All instances of:
http://mxr.mozilla.org/mozilla-central/search?string=nsContentUtils::GetPrefBranch%28%29
...null check prefBranch.

-> Verified.

Note You need to log in before you can comment on or make changes to this bug.