Skip to content

[DRAFT] Support manifest v3 in chrome extension#1977

Open
daniellandau wants to merge 1 commit intomainfrom
manifest-v3-chrome-extension
Open

[DRAFT] Support manifest v3 in chrome extension#1977
daniellandau wants to merge 1 commit intomainfrom
manifest-v3-chrome-extension

Conversation

@daniellandau
Copy link
Copy Markdown
Member

This is a very much not cleaned up hack job of "look at error message", "comment something out", "do minimal change", "rinse and repeat" of an effort, but it does seem to work on Chrome. I have had a half a mind of sticking it to the man and never updating to manifest-v3 and half a mind of it'd be great if they actually did disable the chrome extension so I'd stop getting those obnoxious "we buy chrome extensions" emails, but here we are all the same.

I've never done any webextensions before, so I'd especially appreciate a review focusing on "does this cause actual havoc / security vulnerabilities etc". I didn't test at all on Firefox, so I don't know what if anything should be done about that, or perhaps hope that we never have to update the Firefox extension.

@daniellandau daniellandau requested a review from andyholmes March 29, 2025 17:24
@daniellandau
Copy link
Copy Markdown
Member Author

Fails linting and REUSE, as expected. To be fixed if the approach otherwise is sound

@ferdnyc
Copy link
Copy Markdown
Member

ferdnyc commented Mar 29, 2025

(Warning: Long.)

I've been poking at this a bit, as well. This may work (though frankly I'm surprised that the context menus work without an onclick handler... how do they know what to do?), but I'm not sure it's Right™, and specifically I'm not sure it's right enough to fly with Google's extension evaluation process.

The current design of the extension is as an always-running background process that connects to our native messaging host and relays its events. But that's already not particularly how things are intended to work, and indeed often they don't work because of that. (For example, in current Chrome versions the GSConnect entries often don't show up in the context menu, unless I first open and close the extension icon's popup. Then they're suddenly there.)

With Manifest v3, AIUI things are changing so that extension service workers are very much an on-demand thing: they get launched only when needed and they're expected to shut down ASAP. That means that all of the state-management and connection monitoring in the current background.js isn't really helpful anymore, and in fact is a waste of time and cpu cycles — it needs to focus on doing its work and getting out of there.

Google's own more complex context-menu example (complex relative to their other, even simpler example) takes this approach to the context menu:

  • Create the overall context menu in a runtime.onInstalled listener, from the extension service worker.
  • Also install a contextMenus.onClicked handler from the service worker, to execute the actual function of the menu items.
  • Use the Web Storage API to store user selections regarding the enabled menu options, from the extension popup.
  • Run a storage.onChanged listener in the worker, to pick up changes to the user selections and create/remove context menu options as necessary.

That doesn't exactly help us, because our menu options are supplied by an outside source (the NMH), not user actions. But I think the closest version of this, for us, would be something like:

  • Use an onInstall handler to query the NMH for all (a) paired and (b) connected devices, tuck that list away in the Web Storage API (no State object in the worker anymore), and then set up the initial context menu
  • List all paired devices in the context menu, whether or not they were connected when we last checked. (We can maybe look at graying out the ones that don't seem to be connected, but we shouldn't expect to have up-to-the-minute data on that at all times so maybe it's better to let the user decide what device to send to, whether or not we think it's available.)
  • Inject a content script on each page load, that tells the service worker to check with the NMH and update the Web Storage device list, so that it stays reasonably current. This function should be lightning fast and do minimal unnecessary work (correction: it should do NO unnecessary work, and the minimum necessary work), obviously, if it's going to be run that often.
  • Use the Web Storage data to populate the GSConnect popup when it's opened, possibly after (again) having the worker update the list real quick. (And, that can happen asynchronously -- if the popup code also uses a storage.onChanged listener to respond to changes in the list, then it can edit the list of displayed items after it's already been opened, in response to updates to the Web Storage data. Yes, that will create that annoying situation that's so common in modern online services where a cached list is initially displayed, then the options change a few hundred milliseconds later after the cache is refreshed. But, this is why it's so common in modern interfaces.)

