Closed
Bug 1431029
Opened 7 years ago
Closed 7 years ago
Show a "last accessed" column in the site data manager
Categories
(Firefox :: Settings UI, enhancement, P1)
Tracking
()
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: johannh, Assigned: johannh)
References
(Blocks 1 open bug)
Details
(Whiteboard: [storage-v2])
Attachments
(2 files)
This was a suggestion from the Site Data Manager meeting in Austin. Since we have data for quota storage and cookies about when they were last written/accessed, we could show this in the site data manager to give the user a better understanding of what data can potentially be removed and what site data/cookies have recently been accessed.
Updated•7 years ago
|
Priority: -- → P2
Whiteboard: [storage-v2][triage] → [storage-v2]
Assignee | ||
Comment 1•7 years ago
|
||
Jan, you mentioned in Austin that getting the last access timestamp from quota managed storage should not be too hard. To be honest I'm not sure where to look for that data. Is this already saved somewhere or would we have to add it? If it's more or less just about adding a timestamp to some data structure I'm happy to get pointers on which code would need modification and could do it myself.
Andrew, I already asked you this on IRC but I wasn't really available afterwards due to time zone differences, so I'll just needinfo you here. Do you have any idea if this is feasible?
Thanks :)
Flags: needinfo?(jvarga)
Flags: needinfo?(bugmail)
Comment 2•7 years ago
|
||
Ahh... I totally mis-parsed your question on IRC.
The timestamp of last access is stored in those .metadata-v2 files you see in the root of each origin's storage in QM.
The GetUsageOp that looks up the usage of every origin already reads that information out into the timestamp here:
https://searchfox.org/mozilla-central/source/dom/quota/ActorsParent.cpp#6938
It's mainly a question of adding the data to OriginUsage (defined at https://searchfox.org/mozilla-central/source/dom/quota/PQuotaUsageRequest.ipdl#11) and UsageResult (https://searchfox.org/mozilla-central/source/dom/quota/QuotaResults.h#13) and nsIQuotaUsageResult (https://searchfox.org/mozilla-central/source/dom/quota/nsIQuotaResults.idl#10) and all the plumbing in between.
Flags: needinfo?(jvarga)
Flags: needinfo?(bugmail)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
Priority: P2 → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
This isn't as well-tested as I'd like it to be, mostly because I feel like we're missing some piece in the chain that reliably unit tests SiteDataManager.jsm without going through UI. I filed bug 1439547 for that.
Comment 7•7 years ago
|
||
Yeah, what asuth said. I also quickly went over the C++ patch and it looks good.
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8952357 [details]
Bug 1431029 - Show a "last accessed" column in the site data manager.
https://reviewboard.mozilla.org/r/221610/#review227506
::: browser/components/preferences/in-content/tests/browser_siteData.js:153
(Diff revision 1)
> + let formatter = new Services.intl.DateTimeFormat(undefined, {
> + dateStyle: "short", timeStyle: "short",
> + });
The other advantage of using a fixed date will be that you can put this right next to where you use it...
::: browser/components/preferences/in-content/tests/browser_siteData.js:167
(Diff revision 1)
> + // Set a creation date to compare with. This doesn't have to be too precise,
> + // since we only go down to minute level in the UI.
> + let creationDate = formatter.format(new Date());
But this will have a hard cut-off point, right? So let's say that the site cookie date and the new Date() here end up differing by 10ms on a slow mac debug build. Then the chance that those 20ms cross a minute distinction is 10 / 60,0000. So this will be orange once per 6000 runs. Which probably means once every week or 2 on our current infra set, if the test actually ran (several times) on each push.
Can't we just pass the cookie lastAccessed date here directly? (I'm assuming cookies.add() returns it...)
::: browser/locales/en-US/chrome/browser/preferences/siteDataSettings.dtd:10
(Diff revision 1)
> <!ENTITY window1.title "Settings - Cookies and Site Data">
> <!ENTITY hostCol.label "Site">
> <!ENTITY statusCol.label "Status">
> <!ENTITY cookiesCol.label "Cookies">
> <!ENTITY usageCol.label "Storage">
> +<!ENTITY lastAccessedCol.label "Last Used">
Why not label this 'last accessed' ? "Used" is much more active terminology, but I imagine that cookie usage from 3rd party domains will mean that this labeling will confuse people. People don't use a toplevel google.com page every time a google cookie is accessed or changed.
Attachment #8952357 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8952356 [details]
Bug 1431029 - Expose timestamp for quota managed storage.
https://reviewboard.mozilla.org/r/221608/#review227748
Attachment #8952356 -
Flags: review?(bugmail) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8952357 [details]
Bug 1431029 - Show a "last accessed" column in the site data manager.
https://reviewboard.mozilla.org/r/221610/#review227506
> But this will have a hard cut-off point, right? So let's say that the site cookie date and the new Date() here end up differing by 10ms on a slow mac debug build. Then the chance that those 20ms cross a minute distinction is 10 / 60,0000. So this will be orange once per 6000 runs. Which probably means once every week or 2 on our current infra set, if the test actually ran (several times) on each push.
>
> Can't we just pass the cookie lastAccessed date here directly? (I'm assuming cookies.add() returns it...)
It does not return it, but you have a point and getting the cookies manually doesn't cost us anything, so I did that. Thanks!
> Why not label this 'last accessed' ? "Used" is much more active terminology, but I imagine that cookie usage from 3rd party domains will mean that this labeling will confuse people. People don't use a toplevel google.com page every time a google cookie is accessed or changed.
I'll check with jsavory.
Assignee | ||
Comment 15•7 years ago
|
||
Hmm I didn't get ahold of Jacqueline but I'd like to move forward and not let this bitrot (I'm quite busy on this code right now), so I'll land this with the strings from the design spec and will revise soon if necessary.
Comment 16•7 years ago
|
||
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/963a227648af
Expose timestamp for quota managed storage. r=asuth
https://hg.mozilla.org/integration/autoland/rev/dced0221e5df
Show a "last accessed" column in the site data manager. r=Gijs
Comment 17•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/963a227648af
https://hg.mozilla.org/mozilla-central/rev/dced0221e5df
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in
before you can comment on or make changes to this bug.
Description
•