1489669 - GV API to selectively delete user data using GeckoSession (Clear User Data API)

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P1)

Description

[Jonathan Almeida [:jonalmeida]]

Looking at how Fennec clears data, I can see this is done via the event dispatcher[1]. Since this isn't the recommended way that we would like to do it in Android Components, a newer API would be nice.

For our current requirements, a catch-all method that deletes everything could work, but it's quite possible that we would want to eventually need something with more finer controls (e.g. delete only cookies).

Components specific discussions can be followed there as well[2].

[1]: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/preferences/PrivateDataPreference.java#61
[2]: https://github.com/mozilla-mobile/android-components/issues/644

Comment 1

[Jonathan Almeida [:jonalmeida]]

Needed for r-b and fenix in the future.

Probably a way to select which parts of data to be cleared similar to fennec (with an new API):

GeckoBundle().apply {
    listOf(
        "openTabs",
        "history",
        "searchHistory",
        "downloadFiles",
        "formdata",
        "cookies_sessions",
        "cache",
        "offlineApps",
        "siteSettings",
        "syncedTabs",
        "passwords"
    ).map {
        putBoolean(it, true)
    }
}.also { bundle ->
    geckoSession.eventDispatcher.dispatch("Sanitize:ClearData", bundle)
}

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

Needed for r-b and fenix in the future.

Flagging Chris to add the right whiteboard labels and priorities.

Comment 3

[Chris Peterson [:cpeterson]]

[geckoview:fenix:p2] nice-to-have for Fenix MVP unless someone speaks up.

See r-b #81 for the UI work.

Comment 4

[Chris Peterson [:cpeterson]]

Adding [geckoview:fenix:m5] whiteboard tag because the Fenix team would like this API for Fenix MVP.

https://github.com/mozilla-mobile/android-components/issues/1798
https://github.com/mozilla-mobile/fenix/issues/225
https://github.com/mozilla-mobile/fenix/issues/980

Comment 5

[Chris Peterson [:cpeterson]]

Eugen, can you take a look at this bug for Fenix MVP?

The Fenix team would like a user option to clear all user data for Fenix MVP: mozilla-mobile/fenix#225. Selectively clearing data for individual sites or data types is not needed for Fenix MVP, but snorp says that we should probably go ahead and implement the GV API for selective clearing now.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Chris Peterson [:cpeterson] from comment #5)

Eugen, can you take a look at this bug for Fenix MVP?

The Fenix team would like a user option to clear all user data for Fenix MVP: mozilla-mobile/fenix#225. Selectively clearing data for individual sites or data types is not needed for Fenix MVP, but snorp says that we should probably go ahead and implement the GV API for selective clearing now.

I think what I was getting at is that we need Sanitizer.jsm or something like it exposed in GV. But for the public API maybe we'd just start with a "nuke everything from orbit" API and add the selections later.

Comment 7

[Chris Peterson [:cpeterson]]

James is concerned that GV's sanitizer stays in sync with desktop if new user data types are added.

Comment 8

[Eugen Sawin [:esawin]]

I think we could provide a powerful API here by exposing a subset of nsIClearDataService through our StorageController API.

Proposed API:

StorageController {
  ClearFlags {
    COOKIES
    NETWORK_CACHE
    IMAGE_CACHE
    HISTORY
    DOM_STORAGES
    AUTH_SESSIONS
    PERMISSIONS
    ALL_CACHES
    SITE_SETTINGS
    SITE_DATA
    ALL // 0xFFFFFF so shouldn't be affected by changing or new flags
  }

  void clearDataFromHost(String host, ClearFlags flags)

  void clearDataInTimeRange(Time from, Time to, ClearFlags flags)

  void clearData(ClearFlags flags)
}

The ClearFlags selection would be composed based on desktop's Sanitizer setup to provide a powerful yet not overloaded API.

The ClearFlags constants would need to maintain a consistent mapping to nsIClearDataService, which we hopefully can monitor through tests. Any additions to nsIClearDataService would need to be exposed through the GV API, if applicable.

Baku and snorp, do you see any issues with exposing this API to app consumers and would you agree with the proposed flag selection?

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Looks good to me!

