Closed Bug 863570 Opened 7 years ago Closed 6 years ago

OS.Constants.Path.profileDir is not available on startup

Categories

(Toolkit :: OS.File, defect)

defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: Gijs, Unassigned)

References

Details

The profile directory is available on startup. This was added in bug 277578. The fact that we're not using it is breaking _SessionFile.jsm when used on startup (for sessionstore migration in Firefox reset, for bug 833943).

I'm not sure whether we can use ProfDS always or if there's some reason to switch to what ProfD gives us when it's available. Benjamin, can you elucidate this? I'm happy to come up with a patch if I know what the right solution is.
Flags: needinfo?(benjamin)
Do you have a testcase?
Since bug 810543, normally, profileDir is available as soon as ProfD is available.
When they both are available, ProfD and ProfDS will always be the same value. However, there is a significant difference in what they *mean*:

When ProfD is available, it means that the profile-after-change notification has been sent and the profile is ready for use.

Before that time, ProfDS is available, but the profile may be in a state of migration and nobody should be touching that directory without specific knowledge that they won't mess up migration etc.

So in general I don't think that Path.profileDir should use ProfDS, and the migration code needs specific understanding of ProfDS.
Flags: needinfo?(benjamin)
(In reply to David Rajchenbach Teller [:Yoric] from comment #1)
> Do you have a testcase?
> Since bug 810543, normally, profileDir is available as soon as ProfD is
> available.

In browser/components/sessionstore/src/SessionStore.jsm, change line 96 from a lazy module getter to a sync Cu.import, and watch the world fall over on startup (because _SessionFile does a .join on the profileDir + sessionstore.js, with a null profileDir, which means the path join code fails)

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> When they both are available, ProfD and ProfDS will always be the same
> value. However, there is a significant difference in what they *mean*:
> 
> When ProfD is available, it means that the profile-after-change notification
> has been sent and the profile is ready for use.
> 
> Before that time, ProfDS is available, but the profile may be in a state of
> migration and nobody should be touching that directory without specific
> knowledge that they won't mess up migration etc.
> 
> So in general I don't think that Path.profileDir should use ProfDS, and the
> migration code needs specific understanding of ProfDS.

What are we specifically trying to prevent by not having Path.profileDir use that? The migration code, as far as I know, is pretty much guaranteed to run before many other bits get started up, and things that do start up before (eg. the prefs service) are used by migration code and that doesn't seem to be causing issues. I essentially think enforcing "don't touch this before profile-after-change" should be done in other layers of the code. If code is trying to read/write things that it shouldn't be before profile-after-change, they should observe profile-after-change... As seen above, the current "this is a constant except it might be null early on" breaks the world in some cases (throwing exceptions from jsms in the startup path turns out to break lots of stuff).

Some context: I'm trying to do window/tab restore for Firefox reset. I can work around this bug/feature of OS.File, but it would involve adding stuff "all the way down" to have an alternative path for migration writing a sessionstore file (make _SessionFile not statically use Path.profileDir; add APIs / path parameters to the various sessionstore bits to write to arbitrary locations rather than the default, and pass the profile dir from the migration code). I can do that, but I personally think that making profileDir available from startup onwards on OS.File would make more sense.
Flags: needinfo?(benjamin)
Assignee: gijskruitbosch+bugs → nobody
This has been fixed at some point.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
Flags: needinfo?(benjamin)
You need to log in before you can comment on or make changes to this bug.