Closed
Bug 1238785
Opened 8 years ago
Closed 8 years ago
Delete health.db after Firefox Health Report is removed from Fennec
Categories
(Android Background Services Graveyard :: Firefox Health Report Service, defect)
Android Background Services Graveyard
Firefox Health Report Service
All
Android
Tracking
(firefox48 fixed, fennec48+)
RESOLVED
FIXED
Firefox 48
People
(Reporter: rnewman, Assigned: mcomella)
References
Details
Attachments
(3 files)
Now that FHR is gone, we can delete the existing database file and free up that space. We have existing bugs to do janitorial work, so let's see if we can kill more than one bird with this stone rather than making a special case.
Updated•8 years ago
|
tracking-fennec: ? → 46+
Comment 1•8 years ago
|
||
mcomella, can you take this? (In reply to Richard Newman [:rnewman] from comment #0) > Now that FHR is gone, we can delete the existing database file and free up > that space. > > We have existing bugs to do janitorial work, so let's see if we can kill > more than one bird with this stone rather than making a special case. One dead bird is better than no dead birds, let's not block on fixing the world.
Flags: needinfo?(michael.l.comella)
Comment 3•8 years ago
|
||
This probably isn't going to happen for 46 at this rate. However, we should definitely do this cleanup to free up that space.
tracking-fennec: 46+ → 47+
Assignee | ||
Comment 4•8 years ago
|
||
This is on my radar and I'm working on a fix. For reference, I have a profile from January 12th and it takes up the following space: 72K health.db 41K health.db-journal 113K Not huge, but not insignificant for mobile.
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 5•8 years ago
|
||
This is intentionally kept minimal to ensure simplicity. Review commit: https://reviewboard.mozilla.org/r/45935/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45935/
Attachment #8740699 -
Flags: review?(ahunt)
Attachment #8740700 -
Flags: review?(ahunt)
Attachment #8740701 -
Flags: review?(ahunt)
Assignee | ||
Comment 6•8 years ago
|
||
This controller is under-featured (e.g. it's not scheduling cleanups for future dates and it doesn't cache files it already deleted) in favor of simplicity. Review commit: https://reviewboard.mozilla.org/r/45937/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45937/
Assignee | ||
Comment 7•8 years ago
|
||
I added some log statements to ensure this worked correctly locally - on a new profile: * Log statements were printed listed the two files I expected to be deleted and their paths * The log statements did not appear after closing and reopened fennec, meaning the process short-circuited as expected. Ideally, I'd test that a profile that currently has these files actually gets them deleted, but it's not easy to create profiles. The previous patches also contributed unit tests. Review commit: https://reviewboard.mozilla.org/r/45939/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45939/
Comment 8•8 years ago
|
||
Comment on attachment 8740699 [details] MozReview Request: Bug 1238785 - Add FileCleanupService. r=ahunt https://reviewboard.mozilla.org/r/45935/#review43005 Looks reasonable to me! I was wondering whether we'd want to log when File.delete() fails, but that seems unnecessary to me.
Attachment #8740699 -
Flags: review?(ahunt) → review+
Reporter | ||
Comment 9•8 years ago
|
||
You probably want to delete -shm and -wal suffixes, too; they might differ across android versions. Best to catch em all.
Comment 10•8 years ago
|
||
Comment on attachment 8740700 [details] MozReview Request: Bug 1238785 - Add FileCleanupController and tests. r=ahunt https://reviewboard.mozilla.org/r/45937/#review43015
Attachment #8740700 -
Flags: review?(ahunt) → review+
Comment 11•8 years ago
|
||
Comment on attachment 8740701 [details] MozReview Request: Bug 1238785 - Start file cleanup in onStart. r=ahunt https://reviewboard.mozilla.org/r/45939/#review43017 Looks good to me!
Attachment #8740701 -
Flags: review?(ahunt) → review+
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #9) > You probably want to delete -shm and -wal suffixes, too; they might differ > across android versions. Best to catch em all. Added. Gotta catch 'em all!
Assignee | ||
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/009b0d561a9c10452c674a54aff993db3aeffdcd Bug 1238785 - Add FileCleanupService. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/a955962a5198d3964d336755df7b7968323d657e Bug 1238785 - Add FileCleanupController and tests. r=ahunt https://hg.mozilla.org/integration/fx-team/rev/acaefde5bf02467bf2000059ab4cd2cd23dba0a2 Bug 1238785 - Start file cleanup in onStart. r=ahunt
Assignee | ||
Comment 14•8 years ago
|
||
The only users that this affects are long time users (i.e. had FHR). I don't think these users are likely to uninstall Firefox for the several KB of data that we're cleaning up here so I don't feel it's worth uplifting to 47 (we're tracking 47). Margaret, what do you think?
Flags: needinfo?(margaret.leibovic)
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/009b0d561a9c https://hg.mozilla.org/mozilla-central/rev/a955962a5198 https://hg.mozilla.org/mozilla-central/rev/acaefde5bf02
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 16•8 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #14) > The only users that this affects are long time users (i.e. had FHR). I don't > think these users are likely to uninstall Firefox for the several KB of data > that we're cleaning up here so I don't feel it's worth uplifting to 47 > (we're tracking 47). > > Margaret, what do you think? That's a smart way to think about it. I agree, if they haven't given up on Firefox yet, I don't think an extra 6 weeks will make much of a difference.
tracking-fennec: 47+ → 48+
Flags: needinfo?(margaret.leibovic)
You need to log in
before you can comment on or make changes to this bug.
Description
•