Basically, the worker shouldn't maintain any state internally because it should expect that it's not going to be running most of the time; it should move to a "do what you need to do right now and get out ASAP" model. And it shouldn't fart around with things like monitoring the connected status of the NMH or forwarding device connect/disconnect events, because that's not its job. The current state at the time it's executed is what it has to work with.

I was thinking this might cause problems for requests coming from the phone, like an "open this URL in my desktop browser" request... but neither the extension nor the NMH are involved in that, it just uses Gio.AppInfo.launch_default_for_uri_async() to execute those requests.

So, the worker can, and therefore should, be stateless and focus on making active requests for information, not listening for updates from the NMH — it's not meant to be sticking around long enough to bother with that.

@ferdnyc
Copy link
Copy Markdown
Member

ferdnyc commented Mar 29, 2025

  • List all paired devices in the context menu, whether or not they were connected when we last checked. (We can maybe look at graying out the ones that don't seem to be connected, but we shouldn't expect to have up-to-the-minute data on that at all times so maybe it's better to let the user decide what device to send to, whether or not we think it's available.)

If we don't already have this (I honestly can't remember), we could also think about queuing requests sent to offline paired devices until they next reappear. We should be extremely conservative in doing that, though — I'd say the limit on that queue should be the most recent one or two requests maximum, so that even if a user keeps firing requests at an offline device, they don't end up with 17 stale URLs opening all at once the next time GSConnect sees it.

@daniellandau
Copy link
Copy Markdown
Member Author

I was confused about things working after commenting out the onclick stuff, but that explains it: there's a context menu that I didn't notice exists. That indeed does not work, and maybe it's best to just remove it.

The thing that does seem to work is hitting the extension button to share the current page and if I understood correctly that shouldn't require running anything in the background for extended periods of time.

That means that all of the state-management and connection monitoring in the current background.js isn't really helpful anymore, and in fact is a waste of time and cpu cycles — it needs to focus on doing its work and getting out of there.

Yes I think I agree.

@ferdnyc
Copy link
Copy Markdown
Member

ferdnyc commented Mar 30, 2025

I was confused about things working after commenting out the onclick stuff, but that explains it: there's a context menu that I didn't notice exists. That indeed does not work, and maybe it's best to just remove it.

Well, it is kind of nice to be able to right-click on a link or an image or something, and send that to your phone instead of just the current page. I'm not saying it's critical, but if it could be made to work again... (And I believe it can, just perhaps without constant monitoring of which devices are currently connected to GSConnect.)

@ferdnyc
Copy link
Copy Markdown
Member

ferdnyc commented Mar 30, 2025

I was confused about things working after commenting out the onclick stuff, but that explains it: there's a context menu that I didn't notice exists. That indeed does not work, and maybe it's best to just remove it.

Well, it is kind of nice to be able to right-click on a link or an image or something, and send that to your phone instead of just the current page. I'm not saying it's critical, but if it could be made to work again... (And I believe it can, just perhaps without constant monitoring of which devices are currently connected to GSConnect.)

The other thing we could do, thinking about it more, is emulate how Chrome implements its own "Send to your devices" functionality. They have a context-menu entry for that (which is only enabled for pages, not page elements like links, photos, etc... I am absolutely baffled as to WHY), but it's static -- all it ever shows is "Send to your devices". When you click it, they open a popup (not attached to any toolbar icon, just floating over the page) where you can choose which device to target.

image

So, our context menu could just be a static "Send with GSConnect" option or whatever that, when clicked, opens our popup. But with the added feature that if you use the context-menu entry from a non-page context (link, image, etc.), it sends that URL to whatever device you choose, instead of the current page URL.

@andyholmes
Copy link
Copy Markdown
Collaborator

Sorry for the delay commenting, I really haven't touched the WebExtension since it was written.

I'd guess my two-cents is probably just "do what everyone else is doing" :) I wouldn't expect the state storage to be that resource intensive, but if starting the NMH is less expensive, devices are something that can be queried on-click (it's 3 strings and 2 booleans, per device).

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

Successfully merging this pull request may close these issues.

3 participants