localStorage in background script seems to be temporary

RESOLVED FIXED in Firefox 45

Status

()

Toolkit
WebExtensions: Untriaged
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: evilpie, Assigned: billm)

Tracking

unspecified
mozilla45
Points:
---
Dependency tree / graph
Bug Flags:
blocking-webextensions +

Firefox Tracking Flags

(firefox45 fixed)

Details

(Whiteboard: [storage])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
(Assignee)

Comment 1

2 years ago
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.

Updated

2 years ago
Whiteboard: [storage]
(Assignee)

Updated

2 years ago
Blocks: 1213990

Comment 2

2 years ago
(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)
(Assignee)

Comment 3

2 years ago
I think we just need to fix this bug. Would that work for you?
Flags: needinfo?(wmccloskey)
(Assignee)

Updated

2 years ago
Blocks: 1214433
Priority: -- → P1

Updated

2 years ago
Flags: blocking-webextensions+
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1218756
(Assignee)

Comment 5

2 years ago
Created attachment 8685683 [details] [diff] [review]
patch

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+

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e61ae324d13

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9e61ae324d13
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45

Updated

2 years ago
Iteration: --- → 45.1 - Nov 16
You need to log in before you can comment on or make changes to this bug.