#1509 Migrate to Manifest V3

[sosnovsky]

This PR migrates extension code to Manifest V3

close #1509

---

Tests (delete all except exactly one):

• Tests added or updated

---

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

• addresses the issue it closes (if any)
• code is readable and understandable
• is accompanied with tests, or tests are not needed
• is free of vulnerabilities
• is documented clearly and usefully, or doesn't need documentation

[sosnovsky]

@ioanmo226 here is my current changes for manifest v3 migration, you can continue working on this code, feel free to ask questions:

• the biggest change in manifest v3 is usage of service workers instead of background pages - https://developer.chrome.com/docs/extensions/develop/migrate/to-service-workers. Previously background pages were constantly running in background, while service workers are alive only when we call them from extension.
• it breaks our ExpirationCache, as with current implementation it's not possible to use it for storing cache values for predefined period of time. I renamed it to SimpleExpirationCache, and tried to use chrome.storage.session for storing cache values
• also window. is not available in service worker, it led to many window is undefined errors. I replaced window.location.href with location.href, window.document.body with document.body etc and those errors are gone, but not sure if it breaks this code functionality
• I also copied code from extension/js/background_page to extension/js/service_worker with some changes to make it work with manifest v3

Per updated Google timeline - in June 2024 they'll start disabling manifest v2 extensions in pre-release versions of Chrome, so by that time we should have finished and tested migration to manifest v3.
It'll be great to have fully functional build in the beginning of April, so we can test it to find any issues during April and release final manifest v3 build in the beginning of May.

If some functionality can't be fully migrated to manifest v3 - we can simplify it for now and work on full functionality later. Google regularly adds new features to manifest v3, maybe some new feature will be helpful for us and make implementation easier.

P.S. Also Firefox has a different implementation for manifest v3 - https://extensionworkshop.com/documentation/develop/manifest-v3-migration-guide/. They continue support background pages, but they are non-persistent in manifest v3, need to adapt code for them too.

[ioanatflowcrypt]

@sosnovsky Thank you for your detailed response and code update.

I have now finished checking your code and manfest v3 update guide.

Could you list TODO list (to update to v3) you had in mind? It would help me complete task faster.

PS: I lost 2FA code for my previuos github account and submitted recovery action. Might be recovered in 1-2 days.

[sosnovsky]

Could you list TODO list (to update to v3) you had in mind? It would help me complete task faster.

For now we should start with these issues, as implementing them will make possible to run UI tests which will show broken functionality, and then it'll be easier to understand what needs to be fixed:

• insert "Secure compose" button on each gmail.com page load (currently it's inserted only on first load, I think, when service worker is initialized)
• implement replacement for current ExpirationCache, as with manifest v3 it's not possible to store something in memory for predefined amount in time

Initially I thought it'll be possible to create manifest v2 and v3 builds from the same codebase, then we could merge manifest v3 changes to master one by one, but it seems to be quite complicated, so we'll need to have separate branch for manifest v3 until it's ready to be released.

[ioanmo226]

Thank you for your answer. Let me implement above

[ioanmo226]

@sosnovsky Could you please check if Setup Flowrypt initial screen works fine in your side?
When I tap Sign in with google and then select account and click next, it just do nothing and console displays Uncaught (in promise) Error: No tab with id: 1745224200.

Also Secure compose button is not even inserted on first load in my side.
Could you check if you pushed latest changes?

[sosnovsky]

@sosnovsky Could you please check if Setup Flowrypt initial screen works fine in your side?
When I tap Sign in with google and then select account and click next, it just do nothing and console displays Uncaught (in promise) Error: No tab with id: 1745224200.

It probably doesn't work, as I've tested only already configured extension and didn't go through setup process.

Also Secure compose button is not even inserted on first load in my side.
Could you check if you pushed latest changes?

Yes, it's my latest changes, probably one of my last changes broke compose button insert even on the first load :)
Maybe it's related to inMemoryStorage, need to check if token is successfully fetched there

[ioanmo226]

Got it. Thank you for your answer/

[sosnovsky]

Got it. Thank you for your answer/

Feel free to message me if you'll have any questions about manifest v3 migration, as it's quite complicated issue and probably second opinion will be useful for making some implementation decisions.

[ioanmo226]

Which second option do you mean?

probably second opinion will be useful for making some implementation decisions

[sosnovsky]

Which second option do you mean?

I meant second opinion there :) So if you're not sure how some functionality should be converted to manifest v3, we can discuss implementation details here to find the best solution

