Closed Bug 1345460 Opened 3 years ago Closed 3 years ago

Implement FX_SANITIZE telemetry on Android

Categories

(Firefox for Android :: Metrics, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(1 file)

Bug 1343995 means that when clearing private data on exit, we now actually wait for sanitizing to finish before quitting.

This is good, because it means that sanitizing won't randomly fail anymore if it lost the race with the shutdown process, but it's also bad because quitting used to be near-instant, whereas now it'll be delayed by however long sanitization is taking.

To get a handle on how big of a problem this is, we should collect data on how long sanitization is actually taking in practice.

Desktop already collects a number of FX_SANITIZE_... histograms, so we can simply use those as well and implement them in our Sanitizer.
Marking 54/55 affected based on bug 1343995. Thanks for filing this follow up bug!
Comment on attachment 8845084 [details]
Bug 1345460 - Implement FX_SANITIZE telemetry on Android.

https://reviewboard.mozilla.org/r/118276/#review120914

::: mobile/android/chrome/content/browser.js:1523
(Diff revision 1)
>  
>        if (callback) {
>          callback();
>        }
>      }).catch(function(err) {
>        GlobalEventDispatcher.sendRequest({

I should probably add a call to `TelemetryStopwatch.finish()` here as well.
Attachment #8845084 - Flags: review?(s.kaspari) → review?(liuche)
I'll redirect to liuche. She has the power to review the Android code and the probes. :)
liuche, can you help out here? I'll email to follow up. We're asking because of the request to uplift some code to beta, in bug 134995.
Comment on attachment 8845084 [details]
Bug 1345460 - Implement FX_SANITIZE telemetry on Android.

https://reviewboard.mozilla.org/r/118276/#review124470

Sorry for the delay! I looked at the probe usage and it looks like it follows the pattern of desktop usage. Fwiw, I don't think this needed data review because it's using existing probes in the same way, but thanks for flagging me just in case! r+ for data and code review.
Attachment #8845084 - Flags: review?(liuche) → review+
Thanks!  Can you request uplift? Does this need uplift to 53 or just to 54?
Flags: needinfo?(jh+bugzilla)
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/5691e461dee1
Implement FX_SANITIZE telemetry on Android. r=liuche
Blocks: 1349155
Flags: needinfo?(jh+bugzilla)
Comment on attachment 8845084 [details]
Bug 1345460 - Implement FX_SANITIZE telemetry on Android.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1343995
[User impact if declined]: We want to investigate the impact of bug 1343995 on the delay between pressing the "Quit" button and the UI actually exiting, so we can know how to proceed (e.g. bug 1349155).
[Is this code covered by automated tests?]: Partially.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Just adding telemetry stopwatches along the same lines as Desktop.
[String changes made/needed]: none
Attachment #8845084 - Flags: approval-mozilla-beta?
Attachment #8845084 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5691e461dee1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Comment on attachment 8845084 [details]
Bug 1345460 - Implement FX_SANITIZE telemetry on Android.

Adds some telemetry for diagnostic work, has data review, let's uplift.
Attachment #8845084 - Flags: approval-mozilla-beta?
Attachment #8845084 - Flags: approval-mozilla-beta+
Attachment #8845084 - Flags: approval-mozilla-aurora?
Attachment #8845084 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.