Open Bug 1340511 Opened 7 years ago Updated 11 months ago
Data api for single site
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0 Build ID: 20170216030210 Steps to reproduce: I want to create an extension that remove site data (cookies, localStorage, indexedDB, serviceWorkers, cacheStorage, applicationCache) when all tabs of a site closes.
Note that it would be useful to be able to do this at a time well *after* the last tab for a given domain closes, to be able to keep the session data alive for a few more seconds (in case the users undos the tab close, or loads a new tab to come back to the same domain for some reason). By that point the frame's content-script and its reference to localStorage/etc would be lost. As such there seems to be merit in extending the API like this, as the only way I could see the same thing being done would be to prospectively wait for a new tab for the same domain to be opened and clear its data in the content-script on document_start, which is a wasteful way of doing the same thing (given that browsingData already does it on a broader scale). Either that, or APIs like browser.cookies could be added to manipulate localStorage, indexedDBs, and so forth... but I surmise that would be a lot more work and be more controversial (given the decision in bug 1329745). Of course if this is considered, I'd humbly recommend considering allowing a list of hosts/domains to be passed in to clear, rather than requiring calling the API one-by-one. It appears that the current code for browsingData would behave more efficiently that way, rather than calling it multiple times.
I had some time to investigate this, and came up with a WIP patch that would give us support for a "hostnames" parameter to the cookies part of the API, plus add support for localStorage and indexedDB to the browsingData API (though only for "hostnames" in the indexedDB case for now). Do you think this sort of approach would be acceptable, :bsilverberg?
And just in case it helps, here's a updated version of the patch which also handles the "clear all indexedDBs" case, rather than just host-specific ones (though I can't be 100% sure if I'm doing it properly; I'm just basing it on how the devtools manage indexedDBs in devtools/server/actors/storage.js).
Attachment #8848942 - Attachment is obsolete: true
Thanks for the patch, Thomas. This bug is listed as [design-decision-needed] so I think it needs to be discussed to decide whether this is something we want to add to the API. Caitlin, can you please add this bug to the next WebExtensions API Triage meeting, which I believe is scheduled for tomorrow?
Just added it to the agenda! Thomas and afnankhan, would you be able to attend the meeting tomorrow at 10:30am PT? Call in info is here: https://wiki.mozilla.org/Add-ons/Contribute/Triage Meeting agenda is here: https://docs.google.com/document/d/1V4NP4tWnjHigS2lAosCLfkU2FTcrQnoQzzXZmmB1uzk/edit#
I should be there, thanks for the update! Just in case I somehow miss it, here are some key concerns: 1. It's currently impossible for content scripts to know which indexedDBs are present for their given host, so it's impossible for them all to be cleared without a new API. It may be possible to create a standard (and un-broken) version of webkitGetDatabaseNames to get around this, but I'm not sure if such a concept is just broken by design, as Chrome is removing it in version 60 (https://bugs.chromium.org/p/chromium/issues/detail?id=696010). 2. Right now, only content scripts can alter localStorage, and it's tricky to inform them whether they ought to do so. One has to detect onBeforeNavigation (or similar), set its cookie for the related host to a sentinel value, and then have a document_start content script check for that value (for all_frames) and clear the relevant data stores. It's unclear whether all of this will happen before the frame's scripts run and start accessing the stores, which might be tricky to resolve. 3. Having these APIs would make it possible to avoid such per-frame content scripts, which seems likely to be a significant win. 4. It's probably unnecessary to be able to clear multiple hosts per call to browsingData.clear, though that's how I implemented it in my patches for now. 5. Being able to actually alter, rather than just clear, non-cookie data stores is also a valuable use-case. Some addons already do so for specific cookies, and I'm writing an addon for Mozilla which would benefit from being able to "seed" the storage values before a frame's scripts are run (and currently it seems impossible to do so, given the discussions in bug 1332273). I would recommend considering this use-case during the same meeting as there is some overlap of concerns (even if a decision is pushed back to a later meeting).
In preparation for the triage meeting, I wanted to summarize what I see in the patch attached to this bug, as I think it seems to cover a couple of different things. 1. Implementations for clearing all IndexedDB and all LocalStorage data, which is something that Chrome currently provides but has not yet been implemented for Firefox. That is actually being tracked via Bug 1333050. 2. An extension to the browsingData API which allows a developer to clear data for a specific set of hostnames. This has been implemented for cookies, indexeddb and localstorage. It is not something that Chrome provides. I think what we need to discuss in the triage meeting is item #2, as item #1 is something that is already available in Chrome and just has not been implemented in Firefox yet.
Sure :bsilverberg, that makes sense to me. If possible I would also like to consider my first and fifth points in comment 6, but I don't mind doing so in in the future if that goes against the plans of tomorrow's meeting.
We discussed in the triage meeting that ideally all of the methods of the browsingData API would be enhanced to support hostnames, as opposed to just cookies, localStorage and indexedDB. Follow-up bugs can be filed for that.
Assignee: nobody → wisniewskit
Status: UNCONFIRMED → NEW
Component: WebExtensions: Untriaged → WebExtensions: General
Ever confirmed: true
Priority: -- → P5
Whiteboard: [design-decision-needed] → [design-decision-approved][browsingData] triaged
Thomas, I can do an initial review of the WebExtensions part of this patch, but I'm not in a position to review the stuff specific to localStorage or indexedDB. Can you please post a patch to MozReview for this one? Andy, do you know who we should ask to review the code that interacts with localStorage and indexedDB?
Flags: needinfo?(bob.silverberg) → needinfo?(amckay)
Assignee: wisniewskit → nobody
Thomas, perhaps it makes sense to create a patch which just implements the clearing of localStorage and indexedDB, and then attach that to bug 1333050 which was opened some time ago to track that. Then this bug can be used for adding the ability for some of the methods in browsingData to clear data for specific domains. You can flag both me and :baku for review of the patch that you attach (via MozReview, please) to bug 1333050.
As a user of Self-Destructing Cookies, I'm personally interested in a replacement getting up and running on WebExtensions. This seems to be what's blocking this from happening currently. :bsilverberg, it seems like options for clearing different types of storage have been added separately, is that right? I could split out a patch which focuses on clearing data for particular hostnames and push that up for review.
As this bug has been flagged with [design-decision-approved], patches for implementing these features would be welcome. Thomas attached a patch to this some time ago that added the ability to clear cookies by hostname, but it also included a lot of code for clearing indexedDB and localStorage which does not apply to the cookie issue. If you'd like to work on the cookie issue, Michael, I would suggest opening a new bug which is about just adding this feature for cookies, and then attach a patch to that bug. Please mark that new bug as blocking this bug.
Just FYI, I have some time this week as well, if we'd like to prioritize landing just the cookie-specific bits of my patch ("since" and/or "hostnames" support). I don't mind if Michael wants to beat me to it, though!
(Ah, right, we already have "since" support for cookies, so it would only be "hostnames" that I would split out).
:mismith, I already made an alternative. That's what Cookie AutoDelete is. https://addons.mozilla.org/en-US/firefox/addon/cookie-autodelete/ All I need to close the gap with SDC is the extension of the browsingData API to support deleting localstorage by hostname. So what's the status on it? This is the issue tracker for my extension that deals with the localstorage issue with current solutions and future solutions (which includes the above browsingData extension). https://github.com/mrdokenny/Cookie-AutoDelete/issues/44
For those not following along in the sub-bugs, localStorage can now be cleared on a per-hostname basis in the nightly builds (bug 1388428), and is riding the trains to Firefox 58.
Anyone interested in this might also be interested in Bug 1464895.
You need to log in before you can comment on or make changes to this bug.