1466130 - Use nsIClearDataService in Sanitizer.jsm for android

Status: RESOLVED INACTIVE

(Firefox for Android Graveyard :: General, enhancement, P2)

Comment 1

[Andrea Marchesini [:baku]]

Attachment: android.patch

There are several things to do here. Wondering if we should extend nsIClearDataService to cover the delete-specifc features for android.

Comment 2

[Jan Henning [:JanH]]

Comment on attachment 8982524 [details] [diff] [review]
android.patch

Review of attachment 8982524 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/modules/Sanitizer.jsm
@@ +87,5 @@
>          return true;
>        }
>      },
>  
>      // Compared to desktop, we don't clear plugin data, as plugins

Since you've added CLEAR_PLUGIN_DATA anyway [1], maybe amend the comment as well so there's no contradiction?

[1] No harm done and better to be safe I guess, plus technically the H264 GMP plugin still exists even on Android, and potentially any old data left over from people who had used Flash in the past?

Comment 3

[Johann Hofmann [:johannh]]

Considering that we're only changing mobile/ code this should probably live in the Firefox for Android component :)

Comment 4

[Johann Hofmann [:johannh]]

Comment on attachment 8982524 [details] [diff] [review]
android.patch

Review of attachment 8982524 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks good but please also replace https://searchfox.org/mozilla-central/rev/bf81d741ff5dd11bb364ef21306da599032fd479/mobile/android/modules/Sanitizer.jsm#371

It says fennec-only but that's just because desktop doesn't clear passwords, Fennec is still using Services.logins. One important note is that you'll have to also change the ClearDataService implementation of deleteAll (https://searchfox.org/mozilla-central/source/toolkit/components/cleardata/ClearDataService.js#220) to use Services.logins.removeAllLogins() instead of just passing a noop callback. This will avoid showing the MP prompt.

> Wondering if we should extend nsIClearDataService to cover the delete-specifc features for android.

A thought I had was to switch between different cleaners at module load based on runtime flags and implement the fennec-only loaders in a adjacent file.

::: mobile/android/modules/Sanitizer.jsm
@@ +94,5 @@
> +      async clear() {
> +        let refObj = {};
> +        TelemetryStopwatch.start("FX_SANITIZE_COOKIES_2", refObj);
> +        await clearData(Ci.nsIClearDataService.CLEAR_COOKIES |
> +                        Ci.nsIClearDataService.CLEAR_PLUGIN_DATA |

I'm going to trust you and JanH that this is safe to do on Fennec, I personally would have said just drop the flag.

Comment 5

[Rob Wu [:robwu]]

Sanitizer.jsm was removed from mobile/android when Fennec was dropped.

PS. Once of Sanitizer.jsm's consumers was Fennec's implementation of the extension's browsingData API.
This API is now being resurrected in bug 1625233, by reusing part of the implementation from desktop, which already uses nsIClearDataService. Parts of the data clearing API is outside of Gecko, and handled by the embedder (GeckoView / Android-Components / Fenix).