Closed Bug 693110 Opened 10 years ago Closed 10 years ago
Windows Shell Service::Get Should Check Default Browser
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
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->...
Attachment #566077 - Flags: review?(jmathies) → review+
Fixed nit. Pushed to try: https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=d3804e9f2b65
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 10
> 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.
Depends on: 693638
OK sounds good will do next time, thanks.
You need to log in before you can comment on or make changes to this bug.