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

Status: RESOLVED FIXED

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Maks Polunin]

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

Comment 1

[Benjamin Smedberg]

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?

Comment 2

[Maks Polunin]

(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

Comment 3

[Benjamin Smedberg]

Attachment: profile dir service provider doesn't need to check for existence of localstore file

Excellent debugging.

Comment 4

[Conrad Carlen (not reading bugmail)]

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 :-/

Comment 5

[Benjamin Smedberg]

Attachment: take 2

Comment 6

[Conrad Carlen (not reading bugmail)]

       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 7

[Darin Fisher]

Comment on attachment 141987 [details] [diff] [review]
take 2

minusing based on ccarlen's comments.

Comment 8

[Benjamin Smedberg]

Attachment: updated with (void) etc

Comment 9

[Conrad Carlen (not reading bugmail)]

Comment on attachment 142328 [details] [diff] [review]
updated with (void) etc

r=ccarlen

Comment 10

[Benjamin Smedberg]

fixed on trunk

Comment 11

[[e-mail]]

Does this mean we can clean out all the junk and not ship localstore.rdf?

Comment 12

[Benjamin Smedberg]

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.

Comment 13

[[e-mail]]

Actually I think they could all be auto-generated, it's just that nobody wrote
the xul correctly before...