1340511 - Extend browsingData api for single site

Status: NEW

(WebExtensions :: General, enhancement, P5)

Description

[afnankhan]

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.

Comment 1

[Thomas Wisniewski]

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

[Thomas Wisniewski]

Attachment: 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?

Comment 3

[Thomas Wisniewski]

Attachment: 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).

Comment 4

[Bob Silverberg [:bsilverberg]]

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?

Comment 5

[Caitlin Neiman [:caitmuenster]]

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#

Comment 6

[Thomas Wisniewski]

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).

Comment 7

[Bob Silverberg [:bsilverberg]]

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

[Thomas Wisniewski]

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.

Comment 9

[Bob Silverberg [:bsilverberg]]

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.

Comment 10

[Bob Silverberg [:bsilverberg]]

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?

Comment 11

[Andy McKay]

:baku maybe?

Comment 12

[Bob Silverberg [:bsilverberg]]

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.

Comment 13

[Michael Smith [:mismith] (non-bmo mail: [e-mail])]

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.

Comment 14

[Bob Silverberg [:bsilverberg]]

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.

Comment 15

[Thomas Wisniewski]

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!

Comment 16

[Thomas Wisniewski]

(Ah, right, we already have "since" support for cookies, so it would only be "hostnames" that I would split out).

Comment 17

[Kenny D]

: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

Comment 18

[Thomas Wisniewski]

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.

Comment 19

[Lusito]

Anyone interested in this might also be interested in Bug 1464895.

Comment 20

[Tom S [:evilpie]]

With bug 1551301 and bug 1632990 fixed we can now clear indexedDB and ServiceWorkers by hostname. What is the most important missing one now? I suspect caches, which is also supported by Chrome via origins.

Comment 21

[Lusito]

Awesome, thank you very much for these fixes! These will come for FF 77? I'm assuming I can try them on nightly?

These are currently not possible yet:

• Cache
• Plugin Data
• Form Data
• Passwords

I'd say in this order (top to bottom)

What's also not possible right now as it seems it to clear session storage. Not sure if this is supposed to be cleared together with local storage.

Also, Firefox Mobile (Fennec) currently doesn't support these cleanup options:

• indexedDB
• ServiceWorkers
• Local Storage
• History (+)
• Cache
• Session Storage (+)
• Passwords (+)
• Plugin Data (+), though not sure if this is even relevant for mobile
• Form Data

The ones marked with (+) are not possible to clear at all in Fennec.
Though I guess it might be best to focus the attention on Firefox Preview instead.

Comment 22

[Tom S [:evilpie]]

(In reply to Lusito from comment #21)

Awesome, thank you very much for these fixes! These will come for FF 77? I'm assuming I can try them on nightly?

Yes this is coming in Firefox 77. You can try this in Nightly and Beta at this point.

These are currently not possible yet:

Thanks for you list. I think we can tackle Cache and Plugin Data next.

Comment 23

[Tom S [:evilpie]]

With bug 1636784, cache and pluginData can now be cleaned up by hostname. This is going to be available in Firefox 78 and already in Nightly of course.

Comment 24

[Lusito]

Very nice. Thanks!

Would you mind taking a look at the related bug1353726 after this one? These two have been the biggest limitations for cleaning extensions and the other one would really help with creating container-specific rules.