1463956 - Address and CreditCard engines should implement the ability to wipe client data

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement)

Description

[Mark Hammond [:markh] [:mhammond]]

In bug 1409208 we are offering the ability to delete local data when disconnecting from Sync. These engines don't support the methods we need to allow that to happen, so we get a bit of log noise and this data isn't wiped.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: Bug 1463956 - allow Sync to remove all address and credit-card records.

Review commit: https://reviewboard.mozilla.org/r/246314/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246314/

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8980159 [details]
Bug 1463956 - allow Sync to remove all address and credit-card records.

https://reviewboard.mozilla.org/r/246314/#review252440

::: browser/extensions/formautofill/FormAutofillStorage.jsm:1193
(Diff revision 1)
> +  removeEverything() {
>      this._store.data[this._collectionName] = [];
> -    // test-only, so there's no good reason to request a save!
> +    this._store.saveSoon();
>    }

Hmm… I think part of why this was marked test-only is because it doesn't broadcast "formautofill-storage-changed". I think that should be added like we do for `removeAllLogins`[1].

The notification will ensure the any open prefs UI gets updated[2] and the content process gets notified to clear its knowledge of which data types the user has saved[3].

You may need to stop sync from getting in a loop from that though. You may also need to fix tests that now get this extra observer notification.

While you're here, could you fix the test at [4] to actually use `event` for the notification and add the new notification reason string to that array.

[1] https://dxr.mozilla.org/mozilla-central/rev/9055d9d89a4bca5cf48dda789299559aefca4e54/toolkit/components/passwordmgr/storage-json.js#389
[2] https://dxr.mozilla.org/mozilla-central/rev/9055d9d89a4bca5cf48dda789299559aefca4e54/browser/extensions/formautofill/content/manageDialog.js#247
[3] https://dxr.mozilla.org/mozilla-central/rev/9055d9d89a4bca5cf48dda789299559aefca4e54/browser/extensions/formautofill/FormAutofillParent.jsm#143,149
[4] https://dxr.mozilla.org/mozilla-central/rev/9055d9d89a4bca5cf48dda789299559aefca4e54/browser/extensions/formautofill/test/unit/test_savedFieldNames.js#27-29

Comment 3

[Mark Hammond [:markh] [:mhammond]]

I chatted with Matt on IRC and we agreed on a few additional changes I should make while I'm touching this - new patch incoming.

Matt, do you mind having another look?

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8980159 [details]
Bug 1463956 - allow Sync to remove all address and credit-card records.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246314/diff/1-2/

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8980159 [details]
Bug 1463956 - allow Sync to remove all address and credit-card records.

https://reviewboard.mozilla.org/r/246314/#review252680

Awesome! Thanks a lot!

Comment 6

[Lina Butler [:lina]]

Comment on attachment 8980159 [details]
Bug 1463956 - allow Sync to remove all address and credit-card records.

https://reviewboard.mozilla.org/r/246314/#review252752

Nice!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce5992154d85
allow Sync to remove all address and credit-card records. r=kitcambridge,MattN

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

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