Comment 10

[Andrea Marchesini [:baku]]

It seems a good idea. I have a couple of comments:

void clearDataFromHost(String host, ClearFlags flags)

This is fine, but:

1. what about OriginAttributes? How to delete data for 1 particular container, for instance, for 1 particular GeckoView session ID?

2. We need good documentation here, because this method would delete data for 1 single origin and I suspect it could be used wrongly: for instance, if we have 1 tab open, when deleting the first-party origin, firefox doesn't delete all the third party origins...
   This is actually an existing bug in 'forget about site'.

void clearDataInTimeRange(Time from, Time to, ClearFlags flags)

The time range is not fully supported by all the flags/storages. For some of them, we have to delete all. In your list, time range is not supported by: network cache, image cache, dom storages and auth sessions.

Comment 11

[Eugen Sawin [:esawin]]

(In reply to Andrea Marchesini [:baku] from comment #10)

It seems a good idea. I have a couple of comments:

void clearDataFromHost(String host, ClearFlags flags)

This is fine, but:

1. what about OriginAttributes? How to delete data for 1 particular container, for instance, for 1 particular GeckoView session ID?

Sorry for the confusion, I've left it out because that's already been reviewed in bug 1501108. I'm actually moving the clearSesssionContextData API out of bug 1501108 into this.

2. We need good documentation here, because this method would delete data for 1 single origin and I suspect it could be used wrongly: for instance, if we have 1 tab open, when deleting the first-party origin, firefox doesn't delete all the third party origins...
   This is actually an existing bug in 'forget about site'.

Interesting, is there a way around that bug? Maybe you can link me to the existing bug so that I can make sure the docs reflect that properly.

void clearDataInTimeRange(Time from, Time to, ClearFlags flags)

The time range is not fully supported by all the flags/storages. For some of them, we have to delete all. In your list, time range is not supported by: network cache, image cache, dom storages and auth sessions.

I can add that to the docs, or we could leave this out of the API completely, since it isn't part of the current requirement.

Thanks!

Comment 12

[Eugen Sawin [:esawin]]

Attachment: Bug 1489669 - [1.0] Add Storage Controller API.

Comment 13

[Eugen Sawin [:esawin]]

Attachment: Bug 1489669 - [2.0] Add test for Storage Controller API.

Depends on D31329

Comment 14

[Eugen Sawin [:esawin]]

Attachment: Bug 1489669 - [1.4] Add Storage Controller API.

Comment 15

[Eugen Sawin [:esawin]]

Attachment: Bug 1489669 - [2.2] Add test for Storage Controller API.

Depends on D32153

Comment 16

[Eugen Sawin [:esawin]]

Removed the time range-based clearing because it has too many implementation restrictions that would need to be communicated and enforced.
Removed the session context-based clearing because it's still blocked on bug 1501108.

Moved the clear flags mapping into the JSM module to help with keeping the values in sync.

Extended and fixed tests. We can't load from the local test path due to the URI scheme restrictions and setting/getting localStorage or cookies via evaluateJS yielded a security error.
The tests now depend on Agi's WebExt support for evaluateJS to fix that.

Comment 17

[Eugen Sawin [:esawin]]

Landing the API patch now to unblock Fenix, will land test patch once bug 1553515 lands.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1912d221317a
[1.4] Add Storage Controller API. r=baku,snorp

Comment 19

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/1912d221317a

Comment 20

[Eugen Sawin [:esawin]]

Comment on attachment 9066725 [details]
Bug 1489669 - [1.4] Add Storage Controller API.

Beta/Release Uplift Approval Request

• User impact if declined: This a Fenix MVP release blocker.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This adds a new GeckoView API without affecting existing features.
  There are automated tests (second patch on this bug) and they pass, but they depend on bug 1553515 to land.
• String changes made/needed: None

Comment 21

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9066725 [details]
Bug 1489669 - [1.4] Add Storage Controller API.

new gv API, approved for 68.0b5

Comment 22

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d6c54c50418c

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b310943c988b
[2.2] Add test for Storage Controller API. r=baku,snorp

Comment 24

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/b310943c988b