"Failed to setup pref service" on first-run from ResetAndReadUserPrefs

VERIFIED FIXED in mozilla14

Status

()

Core
Preferences: Backend
P3
normal
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla14
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox13 wontfix)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
(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?
...
(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

6 years ago
Created attachment 605832 [details] [diff] [review]
(Av1) Document UseDefaultPrefFile() and fix its nsresult value
[Checked in: Comment 4]

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

6 years ago
Priority: -- → P3
Target Milestone: --- → mozilla14

Updated

6 years ago
Attachment #605832 - Flags: review?(benjamin) → review+

Comment 3

6 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

6 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

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
(Assignee)

Comment 5

6 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?
(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

6 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

6 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

6 years ago
status-firefox13: affected → wontfix
(Assignee)

Updated

5 years ago
No longer blocks: 734883
(Assignee)

Updated

5 years ago
Blocks: 734883
You need to log in before you can comment on or make changes to this bug.