Closed Bug 1361965 Opened 3 years ago Closed 3 years ago

Provide access to a formautofill "storage" singleton

Categories

(Toolkit :: Form Manager, enhancement)

Product:
Toolkit
Component:
Form Manager
Type:
enhancement
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: markh, Assigned: markh)

Details

Attachments

(1 file)

Bug 1361965 - Provide access to a formautofill storage singleton.
59 bytes, text/x-review-board-request
lchang
: review+
Details 
Sync and other features require access to the same storage manager used by formautofill itself. On IRC we discussed the simplest and most reasonable way is to expose the singleton directly via ProfileStorage.jsm.

I'm about to push a patch I'd like feedback on, which does this. It also fixes a bit of confusion about the init methods returning promises.
Comment on attachment 8864426 [details]
Bug 1361965 - Provide access to a formautofill storage singleton.

As briefly discussed in IRC (although we didn't discuss exactly what name to expose - happy to change it if desired). It also fixes a bit of confusion about the init methods returning promises.
Attachment #8864426 - Flags: feedback?(schung)
Attachment #8864426 - Flags: feedback?(lchang)
Attachment #8864426 - Flags: feedback?(MattN+bmo)
Comment on attachment 8864426 [details]
Bug 1361965 - Provide access to a formautofill storage singleton.

https://reviewboard.mozilla.org/r/136110/#review139116

::: browser/extensions/formautofill/ProfileStorage.jsm:125
(Diff revision 1)
>     * @returns {Promise}
>     * @resolves When the operation finished successfully.
>     * @rejects  JavaScript exception.
>     */
>    initialize() {
> +    if (!this._initializePromise) {

Oh - I meant to mention this: MattN suggested on IRC we make the .init() method be safe to call multiple times, and each consumer should do so once - hence this change.
Comment on attachment 8864426 [details]
Bug 1361965 - Provide access to a formautofill storage singleton.

Looks great! Thanks.
Attachment #8864426 - Flags: feedback?(lchang) → feedback+
Comment on attachment 8864426 [details]
Bug 1361965 - Provide access to a formautofill storage singleton.

I just triggered a try build to see if there's any missing part in xpcshell tests, but overall looks fine to me.
Attachment #8864426 - Flags: feedback?(schung) → feedback+
Comment on attachment 8864426 [details]
Bug 1361965 - Provide access to a formautofill storage singleton.

https://reviewboard.mozilla.org/r/136110/#review139562

Looks great!

::: browser/extensions/formautofill/FormAutofillParent.jsm:67
(Diff revision 1)
> -  init() {
> +  async init() {
>      log.debug("init");
> -    let storePath = OS.Path.join(OS.Constants.Path.profileDir, PROFILE_JSON_FILE_NAME);
> +    await profileStorage.initialize();

Heh, just realized we have the inconsistency of init vs. initialize…
Attachment #8864426 - Flags: feedback?(MattN+bmo) → feedback+
Assignee: nobody → markh
Attachment #8864426 - Flags: review?(lchang)
Comment on attachment 8864426 [details]
Bug 1361965 - Provide access to a formautofill storage singleton.

https://reviewboard.mozilla.org/r/136110/#review140070

Thanks for the patch.

BTW, there are a few lint errors that need to be fixed before landing.
Attachment #8864426 - Flags: review?(lchang) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 67f735cb2b34 -d c3ab38db798e: rebasing 394505:67f735cb2b34 "Bug 1361965 - Provide access to a formautofill storage singleton. r=lchang" (tip)
merging browser/extensions/formautofill/FormAutofillParent.jsm
merging browser/extensions/formautofill/ProfileStorage.jsm
merging browser/extensions/formautofill/test/unit/test_profileStorage.js
merging browser/extensions/formautofill/test/unit/test_transformFields.js
merging tools/lint/eslint/modules.json
warning: conflicts while merging browser/extensions/formautofill/FormAutofillParent.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s ebd8fc3041be -d bcffb170c571: rebasing 394516:ebd8fc3041be "Bug 1361965 - Provide access to a formautofill storage singleton. r=lchang" (tip)
merging browser/extensions/formautofill/FormAutofillParent.jsm
merging browser/extensions/formautofill/ProfileStorage.jsm
merging browser/extensions/formautofill/test/unit/test_profileStorage.js
merging browser/extensions/formautofill/test/unit/test_transformFields.js
merging tools/lint/eslint/modules.json
warning: conflicts while merging browser/extensions/formautofill/FormAutofillParent.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 3dadb1917076 -d 5c7b1cecbe78: rebasing 394509:3dadb1917076 "Bug 1361965 - Provide access to a formautofill storage singleton. r=lchang" (tip)
merging browser/extensions/formautofill/FormAutofillParent.jsm
merging browser/extensions/formautofill/ProfileStorage.jsm
merging browser/extensions/formautofill/test/unit/test_profileStorage.js
merging browser/extensions/formautofill/test/unit/test_transformFields.js
merging tools/lint/eslint/modules.json
warning: conflicts while merging browser/extensions/formautofill/FormAutofillParent.jsm! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by mhammond@skippinet.com.au:
https://hg.mozilla.org/integration/autoland/rev/fa7d4c3714a1
Provide access to a formautofill storage singleton. r=lchang
https://hg.mozilla.org/mozilla-central/rev/fa7d4c3714a1
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.