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

Proposal: document browser.storage.managed initialization semantics and provide initialization event #547

Open
twschiller opened this issue Feb 15, 2024 · 6 comments
Labels
enhancement Enhancement or change to an existing feature proposal Proposal for a change or new feature

Comments

@twschiller
Copy link

twschiller commented Feb 15, 2024

Summary

Problem:

  • browser.storage.managed provides policies to extensions via Google Workspace Policy/Windows GPO/ADMX/etc.
  • It appears that browser.storage.managed is not guaranteed to be set for extension installation/startup (e.g., on force install in a VDI environment)
  • There's no way to distinguish an empty browser.storage.managed policy vs. a policy that has not been initialized yet. Both seem to return {}. Update: Firefox appears to return undefined if not set
  • It doesn't appear that browser.storage.onChange is fired on initialization
  • Currently, extensions resort to hacks to try to handle the race condition: 1) waiting/retrying for 2-4 seconds, 2) automatically reloading the extension on installation
  • Policies often are used to control extension startup behavior (e.g., login screens / first run screens), so there needs to be formally defined installation/startup semantics

Proposal:

  • Document the browser.storage.managed initialization semantics/guarantees across browser vendors
    • If storage.managed should always be available at extension startup, fix the bug in Chromium
  • Document the browser.storage.onChanged behavior for initialization across browser vendors
  • If storage.managed might not be initialized on startup, provide an initialization getter and event for listening for initialization

Proposed API

browser.storage.managed {
  isInitialized: () => Promise<boolean>;
  onInitialized: Events.Event<({items: Record<string, unknown>}) => void>;
}

It would be convenient if onInitialized provided the initial values, but is unnecessary because the handler can retrieve the values in its handler (and I don't anticipate any relevant race conditions).

isInitialized is not strictly necessary if onInitialized event is guaranteed to fire at a time where the background worker has an opportunity to be listening

Current Workarounds/Hacks

Here are the current hacks that open-source browser extension use:

Vendor Behavior Inconsistency

Related Information

@twschiller twschiller changed the title Proposal: document storage.managed initialization semantics and provide initialization indicator Proposal: document browser.storage.managed initialization semantics and provide initialization indicator Feb 15, 2024
@twschiller twschiller changed the title Proposal: document browser.storage.managed initialization semantics and provide initialization indicator Proposal: document browser.storage.managed initialization semantics and provide initialization event Feb 15, 2024
@ghostwords
Copy link

I've since updated Privacy Badger's workaround a bit: EFForg/privacybadger@2ac8b01?w=1

Firefox, unlike Chrome, promptly returns undefined when managed storage is not set, so there is no need for a workaround in Firefox.

As for where the Chromium managed storage race condition bug comes from: I did a bisect a while ago that led me to https://chromium.googlesource.com/chromium/src/+/1b2aef7ac8b8b5422c5ea7f02352059ab28e05ee%5E%21/

@fregante
Copy link

@ghostwords would you be able to publish that logic as a standalone module? The issue is a few years old and it would be nice to have a centralized solution for others to use/improve.

@erosman
Copy link

erosman commented Feb 20, 2024

See also: Inconsistency: storage.managed

@xeenon xeenon added enhancement Enhancement or change to an existing feature proposal Proposal for a change or new feature and removed needs-triage labels Feb 29, 2024
@Rob--W Rob--W added the follow-up: chrome Needs a response from a Chrome representative label Mar 14, 2024
@oliverdunk
Copy link
Member

I ran several tests today to try and reproduce the mentioned behavior on both macOS and Linux. Even after multiple tries and doing various things to speed up initialization (disabling welcome prompts etc.), I was never able to access managed storage before the values had been loaded.

Would someone who has experience reproducing this be able to take a look and see if they can still do so?

I spoke to @rdcronin about this in general, and if there are situations where we can reasonably know we haven't finished loading the values, we are supportive of waiting to resolve the get() call until values are ready.

This will probably end up being best handled in the Chromium bug tracker but will leave this open for now.

@oliverdunk oliverdunk removed the follow-up: chrome Needs a response from a Chrome representative label Jul 1, 2024
@twschiller
Copy link
Author

twschiller commented Jul 2, 2024

I ran several tests today to try and reproduce the mentioned behavior on both macOS and Linux. Even after multiple tries and doing various things to speed up initialization (disabling welcome prompts etc.), I was never able to access managed storage before the values had been loaded.

@oliverdunk thanks for investigating! Are you suggesting that the current implementation might wait to resolve until the values are available (if there are values set)?

Did you try force-installing the extension? In our case, we're deploying in enterprise VDI environments where they're force installing and setting managed storage via GPO on Windows.

Our team will try creating reproduction steps, but it likely won't be until next week after the US holiday

@oliverdunk
Copy link
Member

@oliverdunk thanks for investigating! Are you suggesting that the current implementation might wait to resolve until the values are available (if there are values set)?

I couldn't immediately see code for that, but it did seem to be happening in practice.

Did you try force-installing the extension? In our case, we're deploying in enterprise VDI environments where they're force installing and setting managed storage via GPO on Windows.

I was adding a test extension as a command line flag. I don't think that would be meaningfully different, but that could be one thing to try.

Our team will try creating reproduction steps, but it likely won't be until next week after the US holiday

Thanks, and no worries! I'll wait to hear how it goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or change to an existing feature proposal Proposal for a change or new feature
Projects
None yet
Development

No branches or pull requests

7 participants