-
Notifications
You must be signed in to change notification settings - Fork 56
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: Event priority in Background Service Worker or Event page #464
Comments
I was just fixing a race condition caused by this in Chrome yesterday. Our extension clears some data on extension update since it's version-specific but new data was already written before the |
Event listeners are triggered after the background script's first event loop task fully completes. This is consistent with the behavior of service workers per the specification (which was derived from extensions background scripts). AFAIK, extensions always behave like that. In your example your static code is a part of the first event loop task, so it always runs first. The event listeners run either at the beginning of the next event loop task or at the very end of the first one, after the microtask queue fully depletes. Changing this behavior would arguably break a lot of extensions, so this should be an opt-in. Modern JS frameworks have something similar: when watching an observable you can specify that your callback will run right now, immediately. |
I would agree with @tophf's comment that this seems to be working as intended. @mnoorenberghe, that definitely seems like an interesting use case, happy to keep discussing in the other issue. |
The execution order is determined by the JavaScript language itself (the language specification and implementation), there is no magic. So if you place Web service worker has two lifecycle (installation or upgrade) related events, If you really want to ensure the priority mentioned above, I think of two proposals.
|
PreambleLogically, ProposalSince some of the values for
The same API can be used e.g. const state = browser.runtime.onInstalled.state;
if (state === 'instal' || state === 'update') {
// do something
} |
I’d be curious whether or not the current asynchronous approach is required for implementation specific reasons, but I’ll have to deferred to browser engineers to answer that question. Given the current platform capabilities. The following should address the basic pattern described in @erosman’s last comment. I should note that I’m currently on my phone on an airplane, so I have not tested this logic. const statePromise = new Promise((resolve, reject) => {
const installHandler = (reason) => {
if (browser.runtime.lastError) {
return reject(browser.runtime.lastError);
}
resolve(reason);
browser.runtime.onInstalled.removeListener(installHandler);
}
});
async main() {
let state = await statePromise;
if (state === 'install' || state === 'update') {
// do something
}
}
main(); |
I think @hanguokai's comment is a pretty good summary of where we're at today. @dotproto's snippet is a good workaround that should hopefully provide a solution in the short term. I think the ideas around a |
@dotproto's solution .... Here is a simple example ... let state;
// Note: runtime.onInstalled.addListener does not fire every time a service-worker/Event-page is executed
browser.runtime.onInstalled.addListener(details => {
// set state
state = details.reason;
if (details.reason === 'instal' || details.reason === 'update') {
// do something
}
});
async function main() {
// this MUST run BEFORE checking for onInstalled.addListener result
let pref = await browser.storage.local.get();
// do something else
// this MUST run AFTER checking for onInstalled.addListener result
// do something to the pref only if onInstalled.addListener has fired and state === 'update'
if (state === 'update') {
// do something to pref
pref = ImportedMigrate(pref);
}
// this MUST run AFTER all above
// send pref to an imported function
ImportedFunction(pref);
}
main(); @hanguokai's solution ....Solution 1browser.runtime.onInstalled.addListener(listener, {runImmediately: true}); Besides the conceptual ambiguity of a listener that behaves as a synchronous function, it would necessitate a 2nd listener. // check if the reason was install, update, or enable
browser.runtime.onInstalled.addListener(listener, {runImmediately: true});
// need another listener for uninstall, disable
browser.runtime.onInstalled.addListener(listener2); Solution 2Providing synchronous data, is similar to the amended proposal in #464 (comment). |
I guess that browsers would not allow the
Yes, your amended proposal is similar to my proposal that I submitted 30 minutes before you. This is a feasible solution, regardless of what name used. Note: there is a "update&enable" state. In that case, the two states overlap. |
It appears that currently, there is no specific order that can be relied on. For example:
There is no guarantee that by the time
if (this.showHelp)
is reached,onInstalled
event has fired.It might be worthwhile to consider an event priority in the background script, especially for
runtime.onInstalled
. For example:import
runtime.onInstalled
Update
Please refer to #464 (comment) for the updated proposal.
The text was updated successfully, but these errors were encountered: