Closed Bug 1491126 Opened 6 years ago Closed 6 years ago

Don't create times.json until needed

Categories

(Toolkit :: Startup and Profile System, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mossop, Unassigned)

References

Details

Attachments

(1 obsolete file)

The profile service automatically creates a times.json file with a creation date when creating a profile, regardless of whether that profile is about to be, or ever used. STR:

1. Run firefox -P
2. Create a new profile in the profile manager
3. Exit the profile manager without launching the new profile.

The profile now exists on disk with a creation time even though the profile hasn't been used by the app. Some time later the user can run with this profile and it can appear to be somewhat older than it really is.

times.json is not really needed, ProfileAge.jsm falls back to finding the oldest file in the profile if it doesn't exist. This will correctly give the time the profile was first used so I propose just not creating times.json by default.
Comment on attachment 9009173 [details]
Bug 1491126: Don't create times.json on profile creation. r=chutten! r=froydnj!

Nathan Froyd [:froydnj] has approved the revision.
Attachment #9009173 - Flags: review+
ref for how we've struggled with ProfileAge and times.json recently: bug 1449739

+janerik, +shong

In short: the times.json file created on profile manager creation is actually the more accurate timestamp tracking profile creation dates than any of the others we've studied, so it would be a shame to remove it.
Flags: needinfo?(shong)
Flags: needinfo?(jrediger)
(In reply to Chris H-C :chutten from comment #3)
> ref for how we've struggled with ProfileAge and times.json recently: bug
> 1449739
> 
> +janerik, +shong
> 
> In short: the times.json file created on profile manager creation is
> actually the more accurate timestamp tracking profile creation dates than
> any of the others we've studied, so it would be a shame to remove it.

So bug 1474285 is potentially going to make it older. We'll be creating placeholder profiles for older versions of Firefox to use when run, so you could see a profile created and then the first time the user uses a pre-64 version of Firefox they will start submitting telemetry, which could be a while after the profile (and times.json) was created.
With bug 1466518 :janerik introduced code that reports the date that the ProfileAge directory scan was performed. Maybe we could adapt that code to report the time of the first ProfileAge call even if times.json is present? (maybe we store this "profile first used" timestamp in times.json?)
A problem is that we can't really affect pre-64 builds at this point. Certainly not pre-63. So either we have the situation where times.json might be quite old compared to the first use of the profile with an old build, or we have the error when times.json is not present when queried. Not sure which is preferred.
(In reply to Dave Townsend [:mossop] (he/him) from comment #0)
> The profile now exists on disk with a creation time even though the profile
> hasn't been used by the app. Some time later the user can run with this
> profile and it can appear to be somewhat older than it really is.

IMO, that time in the file _is_ the creation time.
The first usage of that profile might be called "first usage time". If that's what we want to measure we should call it that way and don't mix these 2 things.

(In reply to Dave Townsend [:mossop] (he/him) from comment #0)
> times.json is not really needed, ProfileAge.jsm falls back to finding the
> oldest file in the profile if it doesn't exist. This will correctly give the
> time the profile was first used so I propose just not creating times.json by
> default.

As mentioned by :chutten already, our fallback solution is unreliable. The profile manager does know when it did create a profile and it should reliably record that. Our fallback should stay a fallback only.


Maybe we should first define what we actually want to get out of this timestamp and whether not using a profile for a long time is a relevant data point for us.
Flags: needinfo?(jrediger)
(In reply to Jan-Erik Rediger [:janerik] from comment #7)
> Maybe we should first define what we actually want to get out of this
> timestamp and whether not using a profile for a long time is a relevant data
> point for us.

Many components seem to use the creation date as a proxy for determining whether someone is a "new user". Shield does this to help target experiments. Activity stream and unified autocomplete seem to use it for similar purpose.
Unified autocomplete used to use it to populate a users awesomebar results with popular sites for the first short time after profile creation. That feature appears to be disabled now though.
I strongly, strongly agree with Jan Erik, and +1 his suggestion for adding a new field for "first usage time". 

1: We shouldn't change the logic of how an existing field works, especially if that field is a pretty fundamental attribute we use for defining profiles. The reason being, once we change it, any comparisons to the past (and across versions) are no longer valid. We won't be able to compare "new profile counts" (using for example, the specific definition profile_creation_date = submission_date_s3) across time. We'll have no idea if we have "more" or "less" new profiles then before. 

* If we want to modify or change the behavior of a field, the procedure should be to record a new field using that logic in addition to the existing field. That way we can use the old field (which still uses the original logic) for historical comparisons and use the new field for context. 

2: There is no clear criteria of what is more "correct" for profile_creation_date. Currently, it shows: 
    - When the first run was if profile creation and first run for that profile happened at the same time. 
    - When the profile was created but not first run was if profile was created but not run. 
    - The age of the oldest file in the directory if profile is created via terminal (not automatically by Firefox or via Profile Manager) 

* The issue is that we're conflating 3 things here. I absolutely agree on a need to distinguish those, and find out when/if:
    - the date of first run 
    - the date profile was created
    - if the profile was created not via profile manager and profile_creation_date is reporting age of a random file it sees 
* These three things need to start being recorded (BUT we shouldn't change how `profile_creation_date` works in order to do so. 

3: I'm not sure how difficult it is to update the shield and activity stream logic to look for a different field first (check new_date_field, if not found, use profile_creation_date), but if we change how profile_creation_date works, we cannot go back, the field will mean different things at different times for different versions of users (which is very bad). Updating the logic of applications that use profile_creation_date seems like the safe choice. 



I think we should have a meeting to discuss this.
Flags: needinfo?(shong)
I'd like to be clear about the problem I'm trying to solve. Stopping creating times.json in all cases was one simple solution but there are others that meet my needs but aren't as extreme.

When profiles-per-install lands as users upgrade to supporting builds their profiles will be marked as in-use by current installs of firefox. To protect against downgrades we will create stub profiles for use by older builds of Firefox. Those stub profiles may never be used, or they may be used some time after creation. Because they are only used by older builds of Firefox, we can't change their code to affect how they behave. We can either create the times.json for those stub profiles or not (and we can do that without affecting normal profiles). The consequence is that if the user ends up downgrading and using the stub profiles either the profile creation date will be outdated by some amount (most likely a low amount I guess), or we have to rely on scanning the profile to get its date which apparently isn't accurate.

Which would we prefer to do?
Attachment #9009173 - Attachment is obsolete: true
Per bug 1474285 comment 9 we're going to leave this alone.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: