[go: nahoru, domu]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discuss limits applied to storage.session API #350

Open
oliverdunk opened this issue Feb 10, 2023 · 13 comments
Open

Discuss limits applied to storage.session API #350

oliverdunk opened this issue Feb 10, 2023 · 13 comments
Labels
implemented: chrome Implemented in Chrome implemented: safari Implemented in Safari supportive: firefox Supportive from Firefox

Comments

@oliverdunk
Copy link
Member

storage.session allows extensions to store a small amount of data in memory which persists across background lifetimes. This can be useful for extensions such as password managers which need to store unlock secrets (which should never be persisted to disk) or other extensions which have session data like an authentication token from a server.

Since this API is intended for small amounts of sensitive data, and uses memory which is often a limited resource, the API in Chrome is currently limited to 1MB of data. However, we've had feedback that an increase would be beneficial. We've discussed with other browser vendors, and we're supportive of increasing the limit to 10MB.

There could also be some interesting discussion here about how sizes are calculated but there is not an intention to block on that short term.

Opening this to discuss further and reach some alignment.

@oliverdunk oliverdunk added agenda Discuss in future meetings supportive: chrome Supportive from Chrome follow-up: safari Needs a response from a Safari representative follow-up: firefox Needs a response from a Firefox representative labels Feb 10, 2023
@frivain
Copy link
frivain commented Feb 10, 2023

Hi @oliverdunk my understanding is that Google was already planning to increase the limitation based on https://developer.chrome.com/docs/extensions/mv3/known-issues/#increased-session-storage-quota
Do we have more details about this?

@oliverdunk
Copy link
Member Author

@frivain Yeah, exactly. We (Google) have tentatively settled on 10MB but wanted to raise it in the group before finalising a decision there 🙂

(In case the affiliation is confusing, I recently moved from 1Password to Google and am now working in DevRel there)

@xeenon
Copy link
Collaborator
xeenon commented Feb 10, 2023

Safari recently bumped the limit to 10 MB in Safari Technology Preview 163.

@xeenon xeenon added implemented: safari Implemented in Safari and removed follow-up: safari Needs a response from a Safari representative labels Feb 10, 2023
@Rob--W Rob--W added supportive: firefox Supportive from Firefox follow-up: safari Needs a response from a Safari representative implemented: safari Implemented in Safari and removed implemented: safari Implemented in Safari follow-up: firefox Needs a response from a Firefox representative follow-up: safari Needs a response from a Safari representative labels Feb 10, 2023
@Rob--W
Copy link
Member
Rob--W commented Feb 10, 2023

Firefox has yet to implement the storage.session API (https://bugzilla.mozilla.org/show_bug.cgi?id=1687778), but we're supportive of a 10 MB quota.

@tophf
Copy link
tophf commented Feb 13, 2023

Just because the motivational case was about a password, it's not the only case that needs a solution. Conceptually, this API should be considered a replacement for the variables of a persistent background script. Ideally it could hibernate the entire JS heap, but it's unlikely to happen any time soon, so for now the API should at least support all structured cloneable types by design even if the other API in chrome.storage family don't support it (e.g. in Chrome) because the serialized string is 2-10 times bigger than the internal binary data, which quickly exhausts the limits. Also, to serialize binary things like Uint8Array or ArrayBuffer/Blob extensions have to waste a lot of time in JS even for 1MB of data (~20ms on a moderately fast PC).

@tophf
Copy link
tophf commented Feb 14, 2023

Maybe the limit should be dynamic and depend on the amount of available RAM, the extensions should query the limit by using a new method chrome.storage.session.getQuota or getAvailable or getBytesAvailable. The same should probably apply to all storage areas - because this is how browsers already behave in regards to allocating disk storage. I've heard of corporate setups where users had hundreds of force-installed extensions (auto-generated, I presume), so multiply it by 10MB and you'll get gigabytes of mandatory RAM allocation, which would be infeasible on a lower tier device. On a beefier device (32GB+ of RAM isn't that rare) with just a few extensions installed I would expect the limit to be [much] higher than 10MB, as well as the default SW kill timer to be much longer than 30 seconds.

@lukeselker
Copy link

Very excited about the support behind this! Do we know in what Chrome version the upgraded 10MB limit might be available?

@bershanskiy
Copy link
Member

Out of curiosity, is there anything preventing extensions from storing lots of data in storage.local and then storing only encryption keys in storage.session? I understand the concern about latency associated with extra processing, but is it really that significant? From my experience, I most of the time I am able to find a way (case-specific) to prefetch data before it is actually needed.

@tophf
Copy link
tophf commented Feb 17, 2023

Even if we disregard the extra time to encrypt/decrypt the data, storing volatile data to disk is 1000+ times slower than storing it in-memory, it unnecessarily reduces overall performance of the browser and computer, it also wears down an SSD and incurs significant latencies on an HDD once the data is flushed.

ManifestV3 was designed to increase performance by switching to a service worker, but this won't be true for extensions that have to waste time and resources to rebuild a complex state every time the SW restarts in case it happens frequently i.e. every minute.

@oliverdunk
Copy link
Member Author

Do we know in what Chrome version the upgraded 10MB limit might be available?

Nothing to share at the moment. Given it's just bumping a limit I'm hoping we'll be able to do it soon.

Out of curiosity, is there anything preventing extensions from storing lots of data in storage.local and then storing only encryption keys in storage.session? I understand the concern about latency associated with extra processing, but is it really that significant?

Nothing prevents extensions from doing that, but I do think the latency can be significant. For example, if an extension wants to know if it has data relating to a specific website, this is the difference between decrypting a wide range of data or just looking at an in-memory list of domains.

aarongable pushed a commit to chromium/chromium that referenced this issue Feb 22, 2023
Increase the memory limit of chrome.storage.session from 1MB
(1048576 bytes) to 10MB (10485760 bytes), following discussions in
the Web Extensions Community Group:
w3c/webextensions#350

Bug: 1385167
Change-Id: I1082744ef44f158ac9ca855643f9f8d4486b7acf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4277142
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Emilia Paz <emiliapaz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108638}
@Rob--W Rob--W added implemented: chrome Implemented in Chrome and removed supportive: chrome Supportive from Chrome labels Feb 24, 2023
@Rob--W
Copy link
Member
Rob--W commented Feb 24, 2023

Chrome has implemented this: https://bugs.chromium.org/p/chromium/issues/detail?id=1385167

It will be available in Chrome 112.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implemented: chrome Implemented in Chrome implemented: safari Implemented in Safari supportive: firefox Supportive from Firefox
Projects
None yet
Development

No branches or pull requests

9 participants