If Firefox cannot get prefs due to any error, firefox will crash. This bug was filed from the Socorro interface and is report bp-49b7eb0f-bf45-4f81-b2d0-a199b2111008 . ============================================================= 0 browsercomps.dll nsWindowsShellService::GetShouldCheckDefaultBrowser browser/components/shell/src/nsWindowsShellService.cpp:455 1 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102 2 xul.dll XPC_WN_GetterSetter js/src/xpconnect/src/xpcwrappednativejsops.cpp:1678 3 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:630 4 mozjs.dll js::Invoke js/src/jsinterp.cpp:680 5 mozjs.dll js::InvokeGetterOrSetter js/src/jsinterp.cpp:717 6 mozjs.dll js::Shape::get js/src/jsscopeinlines.h:279 7 mozjs.dll js_GetPropertyHelper js/src/jsobj.cpp:5929 8 mozjs.dll js::Interpret js/src/jsinterp.cpp:3535 9 mozjs.dll js::RunScript js/src/jsinterp.cpp:585 10 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:648 11 mozjs.dll js::Invoke js/src/jsinterp.cpp:680 12 mozjs.dll JS_CallFunctionValue js/src/jsapi.cpp:5124
Created attachment 566077 [details] [diff] [review] Patch for crash fix by error checking v1.
If we're going to overhaul this, can't we use the prefs helpers we have? Also, rstrong should probably review this, don't believe I'm peer in here.
> Also, rstrong should probably review this, don't believe I'm peer in here. We were both added into peers for /browser Firefox module, so I think we can both do reviews here. > If we're going to overhaul this, can't we use the prefs helpers we have? Wasn't really an overhaul, just added error checking. There are a couple of other uses of the old preferences too in the file that I wasn't going to touch as they already had proper error checking. If you want though I can change everything to use the new preferences code.
Comment on attachment 566077 [details] [diff] [review] Patch for crash fix by error checking v1. Ok, sound reasoning. Let's fix the crash and file a follow up bug on migrating the whole file to the new preferences code. nit - > + rv = prefs->GetBoolPref(PREF_CHECKDEFAULTBROWSER, aResult); > + return rv; return prefs->...
Created attachment 566232 [details] [diff] [review] Patch for crash fix by error checking v2. Fixed nit. Pushed to try: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d3804e9f2b65
https://hg.mozilla.org/mozilla-central/rev/e6f675d14be3
> file a follow up bug on > migrating the whole file to the new preferences code. You won't forget?
It was filed about an hour after that here: bug 693638
Thanks. We usually list followup bugs as dependencies.
OK sounds good will do next time, thanks.
or
