Closed
Bug 735573
Opened 12 years ago
Closed 12 years ago
"Failed to setup pref service" on first-run from ResetAndReadUserPrefs
Categories
(Core :: Preferences: Backend, defect, P3)
Core
Preferences: Backend
Tracking
()
VERIFIED
FIXED
mozilla14
Tracking | Status | |
---|---|---|
firefox13 | --- | wontfix |
People
(Reporter: sgautherie, Assigned: sgautherie)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
3.05 KB,
patch
|
benjamin
:
review+
akeybl
:
approval-mozilla-aurora-
|
Details | Diff | Splinter Review |
(Moved from bug 734883 comment 15.) UseDefaultPrefFile() code is: http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp#586 { 593 rv = NS_GetSpecialDirectory(NS_APP_PREFS_50_FILE, getter_AddRefs(aFile)); 594 if (NS_SUCCEEDED(rv)) { 595 rv = ReadAndOwnUserPrefFile(aFile); 596 // Most likely cause of failure here is that the file didn't 597 // exist, so save a new one. mUserPrefReadFailed will be 598 // used to catch an error in actually reading the file. 599 if (NS_FAILED(rv)) { 600 rv2 = SavePrefFileInternal(aFile); 601 NS_ASSERTION(NS_SUCCEEDED(rv2), "Failed to save new shared pref file"); 602 } 603 } 604 605 return rv; } Startup crash tracking does: http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/xre/nsXREDirProvider.cpp#736 { 742 nsresult rv = mozilla::Preferences::ResetAndReadUserPrefs(); 743 if (NS_FAILED(rv)) NS_WARNING("Failed to setup pref service."); } At each first startup of a new profile, the latter warning is reported. I assume the startup crash tracking feature is then implicitly disabled for that startup. Should rv be set to NS_OK when NS_SUCCEEDED(rv2)? Should we (re)try to read the new file? ...
Comment 1•12 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from comment #0) > (Moved from bug 734883 comment 15.) > > UseDefaultPrefFile() code is: > http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/ > Preferences.cpp#586 > { > 593 rv = NS_GetSpecialDirectory(NS_APP_PREFS_50_FILE, > getter_AddRefs(aFile)); > 594 if (NS_SUCCEEDED(rv)) { > 595 rv = ReadAndOwnUserPrefFile(aFile); > 596 // Most likely cause of failure here is that the file didn't > 597 // exist, so save a new one. mUserPrefReadFailed will be > 598 // used to catch an error in actually reading the file. > 599 if (NS_FAILED(rv)) { > 600 rv2 = SavePrefFileInternal(aFile); > 601 NS_ASSERTION(NS_SUCCEEDED(rv2), "Failed to save new shared pref > file"); > 602 } > 603 } > 604 > 605 return rv; > } Getting rid of rv2 and setting the return value of SavePrefFileInternal to rv would get rid of the warning for new profiles. I'm not sure why we are keeping the return values separate here. > Startup crash tracking does: > http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/xre/ > nsXREDirProvider.cpp#736 > { > 742 nsresult rv = mozilla::Preferences::ResetAndReadUserPrefs(); > 743 if (NS_FAILED(rv)) NS_WARNING("Failed to setup pref service."); > } > > At each first startup of a new profile, the latter warning is reported. > I assume the startup crash tracking feature is then implicitly disabled for > that startup. The startup crash count isn't incremented on the first run in a new profile anyways since it needs toolkit.startup.recent_crashes to be set from the previous startup. This warning actually isn't specific to startup crash tracking, it's just that no warning was reported before since it was part of the profile-do-change observer. > Should rv be set to NS_OK when NS_SUCCEEDED(rv2)? I think we could replace rv2 with rv. > Should we (re)try to read the new file? I think it's fine without since this was probably failing before in Observe.
Priority: P3 → --
Summary: Review/Improve interaction between UseDefaultPrefFile() and startup crash tracking → "Failed to setup pref service" on first-run from ResetAndReadUserPrefs
Assignee | ||
Comment 2•12 years ago
|
||
Untested (yet). (In reply to Matthew N. [:MattN] from comment #1) > > Should rv be set to NS_OK when NS_SUCCEEDED(rv2)? By code inspection, that should be enough to fix ResetAndReadUserPrefs(), which is the only caller of this case. > I think we could replace rv2 with rv. I can do it if you think that is wanted, but ftb I assumed rv2 had been used on purpose.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #605832 -
Flags: review?(benjamin)
Assignee | ||
Updated•12 years ago
|
Priority: -- → P3
Target Milestone: --- → mozilla14
Updated•12 years ago
|
Attachment #605832 -
Flags: review?(benjamin) → review+
Comment 3•12 years ago
|
||
Try run for d7cd48e5d178 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=d7cd48e5d178 Results (out of 223 total builds): exception: 2 success: 178 warnings: 28 failure: 15 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sgautherie.bz@free.fr-d7cd48e5d178
Assignee | ||
Comment 4•12 years ago
|
||
Comment on attachment 605832 [details] [diff] [review] (Av1) Document UseDefaultPrefFile() and fix its nsresult value [Checked in: Comment 4] https://hg.mozilla.org/mozilla-central/rev/2cec1f79a141
Attachment #605832 -
Attachment description: (Av1) Document UseDefaultPrefFile() and fix its nsresult value → (Av1) Document UseDefaultPrefFile() and fix its nsresult value
[Checked in: Comment 4]
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Assignee | ||
Comment 5•12 years ago
|
||
Comment on attachment 605832 [details] [diff] [review] (Av1) Document UseDefaultPrefFile() and fix its nsresult value [Checked in: Comment 4] [Approval Request Comment] Regression caused by (bug #): (revealed by) Bug 294260. User impact if declined: Not sure, but wrong interaction (with a 'WARNING') with new feature at least on all our boxes. Testing completed (on m-c, etc.): Try, this comment 4. Risk to taking this patch (and alternatives if risky): (Very) Low. String changes made by this patch: None.
Attachment #605832 -
Flags: approval-mozilla-aurora?
Comment 6•12 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from comment #5) > User impact if declined: Not sure, but wrong interaction (with a 'WARNING') > with new feature at least on all our boxes. This is for first-run only and release builds don't output NS_WARNINGs.
Assignee | ||
Comment 7•12 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=10300331&tree=Firefox&full=1 Rev3 Fedora 12 mozilla-central debug test mochitests-3/5 on 2012-03-22 19:00:02 PDT for push 2cec1f79a141 V.Fixed
Status: RESOLVED → VERIFIED
Comment 8•12 years ago
|
||
Comment on attachment 605832 [details] [diff] [review] (Av1) Document UseDefaultPrefFile() and fix its nsresult value [Checked in: Comment 4] [Triage Comment] Given Comment 6, this doesn't appear necessary for Aurora 13.
Attachment #605832 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Assignee | ||
Updated•12 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•