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

Clarification: package uses onExtensionStart (not chrome.runtime.onInstalled) #16

Closed
twschiller opened this issue May 23, 2024 · 6 comments

Comments

@twschiller
Copy link

twschiller commented May 23, 2024

Context

  • From the package name, it seems this package should run on extension installation. However, this package uses the custom onExtensionStart event, which runs on extension start
  • Chrome has an chrome.runtime.onInstalled event that runs "when the extension is first installed, when the extension is updated to a new version, and when Chrome is updated to a new version."
  • It sounds like the package is behaving as intended, and that it runs on startup not just installation? (But not on MV3 worker recycle?)
@twschiller twschiller changed the title Clarification: package uses onExtensionStart (not install) Clarification: package uses onExtensionStart (not chrome.runtime.onInstalled) May 23, 2024
@fregante
Copy link
Owner

Yeah the package name is not a 100% match. The purpose of the package is to have the content script available when the extension is running. This means:

  • on install
  • on re-enable
  • on update

Also see a related request to bring this behavior to Chrome:

Are you seeing issues with the MV3 restart? That should have been fixed by #9

@twschiller
Copy link
Author

twschiller commented May 24, 2024

Are you seeing issues with the MV3 restart?

We're seeing reinjection on extension reload, which I wasn't expecting from reading the code, given the name of the package. So just figured I'd raise in case anyone else comes across this behavior

@fregante
Copy link
Owner

fregante commented May 24, 2024

reinjection on extension reload

Yes, that's expected. The content script in pixiebrix is deactivated before the reload (onContextInvalidated) so a new one needs to be injected in order to receive messages, restore menuitems and the various triggers. This is not MV3 specific.

If you're seeing reinjection when the service worker reloads, I should look into #9 again.

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2024
@twschiller
Copy link
Author

Yes, that's expected. The content script in pixiebrix is deactivated before the reload (onContextInvalidated) so a new one needs to be injected in order to receive messages, restore menuitems and the various triggers. This is not MV3 specific.

👍 It was causing a race condition on code that was assumed to run on the next tab to be opened/redirected (and not all existing tabs). We added a fix to address the injection into existing tabs/frames

@fregante
Copy link
Owner

Can you point me to the ticket/PR? I'm not following the sequence of events that leads to the race condition.

All tabs will get the content script in PB, whether via this module or natively via the manifest. PB also protects itself from duplicate injections so I think that shouldn't be an issue either (even if ideally it shouldn't happen at all)

@twschiller
Copy link
Author

twschiller commented May 24, 2024

PB also protects itself from duplicate injections so I think that shouldn't be an issue either (even if ideally it shouldn't happen at all)

There were/are parts of the codebase (e.g., mod activation from marketplace activation links) that were not properly considering content script injection on extension reload. (IIRC, you only considered UI elements with the context invalidation work/startup work)

We handled one of the race condition/injection bugs here: pixiebrix/pixiebrix-extension#8502

There's nothing to look at, on our side we just need to double-check assumptions about injection order

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants