Extend browsingData api for single site

NEW
Unassigned

Status

()

Toolkit
WebExtensions: General
P5
normal
3 months ago
8 days ago

People

(Reporter: afnankhan, Unassigned, NeedInfo)

Tracking

(Blocks: 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [design-decision-approved][browsingData] triaged)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 months ago
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.

Updated

3 months ago
Whiteboard: [design-decision-needed]

Comment 1

2 months ago
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.

Comment 2

2 months ago
Created attachment 8848942 [details] [diff] [review]
browsingData-extensions.diff

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?
Flags: needinfo?(bob.silverberg)

Comment 3

2 months ago
Created attachment 8849843 [details] [diff] [review]
browsingData-extensions.diff

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
Blocks: 1215059
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?
Flags: needinfo?(cneiman)
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#
Flags: needinfo?(cneiman)

Comment 6

2 months ago
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.

Comment 8

2 months ago
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
See Also: → bug 1353726
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)

Comment 11

2 months ago
:baku maybe?
Assignee: wisniewskit → nobody
Flags: needinfo?(amckay)
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.
Flags: needinfo?(wisniewskit)
Blocks: 1043081
You need to log in before you can comment on or make changes to this bug.