Handle repeated injection gracefully

[fregante]

• Warn if a message was received when no handlers have been registered at all #74

Due to this error, mistakenly running the messenger twice (e.g. via executeScript) will now cause the second handler to respond with an error: The "registered methods" array is empty for every execution because it's not shared.

Possible solutions

• stop replying with "no handlers registered yet" error
  • Easiest, but losing DX. It could be replaced by a local error log
• detect duplicate injection and completely disable repeated runs (including registerMethods, which would have to throw an error)
  • Medium effort, high chance of future issues (feels like digging a deeper hole)
• make the "registered methods" really global, like window[MESSENGER_SYMBOLS] ??= []
  • Medium effort, medium chance of future issues, we'll have to ensure that all such data is stored globally

cc @twschiller

[twschiller]

FYI -- pre-rendering would explain the weird frame ids we were experiencing (that were causing duplicate injections):
w3c/webextensions#396 (comment)

[fregante]

This is what happens exactly and why duplicate injection is an issue:

1. Two copies of the messenger a run in sequence, each with its own registry
   • They are not deduplicated because webpack does not deduplicate across entry points
2. They both add their own onMessage listener, which looks into its own registry for matching MESSAGE_TYPE
3. The second copy is the one registering our expected ~40 methods

Now, when the content scripts receives a message, who handles it?

5. The first onMessage listener finds an empty registry and it handles the message, responding with the no methods registered here error
6. The second onMessage listener never hears of the message, even if it's ready to call its method

[BLoe]

This is what happens exactly

This summary definitely matches what I was seeing 💯

[twschiller]

Additional commentary from Chrome team on relationship between tabs and frames: https://developer.chrome.com/blog/extension-instantnav/#outermost-frame

FWIW, I think this method is still correct though for detecting in contentScript if it's loaded in an iframe?: https://github.com/pixiebrix/pixiebrix-extension/blob/80b362f6a3f869a81b63eb64b0a79919a642e1fc/src/utils/iframeUtils.ts#L22-L22

[fregante]

Re: Frames

I'm going to guess that those changes do not affect our usage for the most part, at least as far as the messenger is concerned, because we message alive and active documents. I think (I hope) that they didn't change the FRAME 0 === TOP assumption for active documents.

The messenger does not use webNavigation API so it should generally not see pre-rendered pages.

However the extension does use the API and its events, we should probably review its usage:

• Review usage of webNavigation events with regard to pre-rendered documents pixiebrix-extension#6665

---

FWIW, I think this method is still correct though for detecting in contentScript if it's loaded in an iframe?:

That's always valid, it's the only way for a content script to know that anyway, it does not have access to the frame ID in any other way, at least until:

• request: allow to retrieve a frameID from an <iframe> element w3c/webextensions#12

[fregante]

Quickest solution to avoid further frustration:

if (window[MESSENGER_SYMBOL]) {
	console.error('webext-messenger was loaded twice, do XYZ, https://link')
} else {
	window[MESSENGER_SYMBOL] = true;
}