181752 - Move app support directory to Library/Application Support/Camino

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

[Patrick C. Beard]

I have been reorganizing my disk, and have moved my home directory from one disk
partition to another. I used CCC 2.0 to do this, and each time the home
directory was on a different non-System volume. It appears to be crashing in the
bookmark code. I will enclose a crash log.

Comment 1

[Patrick C. Beard]

Attachment: Crash log

Comment 2

[Patrick C. Beard]

I can't reproduce this with the tip of the Chimera branch.

Comment 3

[Conrad Carlen (not reading bugmail)]

That's because bookmarks was fixed to not crash immediately on not having a
profile. But, the underlying problem is still there. The profile dir is recorded
in the profile registry as an alias. An alias is only valid on the volume on
which it was made - moving the target to another volume breaks the alias.
Chimera, on setting the profile, doesn't do any checking as to whether the
profile dir actually exists or have any recovery code. I'm still of the opinion
that Chimera should not use the profile mgr since it only ever has one profile.
It should just use a standalone nsIDirectoryServiceProvider, make the "profile"
dir relative to "~/Application Support" and not have this problem.

Comment 4

[Conrad Carlen (not reading bugmail)]

But, not a bookmarks problem at this point...

Comment 5

[Brant Gurganus]

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.

Comment 6

[Simon Fraser [no longer active]]

*** Bug 191429 has been marked as a duplicate of this bug. ***

Comment 7

[Conrad Carlen (not reading bugmail)]

The profile directory service provider has been factored out of the profile mgr.
It's in mozilla/profile/dirserviceprovider and builds a static lib which can be
used by an application without the profile mgr. I think Chimera should use this.
It would solve this problem and we would be able to remove a component from the app.

I propose making the new "profile" folder be "~/Library/Application
Support/Chimera" instead of "~/Library/Application
Support/Chimera/Profiles/default/xxxxxxxx.slt

Problem is, copying the contents from old to new location would have to be done
by hand unless we want to keep registry code in Chimera only for this purpose.
The registry code is being moved out of xpcom into the profile mgr so, since I
want to stop using that lib, would be a problem. Or, we could find the old
profile dir by digging around in "default" for "prefs.js"

Or, since we're pre-1.0, is it acceptable to relnote copying your old profile
data to the new location?

Comment 8

[Martin Ler]

chimera 0209
after mooving of my homefolder, chimera lost preferences and is not able to save
new one!! neither preferences, nor bookmarks. Help!!!! Everythink else works
correctly...

Comment 9

[Conrad Carlen (not reading bugmail)]

Attachment: WIP patch to not use profile mgr

The patch makes Camino no longer use the profile mgr. It's silly to use the
profile mgr since its purpose is managing a list of profiles and Camino only
supports having one. Also, the profile mgr tracks profiles using aliases,
leading to problems when moving your home directory, etc. The patch just puts
the data in ~/Application Support/Camino, which can always be found.

The reason it's "In Progress" is that it does not migrate your data over from
Chimera/Profiles/default/xxxxxxxx.slt. For now, it has to be done by hand.

Comment 10

[David Haas (gone, not reading mail)]

Attachment: updated patch

I took the WIP patch and added in "copy from old profile into new directory"
support.

I have no idea if anything beyond the PreferenceManager.mm stuff will apply
cleanly.  I applied everything manually, and to recreate this patch I just
changed the relevant part of diffs.txt.

Comment 11

[David Haas (gone, not reading mail)]

cc'ing myself

and as an aside - since this is just for Camino, go native.  Registry code isn't
required with NSFileManager around.

Comment 12

[Conrad Carlen (not reading bugmail)]

