This time limit matches the one we introduced and justified for Android. Android has 20 seconds to process notif, iOS has 30. Theoretically we could increase it to 20 but maybe it is better to keep behaviour similar between platforms?

Why to make this class singleton?
This class is mostly responsible for executing operations on NSURLSession instance. According to Apple Docs, NSURLSession instances should be shared: https://developer.apple.com/documentation/foundation/url_loading_system/fetching_website_data_into_memory?language=objc.

Additionally I have done some experimentation and established that NSE uses different threads to process subsequent notifications but it uses the same process for a prolonged period of time. Once I had sent first notification to iOS device I was seeing the same PID being logged to the console for each notifications for > 15 minutes. Therefore I think it is better to create this object once end just keep it then to create it for every invocation.

This way of implementing singleton is inspired by our CommSecureStoreIOSWrapper.

Why to pass mainQueue here? According to apple docs: https://developer.apple.com/documentation/foundation/nsurlsession/1411597-sessionwithconfiguration, passing nothing makes NSURLSession instance create its own queue and thread to process request completion handlers. We don't want to create new threads when we process notif so I pass mainQueue here since it is always associated with the main thread which is always present so we don't create new threads when processing notif.

Why synchronous? Unfortunately I don't have any Apple docs to support it - just my own experimentation. Basically I tried to make this call async in two ways:

1. Process notification payload and call notification completionHandler in data task completionHandler. I observed that even though we download large notification and call notification completionHandler on a different thread NSE still waits before it starts to process new notification. I sent two notifs: one large and one small. Small notification was displayed afer large notification processing completed even though it took place on separate thread.
2. Download and process notification payload on a separate thread, but call notification completionHandler immediately on the same thread NSE received notification. I observed that after calling completionHandler NSE dropped the download task and large notification payload was not persisted.

So my conclusion is that NSE doesn't process new notification until we call completionHandler of previous notification no matter the thread it takes place. Therefore there is no good way to make this call async in NSE.

I made some of NSE methods static since I want to call them in a block and avoid this issue: https://bytes.vokal.io/objc-block-capture-weakself/. Those methods were stateless anyway .

I'd leave 15 - Shouldn't notif processing be as quick as possible?

Yeah, singleton seems to be good option here

Technically I'd prefer this constant to be defined in the AES Crypto module, however I'm not sure if ObjC <-> Swift won't complicate it too much - might be not worth it

1. Do we have to create this object with each call? Can it be static or singleton? I had the same concern for Android: https://phab.comm.dev/D8566?id=28842#inline-54921
2. Can we give this variable a meaningful name?

This TODO section is about authentication with COMM_ACCESS_TOKEN. I was advices not to land this feature until COMM_ACCESS_TOKEN staff is resolved and utilised in this stack. So I don't think we need a task for it since authentication is a part of this task.

The encryptionKey is sent by the keyserver in olm-encrypted notification. It is delivered as base 64 string an converted to bytes there since AESCryptoModule requires bytes.

This signature got me confused.
General idea of Obj-C/Swift naming is that it can be readable in human language, e.g a few lines below there is

binaryAuthObject = [NSJSONSerialization dataWithJSONObject:jsonAuthObject
                                                       options:0
                                                         error:&jsonError];

// From NSJSONSerialization get data, with given JSON object, options and error

Why is that the case?

Would that mean that it is impossible to use the local blob service?

What unit do we use here?

Capitalize Notifs

Seems like very narrow columns here. Can we stick with 80 chars?

Also typo in instancess

The context is here: https://linear.app/comm/issue/ENG-3797/download-notification-payload-from-blob-service-if-its-size-exceeded#comment-4d35b32c. My understanding is that it was a business decision that was made when MacOS notifications were first introduced. The reason here was probably the fact that initially MacOS notifications were handled entirely inside a main desktop process where there is no access to redux so messageInfos were simply unnecessary. Technically it would be possible to transfer messageInfos from notification to renderer proces using ipc but it wasn't implemented. Making messageInfos useful in MacOS notifications is an option that we can consider but it is out of the scope of this differential. cc @michal

Using local blob service will be possible after replacing one of the blobServiceAddress above. Developer willing to use local blob service has to hardcode its address anyway in lib/facts/blob-service.js.

Its seconds. This constant is used a couple of lines below where setting request timeout which require timeout value in seconds: https://developer.apple.com/documentation/foundation/nsurlsessionconfiguration/1408259-timeoutintervalforrequest

Completely unsolicited thought, but does this code need to be in Objective C? At a high level this seems like a LOT of potentially error-prone code just to make an HTTP request?

I'm not suggesting we do it, but code to make an HTTP request in Swift would be a lot simpler thanks to eg async/await

I will create a follow up task to rewrite it in Swift. However I don't think it is actionable right now. The purpose of this project was to refresh most of the existing code. Rewriting in swift would require to add some headers with bindings do additional testing which could delay this project further.

I was unable to find any instance method of NSHTTPURLResponse to check if the request was successful or not. It is pretty low level API so I think we have to check maunally.

Eh not sure we need a task, it was more just commentary rather than something to prioritize