Closed
Bug 1345460
Opened 8 years ago
Closed 8 years ago
Implement FX_SANITIZE telemetry on Android
Categories
(Firefox for Android Graveyard :: Metrics, defect)
Tracking
(firefox53 fixed, firefox54 fixed, firefox55 fixed)
RESOLVED
FIXED
Firefox 55
People
(Reporter: JanH, Assigned: JanH)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
liuche
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details |
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.
Comment hidden (mozreview-request) |
Marking 54/55 affected based on bug 1343995. Thanks for filing this follow up bug!
Assignee | ||
Comment 3•8 years ago
|
||
mozreview-review |
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.
Updated•8 years ago
|
Attachment #8845084 -
Flags: review?(s.kaspari) → review?(liuche)
Comment 4•8 years ago
|
||
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 6•8 years ago
|
||
mozreview-review |
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)
Comment hidden (mozreview-request) |
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/5691e461dee1
Implement FX_SANITIZE telemetry on Android. r=liuche
Assignee | ||
Comment 10•8 years ago
|
||
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?
Comment 11•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 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+
Comment 13•8 years ago
|
||
bugherder uplift |
Comment 14•8 years ago
|
||
bugherder uplift |
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•