This is the Fennec-specific mirrorworld of Bug 808263. We need profile creation times for FHR, but Fennec takes different code paths to desktop, creating the profile directory in Java and thus skipping what I did in Bug 808263.

Note that the thorough solution to this problem is to fix the call path in nsToolkitProfileService. But on desktop, FHR and other callers will eventually cause times.json to be created, with fairly accurate times; on Android I think it's fine to just write the file ourselves.

Tested by hand, works.

Hindsight being what it is, it's too bad we don't just write the creation times in profiles.ini, with the other profile information.

Yeah, Bug 808263 was a little bit of a hindsight-party. I'm hoping that times.json is sufficient for the future purposes of other people who want to record something like a profile event transaction log, such as moves to SD card, renames, whatever. It's that perennial balancing act between simplicity and extensibility...

Comment on attachment 751887 [details] [diff] [review] Proposed patch. v1 Review of attachment 751887 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoProfile.java @@ +401,5 @@ > + try { > + FileOutputStream stream = new FileOutputStream(profileDir.getAbsolutePath() + File.separator + "times.json"); > + OutputStreamWriter writer = new OutputStreamWriter(stream, Charset.forName("UTF-8")); > + try { > + writer.append("{\"created\": " + System.currentTimeMillis() + "}\n"); I kinda hate hand writing JSON like this. You mind creating a real JSONObject? ("This is firstrun/startup and I don't want to make things slow" might be valid here... but the file i/o is probably a much much larger bottleneck). Also, I hate nested trys. Can we make this one?

(In reply to Wesley Johnston (:wesj) from comment #5) Thanks for the speedy review! > I kinda hate hand writing JSON like this. You mind creating a real > JSONObject? ("This is firstrun/startup and I don't want to make things slow" > might be valid here... but the file i/o is probably a much much larger > bottleneck). Yeah, me too… but. I don't think there's much of an understandability/extensibility win from doing that now, and as you mention it'd add just another papercut to first run. It would also add an import for the JSON classes to a file that's blissfully without. I'd rather leave it as simple string stuff until/unless some additional complexity needs to occur here. Taras would kill me otherwise ;) > Also, I hate nested trys. Can we make this one? writer.close() can throw, and we don't want that exception to propagate, so there needs to be a nested try somewhere.

https://hg.mozilla.org/integration/mozilla-inbound/rev/67673d9b232d Will be wanting this to land as far in advance of FHR as possible. It's a small, self-contained change, so if nothing regresses, I'll request Aurora and Beta approval.

Tracking for 22/23 in that case, please put in the uplift nomination with risk assessment quickly in order to be considered for Beta 4 of FF 22

Comment on attachment 751887 [details] [diff] [review] Proposed patch. v1 [Approval Request Comment] Bug caused by (feature/regressing bug #): None. User impact if declined: New profiles won't track their creation date. This will have an undetermined impact on the advice that FHR can offer, and the accuracy of the conclusions Mozilla can draw from FHR data, because we'll fall back to heuristics such as folder creation date. Testing completed (on m-c, etc.): Landed very recently, so I plan to let this bake for a day or two. I've tested manually and have seen no issues, no strict mode warnings, and I'm happy with the quality. Risk to taking this patch (and alternatives if risky): Slight risk of first-run delay due to file writing during profile creation, but that write will otherwise occur during FHR init, after some additional reading expense. No code risk -- the entire addition is in a try-catch block. String or IDL/UUID changes made by this patch: None.

Why do we need to take this on Beta 22 at all if there's the possibility of regression and little value on that version?

Because it captures accurate data for any new users we accrue between now and the next uplift. This change *only* affects new users, precisely once per install. The value in uplift (even beyond when FHR ships) is that it expands the total set of people who use Firefox for the first time *after* this change lands, and thus reduces the set of people whose profile creation times are determined via a heuristic. If I could go back in time and ship this in January, or alongside Bug 808263, I would.

(In reply to Richard Newman [:rnewman] from comment #12) > If I could go back in time and ship this in January, or alongside Bug > 808263, I would. Yeah, but that landed on m-c and rode the trains. Not of high value for the end user, so this doesn't meet our Beta criteria given risk.