Closed Bug 233850 Opened 21 years ago Closed 20 years ago

localstore.rdf is not created in user profile if it doesn't present in application default/profile dir

Categories

(Core Graveyard :: Profile: BackEnd, defect)

x86
All
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7beta

People

(Reporter: ph0enix, Assigned: benjamin)

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040113

I've created mozilla based application and noticed, that if there is no
<app_dir>/default/profile/localstore.rdf file, it is not creating in user
profile dir, so persistent attributes are not stored

Reproducible: Always
Steps to Reproduce:
1. create mozilla based app (using toolkit/xre and xpfe/bootstrap from mozilla
sources)
2. make sure there is no <app_dir>/default/profile/localstore.rdf
3. make window size persistent
4. run app, change window size and close app
5. run it again - size is not restored since there is no localstore.rdf in user
porfile

Actual Results:  
persistent attributes are not persistent

Expected Results:  
attributes to be persistent
Code to create the file is at:
http://lxr.mozilla.org/mozilla/source/rdf/datasource/src/nsLocalStore.cpp#406

Do you have a debug build (or can you create one) and step through this function
to see what's going wrong?
(In reply to comment #1)
> Code to create the file is at:
> http://lxr.mozilla.org/mozilla/source/rdf/datasource/src/nsLocalStore.cpp#406
> 
> Do you have a debug build (or can you create one) and step through this function
> to see what's going wrong?

it gets into 
1. NS_GetSpecialDirectory(NS_APP_LOCALSTORE_50_FILE, getter_AddRefs(aFile));
2. nsDirectoryService::Get("LclSt", ...)
3. nsSupportsArray::EnumerateBackwards
4. nsProfileDirServiceProvider::GetFile, match inAtom == sApp_LocalStore50
5. nsProfileDirServiceProvider::EnsureProfileFileExists gets to the end and calls
defaultsFile->CopyTo(destDir, nsString());
where defaultsFile is <app_dir>/default/profile/localstore.rdf and destDir is
correct path to profile
6. fails in nsLocalFile::ResolveAndStat trying to stat
<app_dir>/default/profile/localstore.rdf with rv=NS_ERROR_FILE_NOT_FOUND
also fails in XREDirProvider and in nsAppFileLocationProvider

so, if there is no <app_dir>/default/profile/localstore.rdf, it won't be created
in user profile. but if I create even empty localstore.rdf, after exiting user's
localstore.rdf will be filled with necessary data

another branch:
3. FindProviderFile
4. nsDirectoryService::GetFile
creates nsIAtom from "LclSt"
compares it with a number of nsDirectoryService::s*
nothing finds


---
I suppose NS_GetSpecialDirectory should return some code non-matched with
NS_FAILED if file is not exists, because there is file existing check in
nsLocalStore.cpp#417 and it seems will create localstore.rdf
Assignee: nobody → bsmedberg
Status: UNCONFIRMED → ASSIGNED
Attachment #141242 - Flags: superreview?(darin)
Attachment #141242 - Flags: review?(ccarlen)
Comment on attachment 141242 [details] [diff] [review]
profile dir service provider doesn't need to check for existence of localstore file

This does more than eliminate the existance check. With this patch,
localstore.rdf will never be copied from the defaults, even when it exists.
(Except by some code in nsProfile.cpp, which we shouldn't depend upon)

Instead, I would do this:

       rv = localFile->AppendNative(LOCAL_STORE_FILE_50_NAME);
       if (NS_SUCCEEDED(rv))
-	 rv = EnsureProfileFileExists(localFile, domainDir);
+	 (void)EnsureProfileFileExists(localFile, domainDir);

That will still allow the file to copied if it exists but will succeed as long
as it returns a valid file location.

This is one unfortunate thing about directory service that wasn't caught before
being frozen: there should be a "createIfNotExist" flag with which callers
could make this explicit :-/
Attachment #141242 - Flags: review?(ccarlen) → review-
Attachment #141242 - Flags: superreview?(darin)
Attached patch take 2 (obsolete) — Splinter Review
Attachment #141242 - Attachment is obsolete: true
Attachment #141987 - Flags: superreview?(darin)
Attachment #141987 - Flags: review?(ccarlen)
       rv = localFile->AppendNative(LOCAL_STORE_FILE_50_NAME);
-      if (NS_SUCCEEDED(rv))
-        rv = EnsureProfileFileExists(localFile, domainDir);
-    }
+      EnsureProfileFileExists(localFile, domainDir);

Why disregard the rv from AppendNative before the call to
EnsureProfileFileExists? Also, if you're ignoring an rv for a good reason (this
bug), cast it to void to make it explicit. Better yet, cast it to void and leave
a comment as to why.
Comment on attachment 141987 [details] [diff] [review]
take 2

minusing based on ccarlen's comments.
Attachment #141987 - Flags: superreview?(darin) → superreview-
Attachment #141987 - Attachment is obsolete: true
Attachment #142328 - Flags: superreview?(darin)
Attachment #142328 - Flags: review?(ccarlen)
Attachment #141987 - Flags: review?(ccarlen)
Comment on attachment 142328 [details] [diff] [review]
updated with (void) etc

r=ccarlen
Attachment #142328 - Flags: review?(ccarlen) → review+
Attachment #142328 - Flags: superreview?(darin) → superreview+
fixed on trunk
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.7beta
Does this mean we can clean out all the junk and not ship localstore.rdf?
Neil: no... at least, not without some work. The localstore.rdf files that we
ship contain various default values that (as I understand it) are not
auto-generated if they don't exist. It's weird.
Actually I think they could all be auto-generated, it's just that nobody wrote
the xul correctly before...
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: