mozilla -CreateProfile doesn't create prefs.js for subsequent profiles

VERIFIED FIXED in mozilla1.2beta

Status

--
major
VERIFIED FIXED
16 years ago
3 years ago

People

(Reporter: netscape, Assigned: ccarlen)

Tracking

Trunk
mozilla1.2beta
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

When creating new profiles using ./mozilla -CreateProfile <profilename>, the new
profile doesn't contain the prefs.js file.  At first, I thought that this was
just a bug in the static build but I see the same problem in my non-static
builds.  Since the tinderbox will fail the runtime tests if prefs.js isn't
present, I would expect more tinderboxes to show bustage but it appears to be
just select builds that don't work.  If I run ./mozilla -P <profilename>, then
the files show up but the tinderbox script immediately checks for the prefs file
after creating the profile.

Comment 1

16 years ago
adding bnesse, I seem to remember that prefs.js isn't always created
with a new profile ? until it is used?  I don't remember.  Brian?
(Assignee)

Comment 2

16 years ago
It's not copied from the defaults. When a new profile is made, the contents of
<bin>/defaults/profile are copied to the new profile dir. prefs.js is not part
of the defaults. If prefs.js has been showing up in new profiles, it's probably
because something is setting a pref and calling SavePrefFile() early in the
startup process? Anyway, creation of prefs.js is not directly a part of making a
new profile.

Comment 3

16 years ago
Yes, this caused me no end of grief on the Tinderboxen which run tests when I
tried to land the safe save code. The -CreateProfile option follows a codepath
which either 1) does not instantiate a PrefService or 2) never loads/saves the
preferences. This makes sense as neither of these things should be needed to
create an empty profile.

The (empty) prefs.js file will be created the first time the application is run
using the created profile... on exit it will write the current preferences state
(which if you simply launch and quit will be approximately 8 prefs).
I briefly traced the callpath and eventually we wind up in
nsProfile::StartupWithArgs which will return NS_ERROR_ABORT if there's no
current profile (which implies -CreateProfile is being used).   This return
value gets passed back up and eventually winds up as the return value of main1
which causes mozilla to skip the SavePrefFile call on shutdown.

The only thing that pops in mind is that GetCurrentProfile isn't returning 0 but
I'll have to hack a tinderbox with a working test to see what it says.
This is weird.  I stepped through a -CreateProfile run on myotonic with profile,
appshell & apprunner recompiled with debug info.  When I run -CreateProfile
manually, remembering to reset $HOME to the tinderbox dir, it doesn't create the
prefs.js file.  When the command is run from the script, it does. 

AH HA!  For giggles, I removed the tinderbox's entire .mozilla directory.  Now,
when I run -CreateProfile, it creates a prefs.js file for the *first* profile
that gets created.  

Eventually, nsPrefService::ReadUserPrefs gets called with a non-null arg via
nsProfile::LoadNewProfilePrefs.  This causes SavePrefFile to be called from
nsPrefService::UseDefaultPrefFile.  I still don't understand why it's not
creating a prefs.js file for each profile.  
A printf in ReadUserPrefs() shows that UseDefaultProfile() is returning the path
to the prefs.js file of the first profile rather than the current profile.  It
looks like there's a bug in NS_GetSpecialDirectory(). 

Comment 7

16 years ago
seawood: any idea why this is only happening on the static builds?  I looked at
the tinderbox scripts for a while and ? I am a bit confused also.
It's not just the static builds.  Myotonic is not doing a static build.  

I think we are seeing a different problem with the tinderbox static builds where
the profile isn't being created at all.  Note that my local static build creates
the profile (sans prefs.js since I already have a primary profile) just fine.
For whatever reason, the static build on balsa is failing in main1() trying to
create an instance of the startupNotifier.  It never even gets to processing the
cmdline args.
(Assignee)

Comment 10

16 years ago
It's not a bug in NS_GetSpecialDirectory(). When LoadNewProfilePrefs() is called
in creating a new profile, nsProfile::SetCurrentProfile() has not been called so
the profile-relative file locations returned by NS_GetSpecialDirectory() are not
for the profile which was just created. The question is: why is
LoadNewProfilePrefs() called in creating a new profile?
According to lxr, that's the only place that LoadNewProfilePrefs is called and
it's been there awhile.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/profile/src/nsProfile.cpp#882
Created attachment 99221 [details] [diff] [review]
Set the current profile before loading prefs

