Last Comment Bug 693110 - crash nsWindowsShellService::GetShouldCheckDefaultBrowser
: crash nsWindowsShellService::GetShouldCheckDefaultBrowser
Status: RESOLVED FIXED
: crash
Product: Firefox
Classification: Client Software
Component: Shell Integration (show other bugs)
: Trunk
: x86 Windows 7
: -- critical (vote)
: Firefox 10
Assigned To: Brian R. Bondy [:bbondy]
:
Mentors:
Depends on: 693638
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-08 17:20 PDT by Makoto Kato [:m_kato]
Modified: 2012-06-14 06:28 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for crash fix by error checking v1. (2.28 KB, patch)
2011-10-10 16:42 PDT, Brian R. Bondy [:bbondy]
jmathies: review+
Details | Diff | Review
Patch for crash fix by error checking v2. (2.25 KB, patch)
2011-10-11 08:41 PDT, Brian R. Bondy [:bbondy]
netzen: review+
Details | Diff | Review

Description Makoto Kato [:m_kato] 2011-10-08 17:20:29 PDT
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
Comment 1 Brian R. Bondy [:bbondy] 2011-10-10 16:42:20 PDT
Created attachment 566077 [details] [diff] [review]
Patch for crash fix by error checking v1.
Comment 2 Jim Mathies [:jimm] 2011-10-11 06:25:43 PDT
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.
Comment 3 Brian R. Bondy [:bbondy] 2011-10-11 06:30:47 PDT
> 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 4 Jim Mathies [:jimm] 2011-10-11 07:43:11 PDT
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->...
Comment 5 Brian R. Bondy [:bbondy] 2011-10-11 08:41:32 PDT
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
Comment 6 Ed Morley [:emorley] 2011-10-14 03:54:05 PDT
https://hg.mozilla.org/mozilla-central/rev/e6f675d14be3
Comment 7 :Ms2ger 2011-10-14 05:06:09 PDT
> file a follow up bug on
> migrating the whole file to the new preferences code. 

You won't forget?
Comment 8 Brian R. Bondy [:bbondy] 2011-10-14 05:07:59 PDT
It was filed about an hour after that here: bug 693638
Comment 9 :Ms2ger 2011-10-14 05:47:51 PDT
Thanks. We usually list followup bugs as dependencies.
Comment 10 Brian R. Bondy [:bbondy] 2011-10-17 06:55:46 PDT
OK sounds good will do next time, thanks.

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