+  NSString *pathString = [[NSString stringWithString:@"~/Library/Application
Support/Chimera/Profiles/default"] stringByExpandingTildeInPath];

Use the folder manager to find ~/Library/Application Support. I doubt that that
string is valid in all locales.

Other than that, looks pretty good. Without reading the actual registry using
nsIRegistry, you could run into troubles:
(1) The profile was moved somewhere else - after all, the registry contains an
alias to it.
(2) The profile dir is is not called "default" but "default-n"
The profile mgr does a CreateUnique when making new dirs and if there were
orphan profiles around, it would have appended.

If we really want to be certain of finding the real location of the current
profile dir, it means reading old registry - unless we're willing to lose some
edge cases.

Comment 13

[David Haas (gone, not reading mail)]

Attachment: even more updated patch

just like last one, but now . .. 
*using carbon file manager to get Application Support Directory.
*iterate through Chimera/Profiles to find the most-recently-modified directory
to count as our old directory.

This'll at least handle the folks that haven't moved anything but do have
default-x as their profile. but no, it still doesn't handle everything.

Comment 14

[S Woodside]

CC'ing pinkerton. I thought the salted directory was for security reasons.

Comment 15

[S Woodside]

There is a VERY good security reason to have salted names for the profile. I'll
summarize, an attacker CAN write to files in the profile directory, e.g., to
your cookies file, to the cache, potentially to other places. An attacker
typically CANNOT read from the file system. However if they know the location of
the cookies/etc. file, they CAN trick you into loading that file which might
contain malicious code that could compromise your system. This patch would
introduce an attack vector that was deliberately blocked in Mozilla.

So IMO, making that change, to eliminate salted profile dirs, would be
boneheaded from a a security standpoint.

More info here: bug 56002.

Comment 16

[Conrad Carlen (not reading bugmail)]

Simon: I'm familiar with bug 56002. That attack (of which there was never a
reported case before salting was implemented, anyway) relies on the attacking
code being able to know the *absolute* path to a file in your profile. On OS X,
that absolute path will contain your account name, which is just as unguessable
as the salt string. That expoit was really only possible on a single-user system
(like Win95) where C:\WINDOWS\Mozilla\Profiles\default was a valid and constant
path. That not being the case on OS X, the salt is useless.

Comment 17

[S Woodside]

Agreed. I withdraw my objection.

Comment 18

[S Woodside]

After installing this, Camino lost my preferences (e.g., home page setting)

Comment 19

[David Haas (gone, not reading mail)]

is home page setting the only thing it lost?  in other words - did your
bookmarks get migrated over?  History?  Etc?

I believe Camino has code to specifically overrides homepage preferences to
bring you to http://www.mozilla.org/projects/camino/homepage.html unless a
certain flag is set on build.

Comment 20

[Mike Pinkerton (not reading bugmail)]

my concern with just copying outright is that the first time the user upgrades
from 0.7, they'll have 50MB of cache in thousands of small files. this will take
a while to copy and they may force-quit right in the middle.

That's why I've been mulling about how to do this effectively, but haven't come
up with any ideas.

Comment 21

[Conrad Carlen (not reading bugmail)]

The simple thing would be to just not copy any item that was named "Cache" or
"Cache.Trash." The patch is using an iterator already - skipping over certain
items would be pretty straightforward. But, my 50MB Cache dir can be copied (by
Finder) in about 5 secs. Assuming NSFileManager is comparable, I'm not sure how
much of an issue this is? I guess we could calc the size of the profile dir and,
if above a certain size, put up a progress dialog.

Comment 22

[Mike Pinkerton (not reading bugmail)]

it copies quickly because you've updated to the new cache on the trunk,
probably, which has many fewer files. remember how long it takes the cache on
the branch to "clean up the cache" at startup? it can hang for tens of minutes
on slower macs iterating over every file and deleting it.

that's what i'm worried about.

Comment 23

[David Haas (gone, not reading mail)]

It's easy enough to change from "copyPath:toPath" to "movePath:toPath", which I
believe would be pretty much instantaneous no matter how large the cache is. 

Of course, if you downgrade after upgrading, your bookmarks will appear to be
lost  . . . 

Comment 24

[Mike Pinkerton (not reading bugmail)]

right. i think there will be people who downgrade back to 0.7, esp after using
pre-0.8 trunk builds. we cannot hose them by moving the profile.

Comment 25

[David Haas (gone, not reading mail)]

Attachment: yet another update

Updated the "even more updated patch" so that cache folders get moved but
everything else gets copied to the new profile directory. All the caveats of
the last past still apply - I have no idea if anything other than
PreferenceManager.mm will apply cleanly.

Comment 26

[David Haas (gone, not reading mail)]

Attachment: fixed patch

arg - sorry about this - I didn't update before the last patch and apparently
somebody's worked on PreferenceManager.mm.

Comment 27

[Mike Pinkerton (not reading bugmail)]

i don't think it's necessary to move the cache dir. just don't copy it.

Comment 28

[Mike Pinkerton (not reading bugmail)]

there are still a few things wrong with this latest patch:

- can't "Cache" be localized by mozilla? did any of the chimeral10n projects
include a localized mozilla?
- we perform the conversion if we don't find a "history.dat" file. it's
perfectly valid not to have one (if someone tosses it to get rid of their
history wholesale). This shouldn't re-trigger a conversion, and more
importantly, will clobber new data with the data from the 0.7 profile. Bad news.
- i say don't copy or move the cache folder (whatever we determine it to be
called, just skip it wholesale)
- conrad says that there's a way to read the registry inside the old profile
folder in order to get the salted dir. this would be much better than the adhoc
method here. i don't think it lives in the profile library, i think it's a
separate stub library.

Comment 29

[David Haas (gone, not reading mail)]

To answer your questions:

1) Did somebody localize Cache - I don't know.  And if they did - why?  It's a
freakin' hidden directory.  I'd say it'll just skip directories, but then it'd
leave out "chrome".  Which appears to be empty.  OK, it'll skip directories on
copy.  If there's a problem with this, let me know.

2) Sure, we can try something besides history.dat.  How about secmod.db instead?
 Or maybe bookmarks.plist?  A combo of the two or more?  Let me know what works.

3) I know next-to-nothing about Moz internals, and don't have the time to figure
them out right now. If nsIRegistry is located in a teeny tiny library someplace,
feel free to let me know which one & I'll try to figure out what call to make.
Or, maybe you could leave in the profile library for 0.8, and pull it for 0.9
(assuming no one would jump from 0.7 to 0.9 without going through 0.8)?  If you
want to do that, then I can steal from what was here before without a problem.

