Page MenuHomePhabricator

Implement utility functions to check if we have content or notif session with device
AcceptedPublic

Authored by marcin on Thu, Jul 4, 9:30 AM.
Tags
None
Referenced Files
F2322420: D12672.id42042.diff
Tue, Jul 23, 8:36 AM
Unknown Object (File)
Sat, Jul 20, 5:49 PM
Unknown Object (File)
Sat, Jul 20, 5:02 AM
Unknown Object (File)
Sat, Jul 13, 1:38 PM
Unknown Object (File)
Fri, Jul 12, 7:58 PM
Unknown Object (File)
Fri, Jul 12, 4:39 PM
Unknown Object (File)
Thu, Jul 11, 8:41 PM
Unknown Object (File)
Thu, Jul 11, 6:54 PM
Subscribers

Details

Reviewers
bartek
tomek
kamil
Summary

This differential implements simple utility functions to check if we have session for both content and/or notifs with particular device.

Test Plan

Tested in D12673

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin requested review of this revision.Thu, Jul 4, 9:50 AM
tomek added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
1477 ↗(On Diff #42042)

Is it guaranteed that this code never fails? Should we add some error handling?

web/shared-worker/worker/worker-crypto.js
756 ↗(On Diff #42042)

We should never place the await keyword deep inside the statement.

This revision is now accepted and ready to land.Wed, Jul 10, 4:56 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
1477 ↗(On Diff #42042)

We should:

  1. Check for contentCryptoModule nullability.
  2. We can add try{} catch{}. Wouldn't hurt but those are only hash map operations.

Add error handling to CommCoreModule::isContentSessionInitialized

kamil added a subscriber: kamil.
kamil added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
1480

do we need this check if we only care about content?