Details

Reviewers

kamil
michal

Commits

rCOMM948186159a46: Implement method to decrypt desktop notification

Summary

This differential refactors web notification decryption code so that the part that is common for web and desktop is extracted to separate function and other
functions that are specific for web and desktop are implemented.

Test Plan

Test that web notifications are unbroken.
Testing desktop notifications is not possible without IPC boilerplate that will be introduced in future diffs.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Nov 23 2023, 8:41 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptNov 23 2023, 8:41 AM

marcin added a child revision: D9963: Extend showNotification method to cover the case of decryption error notitication.Nov 23 2023, 8:49 AM

Harbormaster completed remote builds in B24380: Diff 33573.Nov 23 2023, 9:13 AM

marcin requested review of this revision.Nov 23 2023, 9:13 AM

michal added inline comments.Nov 24 2023, 6:03 AM

web/push-notif/notif-crypto-utils.js
109 ↗	(On Diff #33573)	Why don't we need to specify `locateFile` for desktop? Could this be awaited in parallel in the previous `Promise.all`? (this question also applies to web, sorry for not catching this before)

marcin added inline comments.Nov 24 2023, 6:11 AM

web/push-notif/notif-crypto-utils.js
109 ↗	(On Diff #33573)	Why don't we need to specify locateFile for desktop? On web this code runs in `service-worker` context where `olmFilename` variable is not automatically populated. On desktop however this code is going to execute in main web thread so we can use `initOlm` as we do in other places in web. Could this be awaited in parallel in the previous Promise.all? (this question also applies to web, sorry for not catching this before) We can await in parallel with previous promises for desktop. We can't do so for web since path to WASM file is retrieved from localforage in previous promises.