[ioanmo226]

Aha, I see

[ioanmo226]

@sosnovsky This one is one is now fixed.

@sosnovsky Could you please check if Setup Flowrypt initial screen works fine in your side?
When I tap Sign in with google and then select account and click next, it just do nothing and console displays Uncaught (in promise) Error: No tab with id: 1745224200.

However InMemoryStore.getUntilAvailable function does not perform as expected; it only invokes the InMemoryStore.get method once, even if the value remains undefined.
This issue likely arises because the service worker remains active only during use and is terminated afterwards, preventing the use of Timer.sleep.
Given the necessity of setInterval or setTimeout in various parts of the extension, I recommend utilizing chrome.alarms instead.

I have implemented this solution in the latest commit. I welcome your feedback and am open to any suggestions you may have. Additionally, please inform me if you consent to adding the alarms permission.

flowcrypt-browser/extension/js/common/platform/store/in-memory-store.ts

Line 23 in e3b3e63

public static getUntilAvailable = async (acctEmail: string, key: string, retryCount = 20): Promise<string | undefined> => {

3abf9f9

PS: I thought there was logic in Catch.setHandledTimeout but I can see that it does nothing.
Maybe we need to implement setHandledTimeout and setHandledInterval with chrome.alarms api?
Or you have better suggestion to implement those?

flowcrypt-browser/extension/js/common/platform/catch.ts

Line 257 in 2ab84bd

return ms; // window.setTimeout(Catch.try(cb), ms); // error-handled: else setTimeout will silently swallow errors

[sosnovsky]

Given the necessity of setInterval or setTimeout in various parts of the extension, I recommend utilizing chrome.alarms instead.
I have implemented this solution in the latest commit. I welcome your feedback and am open to any suggestions you may have. Additionally, please inform me if you consent to adding the alarms permission.

Yes, we'll need to use alarms permission, as it's required for running timers in service worker (https://developer.chrome.com/docs/extensions/develop/migrate/to-service-workers#alarms), so you can add it to manifest.json. As I remember, adding alarms permission shouldn't show additional permissions alert for users.

By the way, when working on manifest v3 update I found tool for testing extension update - https://github.com/GoogleChromeLabs/extension-update-testing-tool. It'll show how update to the new version will look for users, so you can check if any additional permission alerts will be shown (we should try to migrate without such additional alerts, as some users can decline them and then some functionality will be broken))

PS: I thought there was logic in Catch.setHandledTimeout but I can see that it does nothing.
Maybe we need to implement setHandledTimeout and setHandledInterval with chrome.alarms api?

I commented initial logic and temporary replaced it with return ms, as extension failed to build because of window is undefined error. We'll need to reimplement it using chrome.alarms API, you're right

[ioanmo226]

Got it. Thank you for your detailed comment

[sosnovsky]

Do we have to fix this issue in forge and submit PR?

It seems node-forge isn't actively developed now, last release was a couple years ago.
Probably we can apply fixes to node-forge when building extension, like we currently do for openpgp and web-stream-tools, what do you think?

flowcrypt-browser/scripts/build.sh

Line 124 in 2bbe516

apply_regex_replace $STREAMS_REGEX $STREAMS_FILES

[ioanmo226]

I agree but not sure if it can be easily fixed with just regex replace or something like that.
Let me try.

[ioanmo226]

@sosnovsky I tried to bundle background.js with webpack but I'm stuck at this. :-(
It still displays forge.pkcs7, forge.asn1, etc are undefined.

Could you take a look at this issue? Would be much appreciated.
I'll also look at this issue too though.

[sosnovsky]

Could you take a look at this issue? Would be much appreciated. I'll also look at this issue too though.

For sure, I'll check it later today or tomorrow

[ioanmo226]

@sosnovsky Have you checked?

[sosnovsky]

@sosnovsky Have you checked?

Yes, I'm checking it - tried to build without webpack and noticed that forge.js isn't included in lib folder for service worker (only openpgp and streams files are there). I think that's why building with webpack doesn't help, as importing forge doesn't correctly references forge.js, checking it further now

[ioanmo226]

Hmm.. for me even without webpack, forge is included in lib folder.

[ioanmo226]

Maybe you didn't remove this comment in tsconfig.json?

[sosnovsky]

Maybe you didn't remove this comment in tsconfig.json?

Yeah, you're right, after removing this comment forge.js appeared in service worker, thanks for the tip!

[ioanmo226]

@sosnovsky I tried to build forge mjs file from forge repository with rollup bundler.
It seems to work fine. What do you think about my solution?

[sosnovsky]

@sosnovsky I tried to build forge mjs file from forge repository with rollup bundler. It seems to work fine. What do you think about my solution?

For me it doesn't work - secure compose button isn't added to Gmail page and there are errors in console:

Does it work for you?

[ioanmo226]

OHhh. yep
It doesn't work in gmail page.
Just tried extension pages and mock ui tests.

[sosnovsky]

@ioanmo226 did you try to use offscreen document for fixing openpgp and forge imports? Here is another article - https://developer.chrome.com/blog/Offscreen-Documents-in-Manifest-v3
Not sure if it can be used for this, but can be worth checking.

[ioanmo226]

Haven't tried yet. But I heard using offscreen object for referencing variable is not good idea and should not be used.
I think from online article or something like that.
Let me dig deep though

[ioanmo226]

@sosnovsky How about now? What do you think about solution?
When I import like above import * as forge then i have to call forge.default.asn1 instead of forge.asn1. Therefore I added above logic.
Maybe there could be simple solution for this.

Let me know your suggestion.

[sosnovsky]

I tried to find simpler solution, but wasn't able to implement something workable. Your solution works well, let's use it - maybe we'll find some way to replace it with simpler one in the future, but for now it should be ok.

I noticed that many tests are failing because of XMLHttpRequest is not defined error (as it's not available in service worker, only fetch should be used there), maybe we can move ajaxGmailAttachmentGetChunk to content script?

[ioanmo226]

Ok, I'm currently fixing test re-auth after updating chrome extension test (It fails because OAuth2.js coudln't get window object because it was running from background). Fixing it to pass requried screen dimensions to service worker and pass it to OAuth2.

Let me have a look at ajaxGmailAttachmentGetChunk after current one

[ioanmo226]

I have a question. Why don't we just use fetch in ajaxGmailAttachmentGetChunk?

[ioanmo226]

Also I think we couldn't move ajaxGmailAttachmentGetChunk to content script because of below reason?

flowcrypt-browser/extension/js/common/api/email-provider/gmail/gmail.ts

Line 160 in cb1423a

// content script CORS not allowed anymore, have to drag it through background page

[ioanmo226]

I found reason and it's because when message is returned from ajaxGmailGetChunk, with the createObjectUrl usage removed object is returned instead of Buf.
This is a temp fix and it fixed issue.
4262aee

[ioanmo226]

I found reason and it's because when message is returned from ajaxGmailGetChunk, with the createObjectUrl usage removed object is returned instead of Buf.
This is a temp fix and it fixed issue.
4262aee

[ioanmo226]

Also FYI created new PR as we also have to test live test with new branch name

[sosnovsky]

I found reason and it's because when message is returned from ajaxGmailGetChunk, with the createObjectUrl usage removed object is returned instead of Buf. This is a temp fix and it fixed issue. 4262aee

Ok, let's just add // todo: it's a temporary fix, should be removed comment, so we won't forget about it

[ioanmo226]

Okay