clients
clients copied to clipboard
[PM-5882] Adjust usage of `chrome.extension.getBackgroundPage` to ensure no conflicts exist due to the service worker
Type of change
- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other
Objective
The objective of this PR is to ensure that usage of chrome.extension.getBackgroundPage does not alter expected logic and behavior within manifest v3. To that end, this was partially an investigative task, which yielded the following results:
chrome.extension.getBackgroundPageis leveraged explicitlyBrowserApi.getBackgroundPagemethod, which in turn is called in a couple of locations. These includeBrowserApi.isBackgroundPage,BrowserPopupUtils.backgroundInitializationRequired, and withinservices.module.ts.BrowserApi.isBackgroundPageis called byBrowserApi.addListener,BrowserApi.removeListener, and theSessionSyncerclass.- Usage of
BrowserApi.isBackgroundPagein theBrowserApi.addListenerandBrowserApi.removeListenermethods should not be affected by the mv3 service worker, as the call is only made for Safari extensions. That said, I've removed usage of thewindowobject and referenced that global value asselfto fix any potential issues that might arise from calling thewindow. - Usage of
BrowserApi.isBackgroundPagein theSessionSyncerclass is handled withinSessionSyncer.updateandSessionSyncer.updateSession.SessionSyncer.updateuses theBrowserApi.isBackgroundPagecall to save a value to the passedMemoryStorageServiceif the extension is within manifest v2 and the call happens within the backgorund page.SessionSyncer.updateSessionuses theBrowserApi.isBackgroundPagemethod to handle saving to the passedMemoryStorageServiceif the call happens in an manifest v3 extension or within the background page script. In both these instances, it seems that the implementation was put in place with the intention of having different behavior between manifest v2 and 3. Regardless of that implementation choice, it doesn't seem like callingBrowserApi.isBackgroundPagewill cause problems in mv3. - Usage of
BrowserPopupUtils.backgroundInitializationRequiredseems to happen within the context of the vault popup only, and as a result it will not cause issues directly with mv3. More importantly however, the usage of this will be phased out entirely as the only location where this call is used isservices.module.ts. It is a know requirement that we refactor usage ofchrome.extension.getBackgroundPageandBrowserPopupUtils.backgroundInitializationRequiredout of the implementation completely.
Code changes
- apps/browser/src/platform/browser/browser-api.ts: Updating references to the
windowobject to refer to the globalselfobject instead.
Codecov Report
Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.
Project coverage is 24.83%. Comparing base (
929b5eb) to head (5f5cec4). Report is 1 commits behind head on main.
:exclamation: Current head 5f5cec4 differs from pull request most recent head 6067b06. Consider uploading reports for the commit 6067b06 to get more accurate results
| Files | Patch % | Lines |
|---|---|---|
| apps/browser/src/platform/browser/browser-api.ts | 0.00% | 0 Missing and 2 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #8054 +/- ##
=======================================
Coverage 24.83% 24.83%
=======================================
Files 2233 2233
Lines 65511 65511
Branches 12363 12363
=======================================
Hits 16271 16271
Misses 47914 47914
Partials 1326 1326
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Checkmarx One – Scan Summary & Details – 1cb194d5-1c6c-42f4-9956-6b9d49cdd893