Calling nsProfile::SetCurrentProfile fixes the problem of prefs.js only being
created for the first profile.
(Assignee)

Comment 13

16 years ago
-CreateProfile is supposed to simply create a new profile and exit.

> Since the tinderbox will fail the runtime tests if prefs.js isn't present

Why is that? What is being tested for? I'm a little leary of changing this for
the sake of the tinderbox scripts. Can we instead have the scripts pass an arg
which explicitly does whatever is needed (create a new profile, make it current,
save some prefs to it) instead of changing the behavior of -CreateProfile?
> Why is that? What is being tested for? 

The tinderbox script looks for prefs.js to find the actual location of the
profile (obsfucated due to the .slt) and then it adds a handful of preferences
to that file.

> I'm a little leary of changing this for the sake of the tinderbox scripts. 

Well, the current behavior is inconsistent at best.  Due to the inconsistency of
  the prefs.js generation, I thought that this would break the tinderboxes. 
Since most of the tinderboxes do not use multiple profiles, they should be
unaffected by this bug.

> Can we instead have the scripts pass an arg which explicitly does whatever is 
> needed (create a new profile, make it current, save some prefs to it) instead 
> of changing the behavior of -CreateProfile?

If the actual behavior of -CreateProfile changed our from underneath the
tinderbox scripts, then the tb scripts will have to adjust accordingly. 
However, it doesn't sound like the behavior has changed from when the tb scripts
were written.  I think that adding a new runtime option just for the sake of
tinderbox is overkill.  And it definitely shouldn't be done until after we
determine what the correct behavior for -CreateProfile (to generate prefs.js or
not) should be.

Summary: mozilla -CreateProfile doesn't create prefs.js → mozilla -CreateProfile doesn't create prefs.js for subsequent profiles
(Assignee)

Comment 15

16 years ago
Comment on attachment 99221 [details] [diff] [review]
Set the current profile before loading prefs

>Index: profile/src/nsProfile.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/profile/src/nsProfile.cpp,v
>retrieving revision 1.267
>diff -u -r1.267 nsProfile.cpp
>--- nsProfile.cpp	2002/09/07 17:12:07	1.267
>+++ nsProfile.cpp	2002/09/14 22:05:01
>@@ -882,6 +882,7 @@
>             if (NS_SUCCEEDED(rv)) {
>                 *profileDirSet = PR_TRUE;
>                 mCurrentProfileAvailable = PR_TRUE;

Since you're calling SetCurrentProfile(),  mCurrentProfileAvailable = PR_TRUE
can come out. SetCurrentProfile() sets that.

>+                gProfileDataAccess->SetCurrentProfile(currProfileName.get());
> 
>                 // Need to load new profile prefs.
>                 rv = LoadNewProfilePrefs();
Attachment #99221 - Flags: review+
Created attachment 99618 [details] [diff] [review]
Updated with conrad's change
Attachment #99221 - Attachment is obsolete: true
(Assignee)

Comment 17

16 years ago
Comment on attachment 99618 [details] [diff] [review]
Updated with conrad's change

r=ccarlen
Attachment #99618 - Flags: review+
Comment on attachment 99618 [details] [diff] [review]
Updated with conrad's change

Transferring r=

Comment 19

16 years ago
Comment on attachment 99618 [details] [diff] [review]
Updated with conrad's change

sr=alecf
Attachment #99618 - Flags: superreview+
Patch has been checked in.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.2beta
I had to revert back to the first patch as removing the mCurrentProfileAvailable
= PR_TRUE caused the prefs.js file to not be created at all....which broke the
tinderboxes.
(Assignee)

Comment 22

16 years ago
> removing the mCurrentProfileAvailable
= PR_TRUE caused the prefs.js file to not be created at all

Ack - I need to see why.

Updated

16 years ago
QA Contact: ktrina → gbush

Comment 23

16 years ago
verified Linux 2003032505
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.