Closed ghugues closed 3 years ago
Many thanks for the PR !
However, for others suggestions, we will check it for a rework of our sdk and it should be dangerous to refactor this kind of code because it answered to some customers use case unfortunately
Regards,
After some tests, your modification for getUserAgent doesn't work for the first hit
After some tests, your modification for getUserAgent doesn't work for the first hit
You're right. Il made a terrible mistake : the timeout given to wait
is not relative it's absolute (not really intuitive, but I should have tested this).
I see you've already merged the PR so here is the fix https://github.com/lemonde/atinternet-apple-sdk/commit/bf353df31eb3db8cd60de06e28349b5c9f350d43
Thanks again it's ok, all your changes will be integrate in the next release very soon
Description
This pull request contains various fixes that we had to make to the version 2.23.0 of SDK before releasing our apps last week. Please consider adding them to the main repository so that we can use it again instead of our fork (which will inevitably diverge).
#if swift(>=5.3)
condition around Core Telephony constants new in the Xcode 12 SDK so that this project still compiles with Xcode 11.#if canImport(CryptoKit)
around all uses of the CryptoKit framework because it appears to be unavailable at compile time when targeting a 32 bits platform on Xcode 11 (this can be reproduced by compiling for a "Generic iOS Device").Int
is a 32 bits integer (whereasInt
is a 64 bits integer on 64 bits platforms).WKWebView
. There is now a maximum timeout on the semaphore, and the webview is added (hidden) to the main window when possible to make sure Javascript execution is not paused indefinitely..background
quality of service on the network queue because operation and dispatch queues with this qos can be paused indefinitely by the system in some conditions (see https://github.com/AFNetworking/AFNetworking/issues/4223 for example)..background
network service type on the network requests because analytics tracking is not a background download and should not use this type of service. This kind of service could be delayed for long periods of time by the system, which is not what we want here.Going further
By digging around in this SDK I have seen some parts that could really be improved. Mainly :
DispatchSemaphore
to synchronize intrinsically asynchronous operations should be avoid at all costs. This will block a dispatch queue and a thread for no reason and consume resources unnecessarily. This is especially true for network requests.URLSession
for each request and destroying it afterwards is not howURLSession
is designed to be used. It consumes ressources unnecessarily and prevents the system from doing several optimisations, mainly : keeping the HTTP connexion open and reusing it for several requests. But more broadly, most of the performance improvements that HTTP/2 has to offer will be unavailable. Instead a singleURLSession
should be created at app launch and reused for all requests. Also, if there is no need for specific customization, use the.shared
session (or even better, let clients of the SDK provide their own session).WKWebview
simply to obtain the user agent should actually be avoided. It consumes a lot of ressources on initialization and slows down app launch. If really necessary, a user agent could easily be constructed natively without the use of a webview (see what Alamofire does for example).