Comment 30

[Conrad Carlen (not reading bugmail)]

1) "Cache" is never localized, It's set here:
http://lxr.mozilla.org/seamonkey/source/netwerk/cache/src/nsDiskCacheDevice.cpp#1129
to this nicely hardcoded string. In mozilla, it can be set to any arbitrary dir
by way of a pref. Since Camino has never supported setting that pref, we'll
never see that situation.

2) Let's read the registry and avoid all that.

3) There is way to do this that wouldn't rid us of the profile mgr dll: Use
nsIProfile::GetCurrrentProfile(), and use that name with 
nsIProfileInternal::GetProfileDir(). That will return the old profile dir in few
lines of code and no groveling around for files that might not exist. Maybe we
could do that short term and then, if I ever have spare time, I can crib the
nescesary code from nsProfileAccess.cpp to read the data without using the whole
profile mgr.

Comment 31

[David Haas (gone, not reading mail)]

Attachment: Non-working patch

This was my final attempt to patch PreferenceManager.mm.  It doesn't compile,
so clearly I've done something wrong.  I'm going to rely on someone else to
figure out what.

Things that may be good:  

*Patch checks to see if "new" profile directory contains 
bookmarks.plist, cookies.txt, cookperm.txt, history.dat, and prefs.js.	If any
of them are there - it doesn't import.	I haven't tested this yet.  Why?  The
patch won't compile.

*Patch copies files from old profile dir, and skips directories.  Also
untested.

*Patch attempts to read old profile dir using nsIProfile & nsIProfileInternal,
as suggested.  

Things that are bad:

It doesn't compile.  Why?  I get a "no matching function for call to
`nsDerivedSafe<nsIProfile>::GetProfileDir(nsXPIDLString&,
nsGetterAddRefs<nsIFile>)" error.  To set these calls, I pretty much copied &
pasted from other locations in the source where they've been used before.  I
missed something, but I don't know what.  I've tried including more headers,
which didn't work.  I've played around with different calls - still doesn't
work.  I doubt if I got past the first line the second line would work.
(particularly the do_queryinterface call).

I'm going to eat some chinese food and move on with my life.  I hope this helps
whoever ends up getting this thing fixed.

Comment 32

[Mike Pinkerton (not reading bugmail)]

*** Bug 216244 has been marked as a duplicate of this bug. ***

Comment 33

[Mike Pinkerton (not reading bugmail)]

fwiw, it builds after changing that line to:

if
(NS_FAILED(profileInternal->GetProfileDir(oldProfileName.get(),getter_AddRefs(oldProfileDir))))

you were calling a method on the wrong interface (doh!). 

Comment 34

[S Woodside]

Sorry I wasn't CC'd. 

Re comment #19: It lost all of the prefs stored using the Cocoa defaults system,
so my home page, my custom fonts settings, my tabs settings. Everything else
copied over fine.

Re: Cache dir, I agree with Mike, dump it.

Does that, with the correction by Mike, bring this up to speed? This is blocking
212630 the bookmarks manager, which I want to get in.

Comment 35

[S Woodside]

changed summary (was Crash after moving home directory (Chimera 0.6))

Comment 36

[S Woodside]

Localizing "Cache" directory is unnecessary. David has resolved the issue of
which file to use and uses nsIRegistry now. It no longer copies the Cache
directory. People who downgrade after updating will still work. Mike has a patch
that buiids for him.

Is everyone happy now?

Comment 37

[Mike Pinkerton (not reading bugmail)]

uh, no, cuz the patch doesn't even come close to working. all i said was that it
builds.

Comment 38

[S Woodside]

don;'t think this bug is for the original crasher any more.

Comment 39

[Conrad Carlen (not reading bugmail)]

Attachment: new patch

This patch rids us of using the profile dylib altogether. It uses nsIRegistry
to read the profile location out of the existing profile registry. It uses the
standalone directory service provider that the profile mgr does (and exports
for embedding apps that have only 1 profile). It checks for the existance of
prefs.js in the new profile location and, if not present, migrates the old -
skipping the Cache dir. BTW, the patch contains a change where I added
USE_GCC3=YES so that the project can be built within PB when running on
Panther. That should probably be done for all the targets as well.

Comment 40

[Conrad Carlen (not reading bugmail)]

This patch will require a change to Tinderbox scripts for this:
> Deleting /Users/cltbld/Library/Application Support/Chimera...
Also, if the scripts set prefs by writing to prefs.js?

Who can help with the Tinderbox changes?

Comment 41

[Ludovic Hirlimann [:Usul]]

Pinkerton should update the tinbderboxes to Panther Next week or so, might be a
great time to do this too.

Comment 42

[Mike Pinkerton (not reading bugmail)]

landed. thanks conrad!

Comment 43

[Ludovic Hirlimann [:Usul]]

Comment on attachment 140020 [details] [diff] [review]
new patch

Mike some people are using the query for bugs needing reviews. Please take the
time to remove review flags.