Closed Bug 1208874 Opened 9 years ago Closed 9 years ago

localStorage in background script seems to be temporary

Categories

(WebExtensions :: Untriaged, defect, P1)

defect

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Iteration:
45.1 - Nov 16
Tracking Status
firefox45 --- fixed

People

(Reporter: evilpie, Assigned: billm)

References

Details

(Whiteboard: [storage])

Attachments

(1 file)

It looks like RES uses localStorage in its background script, which also seems to be reset everytime. I wonder why they don't use chrome.storage.
This is caused by the fact that we regenerate the UUID that goes in moz-extension://<uuid>/ every startup. We should probably generate it once per install or something like that. Another option would be to hash the telemetry client ID together with the extension ID.
Whiteboard: [storage]
(In reply to Tom Schuster [:evilpie] from comment #0)
> I wonder why they don't use chrome.storage.

Not sure about RES, but a strong case for localStorage vs chrome.storage is synchronous access.

Is there any sure-fire mean for an extension to read its persistent configuration *before* anything "interesting" happens in content-land?

This is especially painful for content blocking extensions of course: as far as I know while this can be worked around in background pages (via localStorage), but the situation is far worse in content scripts, which in Chromium cannot send synchronous messages to retrieve their settings (one of the reasons there's no NoScript for Chromium).

Bill, is there anything I'm missing?
Have we got something already, or a plan for early configuration retrieval in WebExtensions?
Flags: needinfo?(wmccloskey)
I think we just need to fix this bug. Would that work for you?
Flags: needinfo?(wmccloskey)
Blocks: webext
Priority: -- → P1
Flags: blocking-webextensions+
Attached patch patchSplinter Review
This takes a pretty direct approach. I used a pref to store the extension ID -> UUID mapping. One downside is that nothing ever gets removed from this pref. That can be fixed in bug 1213990.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8685683 - Flags: review?(kmaglione+bmo)
Comment on attachment 8685683 [details] [diff] [review]
patch

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

::: toolkit/components/extensions/Extension.jsm
@@ +677,5 @@
> +  let uuid = uuidGenerator.generateUUID().number;
> +  uuid = uuid.slice(1, -1); // Strip of { and } off the UUID.
> +
> +  map[id] = uuid;
> +  Preferences.set(PREF_NAME, JSON.stringify(map));

We should really probably clear this on uninstall. But that would leave orphaned localStorage data, and I think there's still some question about whether we want to clear add-on data on uninstall anyway, so maybe just add a comment and a note to the Chrome incompatibilities docs?
Attachment #8685683 - Flags: review?(kmaglione+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/9e61ae324d13
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Iteration: --- → 45.1 - Nov 16
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: