-
Notifications
You must be signed in to change notification settings - Fork 68
Proposal: runtime.onInvalidated
event
#792
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
base: main
Are you sure you want to change the base?
Proposal: runtime.onInvalidated
event
#792
Conversation
94dca27
to
bbfe5ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Carlos! Left some initial comments.
7e295ab
to
57258f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (with nit around optional field), adding Devlin to review as well.
57258f4
to
dff9ba7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Carlos! Just a few small comments on this one, otherwise, LGTM
runtime.onInvalidated
event
proposals/runtime_on_invalidated.md
Outdated
|
||
### Objective | ||
|
||
This event allows extension contexts which stay alive to be notified when the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"extension context" is very generic, while this seems to primarily be targeting content scripts. Can we restrict this to content scripts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rob--W it is generic on purpose as to cover both content scripts and iframed extension contexts on third party websites. These iframes stay alive on some browsers and require this event to handle their state. The objective states contexts which stay alive
to exclude extension contexts which get killed on invalidation.
Updated the objective to be more explicit on this behaviour.
Co-authored-by: Timothy Hatcher <[email protected]>
Co-authored-by: Timothy Hatcher <[email protected]>
Co-authored-by: Timothy Hatcher <[email protected]>
Co-authored-by: Rob Wu <[email protected]>
What are the next steps for this PR? Seems to be approved but not merged? |
@Rob--W can you do a final review / give approval for this proposal? |
Proposal for #138 and #789
It introduces a new event which fires in contentScripts and extension frames when the background scripts get invalidated. In the event is a reason property stating the reason for the invalidation.
@bershanskiy @fregante love to hear your feedback.