Closed
Bug 506814
Opened 15 years ago
Closed 15 years ago
Get rid of/Change GetPersistentDescriptor/SetPersistentDescriptor
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: vlad, Assigned: adw)
References
Details
(Whiteboard: [ts])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jaas
:
review+
|
Details | Diff | Splinter Review |
We use Get/SetPersistentDescriptor in a bunch of prefs, xpti, etc. code to obtain a "persistent descriptor" for a particular file. On all platforms other than OSX, it's just a plain pathname. On OSX, it's some magic OS Alias reference, which is able to track files across moves, renames, whatever; it's also the cause of base64-encoded filename-blobs in our OSX profile dirs.
The other platforms get away with using native pathnames, and nothing blows up if the user moves or renames one of those files; given that they're generally in the profile dir, they probably won't.
So we should:
- change nsLocalFileOSX's Get/SetPersistent* to just use the pathname (keeping the parsing in Set for backwards compat, since it already handles pathnames that start with /)
and/or
- stop using the PersistentDescriptor stuff and just use pathnames everywhere (Windows does some stuff with UTF8 vs non UTF8 though, so maybe that's harder).
Going through this stuff makes us hit Carbon file URLs and end up super slow.
Reporter | ||
Updated•15 years ago
|
Whiteboard: [ts]
Updated•15 years ago
|
Hardware: x86 → All
Assignee | ||
Comment 2•15 years ago
|
||
Vlad, is this comment relevant? A reason aliases are used on OS X rather than paths?
http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsIFile.idl#231
Assignee | ||
Comment 3•15 years ago
|
||
This is too simplistic, right? Who can review this?
Assignee | ||
Updated•15 years ago
|
Attachment #406075 -
Flags: review?(vladimir)
Comment on attachment 406075 [details] [diff] [review]
patch
You can't change those methods. Their behavior is part of the API, with your modifications those methods do not create persistent descriptors. You should change the consumers to use path methods.
Attachment #406075 -
Flags: review-
Assignee | ||
Updated•15 years ago
|
Attachment #406075 -
Flags: review?(vladimir)
Assignee | ||
Comment 5•15 years ago
|
||
Thanks Josh.
Recording my thoughts: Existing prefs (e.g., complex prefs of type nsILocalFile [1]), xpti.dat entries, etc. already contain base64-encoded aliases. If we change over to paths, we don't know whether a given pref value, xpti.dat entry, etc. is an alias or path -- whether to set persistentDescriptor or call initWithPath. A simple solution is to first assume it's an alias, and if SetPersistentDescriptor fails because the data is not base64-encoded, then it must be a path. (OS X's InitWithPath could do this check, or a new method InitWithPathOrPersistentDescriptor, or worst rewriting all consumers to do it themselves.) Unix and Windows won't feel the difference, but there's extra cost at runtime on OS X. Or, is there some migration path to hook into, and if so, can the locations on disk of all existing base64-encoded aliases be determined during migration? Is this even worth worrying about?
[1] http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/nsPrefBranch.cpp#281
Reporter | ||
Comment 6•15 years ago
|
||
(In reply to comment #2)
> Vlad, is this comment relevant? A reason aliases are used on OS X rather than
> paths?
> http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsIFile.idl#231
That was true back in MacOS pre-X days; I honestly don't think we care any more, and shouldn't treat OSX any different than we do any other Unix system.
Comment 7•15 years ago
|
||
I agree with vlad: paths should *be* the persistent descriptors we use. The only issue I have is whether we need to provide any upgrade path, in case these persistent descriptors are stored anywhere in prefs that persists between versions. It's not an issue for fastload or the other caches.
Assignee | ||
Comment 8•15 years ago
|
||
Are prefs really the only place persistent descriptors are persistently stored? Looking through mxr they are also potentially stored in at least:
* extensions.ini
* download manager DB: mime type => path of preferred application
* profiles.ini
* nsNetscapeProfileMigratorBase, nsProfileMigrator get descriptors from some NSPR registry
1) Are these all caches?
2) If descriptors are persistently stored, and it appears they certainly can be, then there's no choice but to do a migration (or check at every Set per comment 5). Where's the best place to do it? Is it possible to hook into .autoreg/compatibility.ini invalidation, or can the updater do stuff like this?
Comment 9•15 years ago
|
||
I don't think it matters. A path-style persistent descriptor will always start with /. The old style will always start with an alphanumeric. Just detect which kind you have in ReadPersistentDescriptor, keep the code to load the old kind, but only write the new kind.
Assignee | ||
Comment 10•15 years ago
|
||
This should be it then...
Attachment #406075 -
Attachment is obsolete: true
Attachment #406364 -
Flags: review?(joshmoz)
Attachment #406364 -
Flags: review?(joshmoz) → review+
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Comment 11•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in
before you can comment on or make changes to this bug.
Description
•