Closed Bug 1337115 Opened 7 years ago Closed 7 years ago

Investigate telemetry for a completely failed session restore

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox54 fixed)

RESOLVED FIXED
Firefox 54
Tracking Status
firefox54 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(2 files)

Currently we don't send telemetry if we failed to restore from both the main session store file and its backup copy, since this state is also regularly encountered after Firefox is installed or has had its app data cleaned.
I need to check this again, but I think that now even after history cleaning etc. we always write a valid (but empty) file to disk, so other than that all other occurrences of this should indicate an actual problem.

We should therefore try and see if checking for the "First run" pref and excluding that case yields useable results.
Comment on attachment 8838681 [details]
Bug 1337115 - Part 2 - Send telemetry if session restore completely fails and we're not on the first run.

https://reviewboard.mozilla.org/r/113498/#review116072

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java:1382
(Diff revision 1)
> +                                    if (!GeckoSharedPrefs.forProfile(GeckoApp.this).
> +                                            getBoolean(TelemetryCorePingDelegate.PREF_IS_FIRST_RUN, true)) {

I'm not really happy about this living in TelemetryCorePingDelegate and we making it public now. This is a bit hacky.
Comment on attachment 8838681 [details]
Bug 1337115 - Part 2 - Send telemetry if session restore completely fails and we're not on the first run.

https://reviewboard.mozilla.org/r/113498/#review116072

> I'm not really happy about this living in TelemetryCorePingDelegate and we making it public now. This is a bit hacky.

I hope this attempt counts as less hacky :-)
Comment on attachment 8841045 [details]
Bug 1337115 - Part 1 - Make "Is first run" pref generally useable.

https://reviewboard.mozilla.org/r/115350/#review117738

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java:166
(Diff revision 1)
>      public static final String PREFS_VERSION_CODE          = "versionCode";
>      public static final String PREFS_WAS_STOPPED           = "wasStopped";
>      public static final String PREFS_CRASHED_COUNT         = "crashedCount";
>      public static final String PREFS_CLEANUP_TEMP_FILES    = "cleanupTempFiles";
>  
> +    public static final String PREFS_IS_FIRST_RUN = "telemetry-isFirstRun";

Maybe add a comment why this the name has a "telemetry" prefix :)
Attachment #8841045 - Flags: review?(s.kaspari) → review+
Comment on attachment 8838681 [details]
Bug 1337115 - Part 2 - Send telemetry if session restore completely fails and we're not on the first run.

https://reviewboard.mozilla.org/r/113498/#review117742
Attachment #8838681 - Flags: review?(s.kaspari) → review+
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/1f45e38b27af
Part 1 - Make "Is first run" pref generally useable. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/b25d9cfea3f9
Part 2 - Send telemetry if session restore completely fails and we're not on the first run. r=sebastian
https://hg.mozilla.org/mozilla-central/rev/1f45e38b27af
https://hg.mozilla.org/mozilla-central/rev/b25d9cfea3f9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
See Also: → 1345534
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.