1414997 - ExtensionStorageSync might initialize Kinto multiple times

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

This function does an await before setting the value it's waiting for, so if you called it twice in a row it would initialize twice. http://searchfox.org/mozilla-central/source/toolkit/components/extensions/ExtensionStorageSync.jsm#339

It's called transitively from functions called by webextensions, so this can happen in practice, and has been reported in IRC.

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

I don't like not having any insight into the extension-storage sync engine, and since I'm the reporter, I wrote a patch for this.

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1414997 - Ensure extension storage sync initializes Kinto exactly once

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

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

For what it's worth, while the attached test fails without the patch (and it fails rather hard), bug 1415012 is the actual issue the reporter in IRC was experiencing (it wasn't multiple instances of Kinto, it was just that everything was starting a new sync as soon as the previous finished).

Comment 4

[Ethan Glasser-Camp (:glasserc)]

Comment on attachment 8925744 [details]
Bug 1414997 - Ensure extension storage sync initializes Kinto exactly once

https://reviewboard.mozilla.org/r/196890/#review202284

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea8ee7aee5b8
Ensure extension storage sync initializes Kinto exactly once r=glasserc

Comment 6

[Eliza Balazs [:ebalazs_]]

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

Comment 7

[marius.santa]

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.

Comment 8

[Thom Chiovoloni [:tcsc] (ex-moco)]

No, the automated testing is sufficient, and no extension has hit this yet (AFAIK). Thanks!